| Message ID | 20160907114243.16705-1-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 09/08/16 00:59, Jordan Justen wrote: > Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> Thank you, pushed as commit d796d33f1844. Laszlo > On 2016-09-07 04:42:43, Laszlo Ersek wrote: >> Translating QEMU's virtio-block OpenFirmware device path to a UEFI device >> path prefix was one of the earliest case handled in QemuBootOrderLib. At >> that time, I terminated the translation output (the UEFI devpath prefix) >> with a "/HD(" suffix. >> >> The intent was for the translation to prefix-match only boot options with >> HD() device path nodes in them, that is, no auto-generated "device level" >> boot options. This was motivated by prioritizing specific boot options >> created by OS installers over auto-generated "device level" options. >> >> However, practice has shown that: >> >> - OS installers place their installed boot options first in the boot order >> anyway, >> >> - other device types (SATA disks, virtio-scsi disks), where "/HD(" is not >> appended, work just fine, >> >> - requiring "/HD(" actually causes problems: after the OS-installed >> specific boot option has been lost (or purposely removed), the >> auto-generated "device level" boot option does the right thing (see the >> Default Boot Behavior under >> <http://blog.uncooperative.org/blog/2014/02/06/the-efi-system-partition/>). >> The "/HD(" requirement causes such boot options to be dropped, which >> prevents "fallback.efi" from running. >> >> Relax the matching by removing the "/HD(" suffix from the translated >> prefix. >> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Fixes: e06a4cd134064590aa1a855ff4b973023279e805 >> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1373812 >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> >> Notes: >> Public branch: >> <https://github.com/lersek/edk2/commits/drop_hdprefix_vblk_v1> >> >> OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c >> index 86082301a8f5..8cbbdb0568fa 100644 >> --- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c >> +++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c >> @@ -864,29 +864,29 @@ TranslatePciOfwNodes ( >> // OpenFirmware device path (virtio-blk disk): >> // >> // /pci@i0cf8/scsi@6[,3]/disk@0,0 >> // ^ ^ ^ ^ ^ >> // | | | fixed >> // | | PCI function corresponding to disk (optional) >> // | PCI slot holding disk >> // PCI root at system bus port, PIO >> // >> // UEFI device path prefix: >> // >> - // PciRoot(0x0)/Pci(0x6,0x0)/HD( -- if PCI function is 0 or absent >> - // PciRoot(0x0)/Pci(0x6,0x3)/HD( -- if PCI function is present and nonzero >> + // PciRoot(0x0)/Pci(0x6,0x0) -- if PCI function is 0 or absent >> + // PciRoot(0x0)/Pci(0x6,0x3) -- if PCI function is present and nonzero >> // >> Written = UnicodeSPrintAsciiFormat ( >> Translated, >> *TranslatedSize * sizeof (*Translated), // BufferSize in bytes >> - "PciRoot(0x%x)%s/Pci(0x%Lx,0x%Lx)/HD(", >> + "PciRoot(0x%x)%s/Pci(0x%Lx,0x%Lx)", >> PciRoot, >> Bridges, >> PciDevFun[0], >> PciDevFun[1] >> ); >> } else if (NumNodes >= FirstNonBridge + 3 && >> SubstringEq (OfwNode[FirstNonBridge + 0].DriverName, "scsi") && >> SubstringEq (OfwNode[FirstNonBridge + 1].DriverName, "channel") && >> SubstringEq (OfwNode[FirstNonBridge + 2].DriverName, "disk") >> ) { >> // >> @@ -1109,28 +1109,28 @@ TranslateMmioOfwNodes ( >> SubstringEq (OfwNode[1].DriverName, "disk")) { >> // >> // OpenFirmware device path (virtio-blk disk): >> // >> // /virtio-mmio@000000000a003c00/disk@0,0 >> // ^ ^ ^ >> // | fixed >> // base address of virtio-mmio register block >> // >> // UEFI device path prefix: >> // >> - // <VenHwString>/HD( >> + // <VenHwString> >> // >> Written = UnicodeSPrintAsciiFormat ( >> Translated, >> *TranslatedSize * sizeof (*Translated), // BufferSize in bytes >> - "%s/HD(", >> + "%s", >> VenHwString >> ); >> } else if (NumNodes >= 3 && >> SubstringEq (OfwNode[1].DriverName, "channel") && >> SubstringEq (OfwNode[2].DriverName, "disk")) { >> // >> // OpenFirmware device path (virtio-scsi disk): >> // >> // /virtio-mmio@000000000a003a00/channel@0/disk@2,3 >> // ^ ^ ^ ^ >> // | | | LUN >> -- >> 2.9.2 >> > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c index 86082301a8f5..8cbbdb0568fa 100644 --- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c +++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c @@ -864,29 +864,29 @@ TranslatePciOfwNodes ( // OpenFirmware device path (virtio-blk disk): // // /pci@i0cf8/scsi@6[,3]/disk@0,0 // ^ ^ ^ ^ ^ // | | | fixed // | | PCI function corresponding to disk (optional) // | PCI slot holding disk // PCI root at system bus port, PIO // // UEFI device path prefix: // - // PciRoot(0x0)/Pci(0x6,0x0)/HD( -- if PCI function is 0 or absent - // PciRoot(0x0)/Pci(0x6,0x3)/HD( -- if PCI function is present and nonzero + // PciRoot(0x0)/Pci(0x6,0x0) -- if PCI function is 0 or absent + // PciRoot(0x0)/Pci(0x6,0x3) -- if PCI function is present and nonzero // Written = UnicodeSPrintAsciiFormat ( Translated, *TranslatedSize * sizeof (*Translated), // BufferSize in bytes - "PciRoot(0x%x)%s/Pci(0x%Lx,0x%Lx)/HD(", + "PciRoot(0x%x)%s/Pci(0x%Lx,0x%Lx)", PciRoot, Bridges, PciDevFun[0], PciDevFun[1] ); } else if (NumNodes >= FirstNonBridge + 3 && SubstringEq (OfwNode[FirstNonBridge + 0].DriverName, "scsi") && SubstringEq (OfwNode[FirstNonBridge + 1].DriverName, "channel") && SubstringEq (OfwNode[FirstNonBridge + 2].DriverName, "disk") ) { // @@ -1109,28 +1109,28 @@ TranslateMmioOfwNodes ( SubstringEq (OfwNode[1].DriverName, "disk")) { // // OpenFirmware device path (virtio-blk disk): // // /virtio-mmio@000000000a003c00/disk@0,0 // ^ ^ ^ // | fixed // base address of virtio-mmio register block // // UEFI device path prefix: // - // <VenHwString>/HD( + // <VenHwString> // Written = UnicodeSPrintAsciiFormat ( Translated, *TranslatedSize * sizeof (*Translated), // BufferSize in bytes - "%s/HD(", + "%s", VenHwString ); } else if (NumNodes >= 3 && SubstringEq (OfwNode[1].DriverName, "channel") && SubstringEq (OfwNode[2].DriverName, "disk")) { // // OpenFirmware device path (virtio-scsi disk): // // /virtio-mmio@000000000a003a00/channel@0/disk@2,3 // ^ ^ ^ ^ // | | | LUN
Translating QEMU's virtio-block OpenFirmware device path to a UEFI device path prefix was one of the earliest case handled in QemuBootOrderLib. At that time, I terminated the translation output (the UEFI devpath prefix) with a "/HD(" suffix. The intent was for the translation to prefix-match only boot options with HD() device path nodes in them, that is, no auto-generated "device level" boot options. This was motivated by prioritizing specific boot options created by OS installers over auto-generated "device level" options. However, practice has shown that: - OS installers place their installed boot options first in the boot order anyway, - other device types (SATA disks, virtio-scsi disks), where "/HD(" is not appended, work just fine, - requiring "/HD(" actually causes problems: after the OS-installed specific boot option has been lost (or purposely removed), the auto-generated "device level" boot option does the right thing (see the Default Boot Behavior under <http://blog.uncooperative.org/blog/2014/02/06/the-efi-system-partition/>). The "/HD(" requirement causes such boot options to be dropped, which prevents "fallback.efi" from running. Relax the matching by removing the "/HD(" suffix from the translated prefix. Cc: Jordan Justen <jordan.l.justen@intel.com> Fixes: e06a4cd134064590aa1a855ff4b973023279e805 Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1373812 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: Public branch: <https://github.com/lersek/edk2/commits/drop_hdprefix_vblk_v1> OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.9.2 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel