Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correct EFI_BLOCK_IO_PROTOCOL_REVISION3 value

["Michael Kubacki" <michael.kubacki@outlook.com>]

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
>>>
>>>
>>>
>> 
>> 
>> 
>> 
>> 
>> 
> 
> 
> 
> 
>