* [PATCH 0/2] OvmfPkg/QemuBootOrderLib: multiplicity update for OFW <-> UEFI matching
@ 2017-11-27 19:03 Laszlo Ersek
2017-11-27 19:03 ` [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts Laszlo Ersek
2017-11-27 19:03 ` [PATCH 2/2] OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple " Laszlo Ersek
0 siblings, 2 replies; 6+ messages in thread
From: Laszlo Ersek @ 2017-11-27 19:03 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
Two small patches with verbose commit messages.
Repo: https://github.com/lersek/edk2.git
Branch: qemu_boot_order_multimatch
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Laszlo Ersek (2):
OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot
opts
OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple UEFI boot
opts
OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts
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
2017-11-28 7:55 ` Ard Biesheuvel
2017-11-27 19:03 ` [PATCH 2/2] OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple " Laszlo Ersek
1 sibling, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2017-11-27 19:03 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple UEFI boot opts
2017-11-27 19:03 [PATCH 0/2] OvmfPkg/QemuBootOrderLib: multiplicity update for OFW <-> UEFI matching Laszlo Ersek
2017-11-27 19:03 ` [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts Laszlo Ersek
@ 2017-11-27 19:03 ` Laszlo Ersek
2017-11-28 7:56 ` Ard Biesheuvel
1 sibling, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2017-11-27 19:03 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
This means that SetBootOrderFromQemu() will preserve all UEFI boot options
matched by any given OFW devpath, such as PXEv4, HTTPv4, PXEv6 and HTTPv6
boot options for the same NIC. Currently we stop the matching / appending
for the OFW devpath coming from the outer loop whenever we find the first
UEFI boot option match in the inner loop.
(The previous patch was about multiple OFW devpaths matching a single UEFI
boot option (which should never happen). This patch is about a single OFW
devpath matching multiple UEFI boot options. With the "break" statement
removed here, the small optimization from the last patch becomes a bit
more relevant, because now the inner loop always counts up to
ActiveCount.)
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 | 1 -
1 file changed, 1 deletion(-)
diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index a9a62e9d4007..366104adf535 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1863,31 +1863,30 @@ SetBootOrderFromQemu (
for (Idx = 0; Idx < ActiveCount; ++Idx) {
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
} // translation successful
TranslatedSize = ARRAY_SIZE (Translated);
Status = TranslateOfwPath (&FwCfgPtr, ExtraPciRoots, Translated,
&TranslatedSize);
} // scanning of OpenFirmware paths done
if (Status == RETURN_NOT_FOUND && BootOrder.Produced > 0) {
//
// No more OpenFirmware paths, some matches found: rewrite BootOrder NvVar.
// Some of the active boot options that have not been selected over fw_cfg
// should be preserved at the end of the boot order.
//
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts
2017-11-27 19:03 ` [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts Laszlo Ersek
@ 2017-11-28 7:55 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2017-11-28 7:55 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen
On 27 November 2017 at 19:03, Laszlo Ersek <lersek@redhat.com> wrote:
> 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>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> 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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple UEFI boot opts
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
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2017-11-28 7:56 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen
On 27 November 2017 at 19:03, Laszlo Ersek <lersek@redhat.com> wrote:
> This means that SetBootOrderFromQemu() will preserve all UEFI boot options
> matched by any given OFW devpath, such as PXEv4, HTTPv4, PXEv6 and HTTPv6
> boot options for the same NIC. Currently we stop the matching / appending
> for the OFW devpath coming from the outer loop whenever we find the first
> UEFI boot option match in the inner loop.
>
> (The previous patch was about multiple OFW devpaths matching a single UEFI
> boot option (which should never happen). This patch is about a single OFW
> devpath matching multiple UEFI boot options. With the "break" statement
> removed here, the small optimization from the last patch becomes a bit
> more relevant, because now the inner loop always counts up to
> ActiveCount.)
>
> 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>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> index a9a62e9d4007..366104adf535 100644
> --- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> +++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> @@ -1863,31 +1863,30 @@ SetBootOrderFromQemu (
> for (Idx = 0; Idx < ActiveCount; ++Idx) {
> 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
> } // translation successful
>
> TranslatedSize = ARRAY_SIZE (Translated);
> Status = TranslateOfwPath (&FwCfgPtr, ExtraPciRoots, Translated,
> &TranslatedSize);
> } // scanning of OpenFirmware paths done
>
> if (Status == RETURN_NOT_FOUND && BootOrder.Produced > 0) {
> //
> // No more OpenFirmware paths, some matches found: rewrite BootOrder NvVar.
> // Some of the active boot options that have not been selected over fw_cfg
> // should be preserved at the end of the boot order.
> //
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple UEFI boot opts
2017-11-28 7:56 ` Ard Biesheuvel
@ 2017-11-28 20:39 ` Laszlo Ersek
0 siblings, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2017-11-28 20:39 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Jordan Justen, edk2-devel-01
On 11/28/17 08:56, Ard Biesheuvel wrote:
> On 27 November 2017 at 19:03, Laszlo Ersek <lersek@redhat.com> wrote:
>> This means that SetBootOrderFromQemu() will preserve all UEFI boot options
>> matched by any given OFW devpath, such as PXEv4, HTTPv4, PXEv6 and HTTPv6
>> boot options for the same NIC. Currently we stop the matching / appending
>> for the OFW devpath coming from the outer loop whenever we find the first
>> UEFI boot option match in the inner loop.
>>
>> (The previous patch was about multiple OFW devpaths matching a single UEFI
>> boot option (which should never happen). This patch is about a single OFW
>> devpath matching multiple UEFI boot options. With the "break" statement
>> removed here, the small optimization from the last patch becomes a bit
>> more relevant, because now the inner loop always counts up to
>> ActiveCount.)
>>
>> 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>
>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Series pushed as commit range f9bc2f876326..dc32e820f028.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-28 20:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-27 19:03 [PATCH 0/2] OvmfPkg/QemuBootOrderLib: multiplicity update for OFW <-> UEFI matching Laszlo Ersek
2017-11-27 19:03 ` [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts Laszlo Ersek
2017-11-28 7:55 ` 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox