public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Jordan Justen <jordan.l.justen@intel.com>
Subject: [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts
Date: Mon, 27 Nov 2017 20:03:53 +0100	[thread overview]
Message-ID: <20171127190354.13699-2-lersek@redhat.com> (raw)
In-Reply-To: <20171127190354.13699-1-lersek@redhat.com>

The SetBootOrderFromQemu() function implements a nested loop where

- the outer loop iterates over all OpenFirmware (OFW) device paths in the
  QEMU boot order, and translates each to a UEFI device path prefix;

- the inner loop matches the current (translated) prefix against all
  active UEFI boot options in turn;

- if the UEFI boot option is matched by the translated prefix, the UEFI
  boot option is appended to the "new" UEFI boot order, and marked as
  "has been appended".

This patch adds a micro-optimization where already matched / appended UEFI
boot options are skipped in the inner loop. This is not a functional
change. A functional change would be if, as a consequence of the patch,
some UEFI boot options would no longer be *doubly* matched.

For a UEFI boot option to be matched by two translated prefixes, one of
those prefixes would have to be a (proper, or equal) prefix of the other
prefix. The PCI and MMIO OFW translation routines output such only in the
following cases:

- When the original OFW device paths are prefixes of each other. This is
  not possible from the QEMU side. (Only leaf devices are bootable.)

- When the translation rules in the routines are incomplete, and don't
  look at the OFW device paths for sufficient length (i.e., at nodes where
  they would already differ, and the difference would show up in the
  translation output).

  This would be a shortcoming of the translation routines and should be
  fixed in TranslatePciOfwNodes() and TranslateMmioOfwNodes(), whenever
  identified.

Even in the second case, this patch would replace the double appending of
a single UEFI boot option (matched by two different OFW device paths) with
a correct, or cross-, matching of two different UEFI boot options. Again,
this is not expected, but arguably it would be more correct than duplicate
boot option appending, should it occur due to any (unexpected, unknown)
lack of detail in the translation routines.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index 7c1f375beb20..a9a62e9d4007 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1849,31 +1849,32 @@ SetBootOrderFromQemu (
   //
   TranslatedSize = ARRAY_SIZE (Translated);
   Status = TranslateOfwPath (&FwCfgPtr, ExtraPciRoots, Translated,
              &TranslatedSize);
   while (Status == RETURN_SUCCESS ||
          Status == RETURN_UNSUPPORTED ||
          Status == RETURN_PROTOCOL_ERROR ||
          Status == RETURN_BUFFER_TOO_SMALL) {
     if (Status == RETURN_SUCCESS) {
       UINTN Idx;
 
       //
       // match translated OpenFirmware path against all active boot options
       //
       for (Idx = 0; Idx < ActiveCount; ++Idx) {
-        if (Match (
+        if (!ActiveOption[Idx].Appended &&
+            Match (
               Translated,
               TranslatedSize, // contains length, not size, in CHAR16's here
               ActiveOption[Idx].BootOption->FilePath
               )
             ) {
           //
           // match found, store ID and continue with next OpenFirmware path
           //
           Status = BootOrderAppend (&BootOrder, &ActiveOption[Idx]);
           if (Status != RETURN_SUCCESS) {
             goto ErrorFreeExtraPciRoots;
           }
           break;
         }
       } // scanned all active boot options
-- 
2.14.1.3.gb7cf6e02401b




  reply	other threads:[~2017-11-27 18:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-27 19:03 [PATCH 0/2] OvmfPkg/QemuBootOrderLib: multiplicity update for OFW <-> UEFI matching Laszlo Ersek
2017-11-27 19:03 ` Laszlo Ersek [this message]
2017-11-28  7:55   ` [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts Ard Biesheuvel
2017-11-27 19:03 ` [PATCH 2/2] OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple " Laszlo Ersek
2017-11-28  7:56   ` Ard Biesheuvel
2017-11-28 20:39     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171127190354.13699-2-lersek@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox