[PATCH] OvmfPkg/QemuBootOrderLib: drop too strict "/HD(" suffix from vblk prefix

* [PATCH] OvmfPkg/QemuBootOrderLib: drop too strict "/HD(" suffix from vblk prefix
@ 2016-09-07 11:42 Laszlo Ersek
  2016-09-07 22:59 ` Jordan Justen
  0 siblings, 1 reply; 3+ messages in thread
From: Laszlo Ersek @ 2016-09-07 11:42 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Jordan Justen

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



^ permalink raw reply related	[flat|nested] 3+ messages in thread