Hi, Hao Besides Laszlo's comment, I would suggest you update the commit log title to highlight the fact that this is an "incompatible" semantic change and the consumer of SATA device path or ATA_PASS_THRU would have to re-examine its usage to follow UEFI 2.5 mantis 1353 and 1472.
Thanks Feng -----Original Message----- From: Wu, Hao A Sent: Tuesday, May 10, 2016 9:37 AM To: Laszlo Ersek <[email protected]>; [email protected]; Tian, Feng <[email protected]>; Justen, Jordan L <[email protected]> Subject: RE: [edk2] [PATCH 0/4] ATA PassThru & SATA device path PortMultiplier update > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Monday, May 09, 2016 7:18 PM > To: Wu, Hao A; [email protected]; Tian, Feng; Justen, Jordan L > Subject: Re: [edk2] [PATCH 0/4] ATA PassThru & SATA device path > PortMultiplier update > > Hello Hao, > > On 05/09/16 05:17, Hao Wu wrote: > > The UEFI 2.6 spec updated the description of the port multiplier > > port number parameter in SATA Device Path Node and ATA Pass-Through > > Protocol. > > (1) > > It seems to me that this spec update was part of release 2.5, not 2.6; > namely for Mantis ticket 1353: > <https://mantis.uefi.org/mantis/view.php?id=1353>. > > The changelog at the beginning of the spec states: > > Revision Revision History (numbers = Mantis ticket numbers) Date > -------- -------------------------------------------------- ----------- > 2.5 1353 SATA Device Path Node Errata April, 2015 > > Can you update the spec version in the first three patches? (Maybe > reference the mantis ticket as well.) Yes. Thanks for catching that. I will update the log of the first three commits. > > (2) > > This patch series breaks the following two library instances: > > OvmfPkg/Library/QemuBootOrderLib > OvmfPkg/Library/QemuNewBootOrderLib > > Namely, on the Q35 machine type of QEMU, there is no port multiplier, > hence the middle number (the Port Multiplier Port Number) in the > Sata() device path node changes from 0x0 to 0xFFFF. > > For Qemu[New]BootOrderLib, this is not hard to fix. I will post two > additional patches, appended to your series, that should be please > reviewed (by Jordan) and committed (by you) together with the rest of > your patches. If you have to submit a v2 of the series, please don't > forget to preserve my patches as well. > > Please confirm that you can pick up my patches with "git am", from the > list / your inbox. Yes, I can apply the patches. I will add those two commits in my v2 series. > > (3) > > This change will also break boot options for preexistent OVMF virtual > machines that use the Sata() device path node (i.e., non-short-form > SATA boot options). This is independent of point (2) above: the > matching in question is performed by the boot manager. > > I don't have a good idea how to deal with this; we probably can't, and > users will have to update their boot options manually. Agree. Best Regards, Hao Wu > > Thanks > Laszlo > > > > > > Now, this parameter should be set to 0xFFFF instead of 0 to indicate > > that an ATA device is directly attached on the controller port. > > > > > > Hao Wu (4): > > MdePkg Protocol/DevicePath.h: Update SATA Device Path comments > > MdePkg Protocol/AtaPassThru.h: Update PortMultiplierPort related > > comments > > MdeModulePkg AtaAtapiPassThru: Use the new PortMultiplierPort > > semantics > > MdeModulePkg AtaAtapiPassThru: Fix incorrect parameter description > > comment > > > > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 14 ++-- > > .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 75 > +++++++++++++++++----- > > .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 12 ++-- > > MdePkg/Include/Protocol/AtaPassThru.h | 8 +-- > > MdePkg/Include/Protocol/DevicePath.h | 4 +- > > 5 files changed, 78 insertions(+), 35 deletions(-) > > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
