public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ray" <ray.ni@intel.com>
To: Michael Kubacki <michael.kubacki@outlook.com>,
	"devel@edk2.groups.io" <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
Date: Mon, 21 Sep 2020 02:28:13 +0000	[thread overview]
Message-ID: <BY5PR11MB4007ED43BBC2B40A2343AF358C3A0@BY5PR11MB4007.namprd11.prod.outlook.com> (raw)
In-Reply-To: <MWHPR07MB34405AE7DEEBB083EE4B3208E93C0@MWHPR07MB3440.namprd07.prod.outlook.com>

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

  reply	other threads:[~2020-09-21  2:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2020-09-21 20:00           ` Michael Kubacki
2020-09-22  1:22             ` 回复: " gaoliming
2020-09-22 21:34               ` Michael D Kinney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BY5PR11MB4007ED43BBC2B40A2343AF358C3A0@BY5PR11MB4007.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox