From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C475D1A1ED1 for ; Wed, 7 Sep 2016 15:59:59 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP; 07 Sep 2016 15:59:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,297,1470726000"; d="scan'208";a="165601070" Received: from jjloucai-mobl1.amr.corp.intel.com (HELO localhost) ([10.252.131.171]) by fmsmga004.fm.intel.com with ESMTP; 07 Sep 2016 15:59:59 -0700 MIME-Version: 1.0 To: Laszlo Ersek , "edk2-devel-01" Message-ID: <147328919894.15355.2865979254185805527@jljusten-ivb> From: Jordan Justen In-Reply-To: <20160907114243.16705-1-lersek@redhat.com> References: <20160907114243.16705-1-lersek@redhat.com> User-Agent: alot/0.3.7 Date: Wed, 07 Sep 2016 15:59:58 -0700 Subject: Re: [PATCH] OvmfPkg/QemuBootOrderLib: drop too strict "/HD(" suffix from vblk prefix X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Sep 2016 23:00:00 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jordan Justen 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 > ). > 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 > Fixes: e06a4cd134064590aa1a855ff4b973023279e805 > Ref: https://bugzilla.redhat.com/show_bug.cgi?id=3D1373812 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek > --- > = > Notes: > Public branch: > > = > OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > = > diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPk= g/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 non= zero > // > Written =3D 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 >=3D FirstNonBridge + 3 && > SubstringEq (OfwNode[FirstNonBridge + 0].DriverName, "scsi"= ) && > SubstringEq (OfwNode[FirstNonBridge + 1].DriverName, "chann= el") && > 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: > // > - // /HD( > + // > // > Written =3D UnicodeSPrintAsciiFormat ( > Translated, > *TranslatedSize * sizeof (*Translated), // BufferSize in= bytes > - "%s/HD(", > + "%s", > VenHwString > ); > } else if (NumNodes >=3D 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 >=20