* [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value @ 2020-09-14 18:11 Michael Kubacki 2020-09-15 0:54 ` 回复: [edk2-devel] " gaoliming ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Michael Kubacki @ 2020-09-14 18:11 UTC (permalink / raw) To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu From: Michael Kubacki <michael.kubacki@microsoft.com> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 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 <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> --- 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-14 18:11 [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value Michael Kubacki @ 2020-09-15 0:54 ` gaoliming 2020-09-17 16:49 ` Michael Kubacki 2020-09-15 3:27 ` Zhiguang Liu 2020-09-18 1:25 ` [edk2-devel] " Ni, Ray 2 siblings, 1 reply; 13+ messages in thread From: gaoliming @ 2020-09-15 0:54 UTC (permalink / raw) To: devel, michael.kubacki; +Cc: 'Michael D Kinney', 'Zhiguang Liu' Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> > -----邮件原件----- > 发件人: bounce+27952+65228+4905953+8761045@groups.io > <bounce+27952+65228+4905953+8761045@groups.io> 代表 Michael > Kubacki > 发送时间: 2020年9月15日 2:11 > 收件人: devel@edk2.groups.io > 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com> > 主题: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > EFI_BLOCK_IO_PROTOCOL_REVISION3 value > > From: Michael Kubacki <michael.kubacki@microsoft.com> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 > > 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 <michael.d.kinney@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > --- > 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 > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-15 0:54 ` 回复: [edk2-devel] " gaoliming @ 2020-09-17 16:49 ` Michael Kubacki 2020-09-18 1:08 ` 回复: " gaoliming 0 siblings, 1 reply; 13+ messages in thread From: Michael Kubacki @ 2020-09-17 16:49 UTC (permalink / raw) To: devel, gaoliming; +Cc: 'Michael D Kinney', 'Zhiguang Liu' Hi Liming, I have rebased this commit and added all Reviewed-bys on: https://github.com/makubacki/edk2/commits/fix_block_io_protocol_rev3_value Please PR it when you are ready. Thanks, Michael On 9/14/2020 5:54 PM, gaoliming wrote: > Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> > >> -----邮件原件----- >> 发件人: bounce+27952+65228+4905953+8761045@groups.io >> <bounce+27952+65228+4905953+8761045@groups.io> 代表 Michael >> Kubacki >> 发送时间: 2020年9月15日 2:11 >> 收件人: devel@edk2.groups.io >> 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao >> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com> >> 主题: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct >> EFI_BLOCK_IO_PROTOCOL_REVISION3 value >> >> From: Michael Kubacki <michael.kubacki@microsoft.com> >> >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 >> >> 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 <michael.d.kinney@intel.com> >> Cc: Liming Gao <gaoliming@byosoft.com.cn> >> Cc: Zhiguang Liu <zhiguang.liu@intel.com> >> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> >> --- >> 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 >> >> >> > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* 回复: 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-17 16:49 ` Michael Kubacki @ 2020-09-18 1:08 ` gaoliming 0 siblings, 0 replies; 13+ messages in thread From: gaoliming @ 2020-09-18 1:08 UTC (permalink / raw) To: devel, michael.kubacki; +Cc: 'Michael D Kinney', 'Zhiguang Liu' Michael: Here is pull request https://github.com/tianocore/edk2/pull/936 Thanks Liming > -----邮件原件----- > 发件人: bounce+27952+65387+4905953+8761045@groups.io > <bounce+27952+65387+4905953+8761045@groups.io> 代表 Michael > Kubacki > 发送时间: 2020年9月18日 0:49 > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn > 抄送: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu' > <zhiguang.liu@intel.com> > 主题: Re: 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > EFI_BLOCK_IO_PROTOCOL_REVISION3 value > > Hi Liming, > > I have rebased this commit and added all Reviewed-bys on: > https://github.com/makubacki/edk2/commits/fix_block_io_protocol_rev3_va > lue > > Please PR it when you are ready. > > Thanks, > Michael > > On 9/14/2020 5:54 PM, gaoliming wrote: > > Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> > > > >> -----邮件原件----- > >> 发件人: bounce+27952+65228+4905953+8761045@groups.io > >> <bounce+27952+65228+4905953+8761045@groups.io> 代表 Michael > >> Kubacki > >> 发送时间: 2020年9月15日 2:11 > >> 收件人: devel@edk2.groups.io > >> 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao > >> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com> > >> 主题: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > >> EFI_BLOCK_IO_PROTOCOL_REVISION3 value > >> > >> From: Michael Kubacki <michael.kubacki@microsoft.com> > >> > >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 > >> > >> 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 <michael.d.kinney@intel.com> > >> Cc: Liming Gao <gaoliming@byosoft.com.cn> > >> Cc: Zhiguang Liu <zhiguang.liu@intel.com> > >> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > >> --- > >> 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 > >> > >> > >> > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-14 18:11 [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value Michael Kubacki 2020-09-15 0:54 ` 回复: [edk2-devel] " gaoliming @ 2020-09-15 3:27 ` Zhiguang Liu 2020-09-18 1:25 ` [edk2-devel] " Ni, Ray 2 siblings, 0 replies; 13+ messages in thread From: Zhiguang Liu @ 2020-09-15 3:27 UTC (permalink / raw) To: michael.kubacki@outlook.com, devel@edk2.groups.io Cc: Kinney, Michael D, Liming Gao Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com> > -----Original Message----- > From: michael.kubacki@outlook.com <michael.kubacki@outlook.com> > Sent: Tuesday, September 15, 2020 2:11 AM > To: devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > Subject: [PATCH v1 1/1] MdePkg: Correct > EFI_BLOCK_IO_PROTOCOL_REVISION3 value > > From: Michael Kubacki <michael.kubacki@microsoft.com> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 > > 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 <michael.d.kinney@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > --- > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-14 18:11 [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value Michael Kubacki 2020-09-15 0:54 ` 回复: [edk2-devel] " gaoliming 2020-09-15 3:27 ` Zhiguang Liu @ 2020-09-18 1:25 ` Ni, Ray 2020-09-18 18:54 ` Michael Kubacki 2 siblings, 1 reply; 13+ messages in thread From: Ni, Ray @ 2020-09-18 1:25 UTC (permalink / raw) To: devel@edk2.groups.io, michael.kubacki@outlook.com Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang 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 revision3 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 <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 <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang > <zhiguang.liu@intel.com> > Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value > > From: Michael Kubacki <michael.kubacki@microsoft.com> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 > > 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 <michael.d.kinney@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > --- > 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 > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-18 1:25 ` [edk2-devel] " Ni, Ray @ 2020-09-18 18:54 ` Michael Kubacki 2020-09-18 23:53 ` Ni, Ray 0 siblings, 1 reply; 13+ messages in thread From: Michael Kubacki @ 2020-09-18 18:54 UTC (permalink / raw) To: devel, ray.ni; +Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang Hi Ray, Rev3 adds the UINT32 field OptimalTransferLengthGranularity field to EFI_BLOCK_IO_MEDIA. A preexisting binary Block I/O producer that uses 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 field checks for >= 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 the 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 provide any further insight if needed. That said, this change was made to fix a bug in the edk2 implementation to remove a conflict with the UEFI Spec, the two should be in agreement. I suggest the change be added to the next stable tag release notes so 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 revision3 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 <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 <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang >> <zhiguang.liu@intel.com> >> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value >> >> From: Michael Kubacki <michael.kubacki@microsoft.com> >> >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 >> >> 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 <michael.d.kinney@intel.com> >> Cc: Liming Gao <gaoliming@byosoft.com.cn> >> Cc: Zhiguang Liu <zhiguang.liu@intel.com> >> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> >> --- >> 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 >> >> >> > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-18 18:54 ` Michael Kubacki @ 2020-09-18 23:53 ` Ni, Ray 2020-09-19 0:26 ` Michael Kubacki 0 siblings, 1 reply; 13+ messages in thread From: Ni, Ray @ 2020-09-18 23:53 UTC (permalink / raw) To: devel@edk2.groups.io, michael.kubacki@outlook.com Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang [-- Attachment #1: Type: text/plain, Size: 3758 bytes --] As far as I know, EFI disk utility consumes this new field for performance. The utility is in Intel website for external downloads. ________________________________ 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael Kubacki <michael.kubacki@outlook.com> 发送时间: Saturday, September 19, 2020 2:54:54 AM 收件人: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com> 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> 主题: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct 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 uses 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 field checks for >= 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 the 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 provide any further insight if needed. That said, this change was made to fix a bug in the edk2 implementation to remove a conflict with the UEFI Spec, the two should be in agreement. I suggest the change be added to the next stable tag release notes so 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 revision3 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 <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 <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang >> <zhiguang.liu@intel.com> >> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value >> >> From: Michael Kubacki <michael.kubacki@microsoft.com> >> >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 >> >> 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 <michael.d.kinney@intel.com> >> Cc: Liming Gao <gaoliming@byosoft.com.cn> >> Cc: Zhiguang Liu <zhiguang.liu@intel.com> >> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> >> --- >> 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 >> >> >> > > > > > > [-- Attachment #2: Type: text/html, Size: 5235 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-18 23:53 ` Ni, Ray @ 2020-09-19 0:26 ` Michael Kubacki 2020-09-21 2:28 ` Ni, Ray 0 siblings, 1 reply; 13+ messages in thread From: Michael Kubacki @ 2020-09-19 0:26 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang 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. > > ------------------------------------------------------------------------ > *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael > Kubacki <michael.kubacki@outlook.com> > *发送时间:* Saturday, September 19, 2020 2:54:54 AM > *收件人:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray > <ray.ni@intel.com> > *抄送:* Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > *主题:* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > 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 uses > 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 field > checks for >= 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 the > 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 provide > any further insight if needed. > > That said, this change was made to fix a bug in the edk2 implementation > to remove a conflict with the UEFI Spec, the two should be in agreement. > > I suggest the change be added to the next stable tag release notes so > 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 revision3 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 <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 <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang >>> <zhiguang.liu@intel.com> >>> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value >>> >>> From: Michael Kubacki <michael.kubacki@microsoft.com> >>> >>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 >>> >>> 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 <michael.d.kinney@intel.com> >>> Cc: Liming Gao <gaoliming@byosoft.com.cn> >>> Cc: Zhiguang Liu <zhiguang.liu@intel.com> >>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> >>> --- >>> 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 >>> >>> >>> >> >> >> >> >> >> > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-19 0:26 ` Michael Kubacki @ 2020-09-21 2:28 ` Ni, Ray 2020-09-21 20:00 ` Michael Kubacki 0 siblings, 1 reply; 13+ messages in thread From: Ni, Ray @ 2020-09-21 2:28 UTC (permalink / raw) To: Michael Kubacki, devel@edk2.groups.io Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang Disk Utility code that consumes the REVISION3 macro: if (BlkIo->Revision >= EFI_BLOCK_IO_PROTOCOL_REVISION3 && BlkIo->Media->OptimalTransferLengthGranularity != 0 ) { // // Compute the least common multiple of OptimalTransferLengthGranularity and LogicalBlocksPerPhysicalBlock // *OptimalTransferBlocks = Lcm ( BlkIo->Media->OptimalTransferLengthGranularity, *OptimalTransferBlocks ); } Even we could release a new version of tool with the correct value, there 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 <michael.kubacki@outlook.com> > Sent: Saturday, September 19, 2020 8:26 AM > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > 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. > > > > ------------------------------------------------------------------------ > > *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael > > Kubacki <michael.kubacki@outlook.com> > > *发送时间:* Saturday, September 19, 2020 2:54:54 AM > > *收件人:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray > > <ray.ni@intel.com> > > *抄送:* Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > > *主题:* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > > 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 uses > > 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 field > > checks for >= 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 the > > 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 provide > > any further insight if needed. > > > > That said, this change was made to fix a bug in the edk2 implementation > > to remove a conflict with the UEFI Spec, the two should be in agreement. > > > > I suggest the change be added to the next stable tag release notes so > > 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 revision3 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 <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 <michael.d.kinney@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Liu, Zhiguang > >>> <zhiguang.liu@intel.com> > >>> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > EFI_BLOCK_IO_PROTOCOL_REVISION3 value > >>> > >>> From: Michael Kubacki <michael.kubacki@microsoft.com> > >>> > >>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 > >>> > >>> 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 <michael.d.kinney@intel.com> > >>> Cc: Liming Gao <gaoliming@byosoft.com.cn> > >>> Cc: Zhiguang Liu <zhiguang.liu@intel.com> > >>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > >>> --- > >>> 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 > >>> > >>> > >>> > >> > >> > >> > >> > >> > >> > > > > > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-21 2:28 ` Ni, Ray @ 2020-09-21 20:00 ` Michael Kubacki 2020-09-22 1:22 ` 回复: " gaoliming 0 siblings, 1 reply; 13+ messages in thread From: Michael Kubacki @ 2020-09-21 20:00 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang I found the issue while manually reviewing some protocol structures against the UEFI spec. 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. Thanks, Michael On 9/20/2020 7:28 PM, Ni, Ray wrote: > Disk Utility code that consumes the REVISION3 macro: > if (BlkIo->Revision >= EFI_BLOCK_IO_PROTOCOL_REVISION3 && > BlkIo->Media->OptimalTransferLengthGranularity != 0 > ) { > // > // Compute the least common multiple of OptimalTransferLengthGranularity and LogicalBlocksPerPhysicalBlock > // > *OptimalTransferBlocks = Lcm ( > BlkIo->Media->OptimalTransferLengthGranularity, > *OptimalTransferBlocks > ); > } > > Even we could release a new version of tool with the correct value, there 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 <michael.kubacki@outlook.com> >> Sent: Saturday, September 19, 2020 8:26 AM >> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> >> 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. >>> >>> ------------------------------------------------------------------------ >>> *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael >>> Kubacki <michael.kubacki@outlook.com> >>> *发送时间:* Saturday, September 19, 2020 2:54:54 AM >>> *收件人:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray >>> <ray.ni@intel.com> >>> *抄送:* Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao >>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> >>> *主题:* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct >>> 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 uses >>> 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 field >>> checks for >= 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 the >>> 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 provide >>> any further insight if needed. >>> >>> That said, this change was made to fix a bug in the edk2 implementation >>> to remove a conflict with the UEFI Spec, the two should be in agreement. >>> >>> I suggest the change be added to the next stable tag release notes so >>> 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 revision3 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 <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 <michael.d.kinney@intel.com>; Liming Gao >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang >>>>> <zhiguang.liu@intel.com> >>>>> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct >> EFI_BLOCK_IO_PROTOCOL_REVISION3 value >>>>> >>>>> From: Michael Kubacki <michael.kubacki@microsoft.com> >>>>> >>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 >>>>> >>>>> 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 <michael.d.kinney@intel.com> >>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn> >>>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com> >>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> >>>>> --- >>>>> 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 >>>>> >>>>> >>>>> >>>> >>>> >>>> >>>> >>>> >>>> >>> >>> >>> >>> >>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-21 20:00 ` Michael Kubacki @ 2020-09-22 1:22 ` gaoliming 2020-09-22 21:34 ` Michael D Kinney 0 siblings, 1 reply; 13+ messages in thread From: gaoliming @ 2020-09-22 1:22 UTC (permalink / raw) To: 'Michael Kubacki', 'Ni, Ray', devel Cc: 'Kinney, Michael D', 'Liu, Zhiguang' Michael and Ray: So far, I don't see the big impact with this change. I search Edk2/Edk2Platform code base. I only find MdeModulePkg ScsiDiskDxe installs BlkIo with revision 3, and PartitionDxe checks BlkIo revision. They are in the same package, and are used together. So, there is no impact for them. Now, the known impact is Disk Utility. The old binary Disk Utility may not work with the bios image built from the latest edk2 on ScsiDisk. I would suggest to clarify this impact in the release note of the stable tag 202011. If more impact is reported later, we can consider the solution again. Thanks Liming > -----邮件原件----- > 发件人: Michael Kubacki <michael.kubacki@outlook.com> > 发送时间: 2020年9月22日 4:00 > 收件人: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > 主题: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > EFI_BLOCK_IO_PROTOCOL_REVISION3 value > > I found the issue while manually reviewing some protocol structures > against the UEFI spec. > > 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. > > Thanks, > Michael > > On 9/20/2020 7:28 PM, Ni, Ray wrote: > > Disk Utility code that consumes the REVISION3 macro: > > if (BlkIo->Revision >= EFI_BLOCK_IO_PROTOCOL_REVISION3 && > > BlkIo->Media->OptimalTransferLengthGranularity != 0 > > ) { > > // > > // Compute the least common multiple of > OptimalTransferLengthGranularity and LogicalBlocksPerPhysicalBlock > > // > > *OptimalTransferBlocks = Lcm ( > > BlkIo->Media->OptimalTransferLengthGranularity, > > *OptimalTransferBlocks > > ); > > } > > > > Even we could release a new version of tool with the correct value, there 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 <michael.kubacki@outlook.com> > >> Sent: Saturday, September 19, 2020 8:26 AM > >> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > >> 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. > >>> > >>> ------------------------------------------------------------------------ > >>> *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> 代表 > Michael > >>> Kubacki <michael.kubacki@outlook.com> > >>> *发送时间:* Saturday, September 19, 2020 2:54:54 AM > >>> *收件人:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray > >>> <ray.ni@intel.com> > >>> *抄送:* Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > >>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > >>> *主题:* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > >>> 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 uses > >>> 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 field > >>> checks for >= 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 the > >>> 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 provide > >>> any further insight if needed. > >>> > >>> That said, this change was made to fix a bug in the edk2 implementation > >>> to remove a conflict with the UEFI Spec, the two should be in agreement. > >>> > >>> I suggest the change be added to the next stable tag release notes so > >>> 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 revision3 > 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 <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 <michael.d.kinney@intel.com>; Liming Gao > >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang > >>>>> <zhiguang.liu@intel.com> > >>>>> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > >> EFI_BLOCK_IO_PROTOCOL_REVISION3 value > >>>>> > >>>>> From: Michael Kubacki <michael.kubacki@microsoft.com> > >>>>> > >>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 > >>>>> > >>>>> 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 <michael.d.kinney@intel.com> > >>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn> > >>>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com> > >>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > >>>>> --- > >>>>> 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 > >>>>> > >>>>> > >>>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>> > >>> > >>> > >>> > >>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value 2020-09-22 1:22 ` 回复: " gaoliming @ 2020-09-22 21:34 ` Michael D Kinney 0 siblings, 0 replies; 13+ messages in thread From: Michael D Kinney @ 2020-09-22 21:34 UTC (permalink / raw) To: devel@edk2.groups.io, gaoliming@byosoft.com.cn, 'Michael Kubacki', Ni, Ray, Kinney, Michael D Cc: Liu, Zhiguang Liming, I agree with this approach. Changing the UEFI spec does not seem like a good option because there can be other implementations of the UEFI Spec or portions of it. Fixing the EDK II implementation and working with the community and all downstream consumers should be the focus to make sure everyone is informed of this change and its potential impact. Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming > Sent: Monday, September 21, 2020 6:23 PM > To: 'Michael Kubacki' <michael.kubacki@outlook.com>; Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com> > Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value > > Michael and Ray: > So far, I don't see the big impact with this change. I search Edk2/Edk2Platform code base. I only find MdeModulePkg ScsiDiskDxe > installs BlkIo with revision 3, and PartitionDxe checks BlkIo revision. They are in the same package, and are used together. So, > there is no impact for them. Now, the known impact is Disk Utility. The old binary Disk Utility may not work with the bios image > built from the latest edk2 on ScsiDisk. I would suggest to clarify this impact in the release note of the stable tag 202011. If more > impact is reported later, we can consider the solution again. > > Thanks > Liming > > -----邮件原件----- > > 发件人: Michael Kubacki <michael.kubacki@outlook.com> > > 发送时间: 2020年9月22日 4:00 > > 收件人: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > > 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > > 主题: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > > EFI_BLOCK_IO_PROTOCOL_REVISION3 value > > > > I found the issue while manually reviewing some protocol structures > > against the UEFI spec. > > > > 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. > > > > Thanks, > > Michael > > > > On 9/20/2020 7:28 PM, Ni, Ray wrote: > > > Disk Utility code that consumes the REVISION3 macro: > > > if (BlkIo->Revision >= EFI_BLOCK_IO_PROTOCOL_REVISION3 && > > > BlkIo->Media->OptimalTransferLengthGranularity != 0 > > > ) { > > > // > > > // Compute the least common multiple of > > OptimalTransferLengthGranularity and LogicalBlocksPerPhysicalBlock > > > // > > > *OptimalTransferBlocks = Lcm ( > > > BlkIo->Media->OptimalTransferLengthGranularity, > > > *OptimalTransferBlocks > > > ); > > > } > > > > > > Even we could release a new version of tool with the correct value, there 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 <michael.kubacki@outlook.com> > > >> Sent: Saturday, September 19, 2020 8:26 AM > > >> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > > >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > > >> 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. > > >>> > > >>> ------------------------------------------------------------------------ > > >>> *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> 代表 > > Michael > > >>> Kubacki <michael.kubacki@outlook.com> > > >>> *发送时间:* Saturday, September 19, 2020 2:54:54 AM > > >>> *收件人:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray > > >>> <ray.ni@intel.com> > > >>> *抄送:* Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao > > >>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> > > >>> *主题:* Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > > >>> 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 uses > > >>> 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 field > > >>> checks for >= 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 the > > >>> 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 provide > > >>> any further insight if needed. > > >>> > > >>> That said, this change was made to fix a bug in the edk2 implementation > > >>> to remove a conflict with the UEFI Spec, the two should be in agreement. > > >>> > > >>> I suggest the change be added to the next stable tag release notes so > > >>> 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 revision3 > > 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 <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 <michael.d.kinney@intel.com>; Liming Gao > > >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang > > >>>>> <zhiguang.liu@intel.com> > > >>>>> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct > > >> EFI_BLOCK_IO_PROTOCOL_REVISION3 value > > >>>>> > > >>>>> From: Michael Kubacki <michael.kubacki@microsoft.com> > > >>>>> > > >>>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2961 > > >>>>> > > >>>>> 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 <michael.d.kinney@intel.com> > > >>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn> > > >>>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com> > > >>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > > >>>>> --- > > >>>>> 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 > > >>>>> > > >>>>> > > >>>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>> > > >>> > > >>> > > >>> > > >>> > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-09-22 21:34 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-14 18:11 [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value Michael Kubacki 2020-09-15 0:54 ` 回复: [edk2-devel] " gaoliming 2020-09-17 16:49 ` Michael Kubacki 2020-09-18 1:08 ` 回复: " gaoliming 2020-09-15 3:27 ` Zhiguang Liu 2020-09-18 1:25 ` [edk2-devel] " Ni, Ray 2020-09-18 18:54 ` Michael Kubacki 2020-09-18 23:53 ` Ni, Ray 2020-09-19 0:26 ` Michael Kubacki 2020-09-21 2:28 ` Ni, Ray 2020-09-21 20:00 ` Michael Kubacki 2020-09-22 1:22 ` 回复: " gaoliming 2020-09-22 21:34 ` Michael D Kinney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox