From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web12.3468.1600737755759319443 for ; Mon, 21 Sep 2020 18:22:37 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Tue, 22 Sep 2020 09:22:31 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Michael Kubacki'" , "'Ni, Ray'" , Cc: "'Kinney, Michael D'" , "'Liu, Zhiguang'" References: In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHYxIDEvMV0gTWRlUGtnOiBDb3JyZWN0IEVGSV9CTE9DS19JT19QUk9UT0NPTF9SRVZJU0lPTjMgdmFsdWU=?= Date: Tue, 22 Sep 2020 09:22:33 +0800 Message-ID: <008d01d6907e$d8e71430$8ab53c90$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHrQTElO+C2weWgVKpiTxNgYkWi+gD1wFtlAhOChlIBCZpmFgLdQMnxAaXJI9ECJFf8Rqj0osEQ Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Michael and Ray: So far, I don't see the big impact with this change. I search Edk2/Edk2P= latform code base. I only find MdeModulePkg ScsiDiskDxe installs BlkIo with= revision 3, and PartitionDxe checks BlkIo revision. They are in the same p= ackage, and are used together. So, there is no impact for them. Now, the kn= own impact is Disk Utility. The old binary Disk Utility may not work with t= he bios image built from the latest edk2 on ScsiDisk. I would suggest to cl= arify this impact in the release note of the stable tag 202011. If more imp= act is reported later, we can consider the solution again.=20 Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Michael Kubacki > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B49=E6=9C=8822=E6=97=A5= 4:00 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Ni, Ray ; devel@edk2.grou= ps.io > =E6=8A=84=E9=80=81: Kinney, Michael D ; Limi= ng Gao > ; Liu, Zhiguang > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > EFI_BLOCK_IO_PROTOCOL_REVISION3 value >=20 > I found the issue while manually reviewing some protocol structures > against the UEFI spec. >=20 > I'm going to leave that question to the MdePkg and UEFI Specification > maintainers as changing the specification may have an impact on other > implementations that needs to be considered. My only preference on the > topic is that the edk2 implementation and UEFI Specification do not > conflict. Since the patch to match the value against the spec has been > submitted, I'm happy to make any potential modifications based on the > decision. >=20 > Thanks, > Michael >=20 > On 9/20/2020 7:28 PM, Ni, Ray wrote: > > Disk Utility code that consumes the REVISION3 macro: > > if (BlkIo->Revision >=3D EFI_BLOCK_IO_PROTOCOL_REVISION3 && > > BlkIo->Media->OptimalTransferLengthGranularity !=3D 0 > > ) { > > // > > // Compute the least common multiple of > OptimalTransferLengthGranularity and LogicalBlocksPerPhysicalBlock > > // > > *OptimalTransferBlocks =3D Lcm ( > > BlkIo->Media->OptimalTransferLengthGranularity, > > *OptimalTransferBlocks > > ); > > } > > > > Even we could release a new version of tool with the correct value, th= ere is > no way to inform the > > old tool users to do the upgrade. > > > > What caused the issue to be found? > > Would keeping the implementation unchanged but correcting the spec > content be the easier way? > > > > Thanks, > > Ray > > > >> -----Original Message----- > >> From: Michael Kubacki > >> Sent: Saturday, September 19, 2020 8:26 AM > >> To: Ni, Ray ; devel@edk2.groups.io > >> Cc: Kinney, Michael D ; Liming Gao > >> ; Liu, Zhiguang > >> Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > >> EFI_BLOCK_IO_PROTOCOL_REVISION3 value > >> > >> What do you propose as an alternative? > >> > >> Can a new version of the tool be released with the correct value? > >> > >> Thanks, > >> Michael > >> > >> On 9/18/2020 4:53 PM, Ni, Ray wrote: > >>> As far as I know, EFI disk utility consumesthis new field for > >>> performance. The utility is in Intel website for external downloads. > >>> > >>> --------------------------------------------------------------------= ---- > >>> *=E5=8F=91=E4=BB=B6=E4=BA=BA:* devel@edk2.groups.io =E4=BB=A3=E8=A1=A8 > Michael > >>> Kubacki > >>> *=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4:* Saturday, September 19, 2020= 2:54:54 AM > >>> *=E6=94=B6=E4=BB=B6=E4=BA=BA:* devel@edk2.groups.io ; Ni, Ray > >>> > >>> *=E6=8A=84=E9=80=81:* Kinney, Michael D = ; Liming Gao > >>> ; Liu, Zhiguang > >>> *=E4=B8=BB=E9=A2=98:* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correc= t > >>> EFI_BLOCK_IO_PROTOCOL_REVISION3 value > >>> Hi Ray, > >>> > >>> Rev3 adds the UINT32 field OptimalTransferLengthGranularity field to > >>> EFI_BLOCK_IO_MEDIA. A preexisting binary Block I/O producer that use= s > >>> this field will set their revision to the higher value and the only > >>> check I see in edk2 (PartitionDxe) on the revision to access this fi= eld > >>> checks for >=3D EFI_BLOCK_IO_PROTOCOL_REVISION3. > >>> > >>> If a binary Block I/O producer is built with the new value that is > >>> consumed by a module built with the older value it might ignore the > >>> OptimalTransferLengthGranularity field. I do not see where this is t= he > >>> case in edk2 other than PartitionDxe which sets the > >>> OptimalTransferLengthGranularity field to zero for Rev3. > >>> > >>> You have contributed to this code in the past so feel free to provid= e > >>> any further insight if needed. > >>> > >>> That said, this change was made to fix a bug in the edk2 implementat= ion > >>> to remove a conflict with the UEFI Spec, the two should be in agreem= ent. > >>> > >>> I suggest the change be added to the next stable tag release notes s= o > >>> authors of such modules are made aware they should release an update > >>> with the new revision value. > >>> > >>> Thanks, > >>> Michael > >>> > >>> On 9/17/2020 6:25 PM, Ni, Ray wrote: > >>>> Mike, > >>>> Have you evaluated the impact to the already-released module that > relies > >> on the macro value? > >>>> > >>>> Basically, you changed to a smaller value that may cause a revision= 3 > check > >> fail: > >>>> a released module expects the revision is bigger than 0x31, but the > value is > >> 0x1f. > >>>> > >>>> Thanks, > >>>> Ray > >>>> > >>>>> -----Original Message----- > >>>>> From: devel@edk2.groups.io On Behalf Of > >> Michael Kubacki > >>>>> Sent: Tuesday, September 15, 2020 2:11 AM > >>>>> To: devel@edk2.groups.io > >>>>> Cc: Kinney, Michael D ; Liming Gao > >> ; Liu, Zhiguang > >>>>> > >>>>> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > >> EFI_BLOCK_IO_PROTOCOL_REVISION3 value > >>>>> > >>>>> From: Michael Kubacki > >>>>> > >>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2961 > >>>>> > >>>>> The value of EFI_BLOCK_IO_PROTOCOL_REVISION3 is currently > >>>>> 0x00020031. However, the value assigned in the UEFI Specification > >>>>> 2.8B is ((2<<16) | (31)) which is 0x0002001F. > >>>>> > >>>>> Cc: Michael D Kinney > >>>>> Cc: Liming Gao > >>>>> Cc: Zhiguang Liu > >>>>> Signed-off-by: Michael Kubacki > >>>>> --- > >>>>> MdePkg/Include/Protocol/BlockIo.h | 2 +- > >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/MdePkg/Include/Protocol/BlockIo.h > >> b/MdePkg/Include/Protocol/BlockIo.h > >>>>> index 7b332691ede3..3bd76885e11c 100644 > >>>>> --- a/MdePkg/Include/Protocol/BlockIo.h > >>>>> +++ b/MdePkg/Include/Protocol/BlockIo.h > >>>>> @@ -201,7 +201,7 @@ typedef struct { > >>>>> > >>>>> #define EFI_BLOCK_IO_PROTOCOL_REVISION 0x00010000 > >>>>> #define EFI_BLOCK_IO_PROTOCOL_REVISION2 0x00020001 > >>>>> -#define EFI_BLOCK_IO_PROTOCOL_REVISION3 0x00020031 > >>>>> +#define EFI_BLOCK_IO_PROTOCOL_REVISION3 0x0002001F > >>>>> > >>>>> /// > >>>>> /// Revision defined in EFI1.1. > >>>>> -- > >>>>> 2.28.0.windows.1 > >>>>> > >>>>> > >>>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>> > >>> > >>>=20 > >>> > >>>