From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5C3FE1A1F52 for ; Thu, 8 Sep 2016 01:06:33 -0700 (PDT) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B9F637F7A9; Thu, 8 Sep 2016 08:06:32 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-57.phx2.redhat.com [10.3.116.57]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8886Vjq028840; Thu, 8 Sep 2016 04:06:31 -0400 To: Jordan Justen References: <20160907114243.16705-1-lersek@redhat.com> <147328919894.15355.2865979254185805527@jljusten-ivb> Cc: edk2-devel-01 From: Laszlo Ersek Message-ID: Date: Thu, 8 Sep 2016 10:06:30 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <147328919894.15355.2865979254185805527@jljusten-ivb> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 08 Sep 2016 08:06:32 +0000 (UTC) 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: Thu, 08 Sep 2016 08:06:33 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 09/08/16 00:59, Jordan Justen wrote: > Reviewed-by: Jordan Justen 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 >> ). >> 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=1373812 >> 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/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: >> // >> - // /HD( >> + // >> // >> 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 >