From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, ard.biesheuvel@linaro.org
Subject: Re: [edk2-devel] [PATCH 11/13] OvmfPkg/PlatformBootManagerLib: switch to QemuLoadImageLib
Date: Tue, 3 Mar 2020 10:52:12 +0100 [thread overview]
Message-ID: <f832ad31-0724-ebb7-1a55-e96c797454c1@redhat.com> (raw)
In-Reply-To: <20200302072936.29221-12-ard.biesheuvel@linaro.org>
On 03/02/20 08:29, Ard Biesheuvel wrote:
> Replace the open coded sequence to load Linux on x86 with a short and
> generic sequence invoking QemuLoadImageLib, which can be provided by
> a generic version that only supports the LoadImage and StartImage boot
> services, and one that incorporates the entire legacy loading sequence
> as well.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 +-
> OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c | 157 +++-----------------
> 2 files changed, 24 insertions(+), 135 deletions(-)
>
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index f89cce187942..40ac5dd7f9d5 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -48,7 +48,7 @@ [LibraryClasses]
> NvVarsFileLib
> QemuFwCfgLib
> QemuFwCfgS3Lib
> - LoadLinuxLib
> + QemuLoadImageLib
> QemuBootOrderLib
> ReportStatusCodeLib
> UefiLib
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
> index ddfef925edd3..a15b48d360d2 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
> @@ -9,11 +9,8 @@
>
> #include <Library/BaseLib.h>
> #include <Library/DebugLib.h>
> -#include <Library/LoadLinuxLib.h>
> -#include <Library/MemoryAllocationLib.h>
> -#include <Library/QemuFwCfgLib.h>
> +#include <Library/QemuLoadImageLib.h>
> #include <Library/ReportStatusCodeLib.h>
> -#include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
>
>
> @@ -23,146 +20,38 @@ TryRunningQemuKernel (
> )
> {
> EFI_STATUS Status;
> - UINTN KernelSize;
> - UINTN KernelInitialSize;
> - VOID *KernelBuf;
> - UINTN SetupSize;
> - VOID *SetupBuf;
> - UINTN CommandLineSize;
> - CHAR8 *CommandLine;
> - UINTN InitrdSize;
> - VOID* InitrdData;
> -
> - SetupBuf = NULL;
> - SetupSize = 0;
> - KernelBuf = NULL;
> - KernelInitialSize = 0;
> - CommandLine = NULL;
> - CommandLineSize = 0;
> - InitrdData = NULL;
> - InitrdSize = 0;
> -
> - if (!QemuFwCfgIsAvailable ()) {
> - return EFI_NOT_FOUND;
> - }
> -
> - QemuFwCfgSelectItem (QemuFwCfgItemKernelSize);
> - KernelSize = (UINTN) QemuFwCfgRead64 ();
> -
> - QemuFwCfgSelectItem (QemuFwCfgItemKernelSetupSize);
> - SetupSize = (UINTN) QemuFwCfgRead64 ();
> -
> - if (KernelSize == 0 || SetupSize == 0) {
> - DEBUG ((EFI_D_INFO, "qemu -kernel was not used.\n"));
> - return EFI_NOT_FOUND;
> - }
> -
> - SetupBuf = LoadLinuxAllocateKernelSetupPages (EFI_SIZE_TO_PAGES (SetupSize));
> - if (SetupBuf == NULL) {
> - DEBUG ((EFI_D_ERROR, "Unable to allocate memory for kernel setup!\n"));
> - return EFI_OUT_OF_RESOURCES;
> - }
> -
> - DEBUG ((EFI_D_INFO, "Setup size: 0x%x\n", (UINT32) SetupSize));
> - DEBUG ((EFI_D_INFO, "Reading kernel setup image ..."));
> - QemuFwCfgSelectItem (QemuFwCfgItemKernelSetupData);
> - QemuFwCfgReadBytes (SetupSize, SetupBuf);
> - DEBUG ((EFI_D_INFO, " [done]\n"));
> -
> - Status = LoadLinuxCheckKernelSetup (SetupBuf, SetupSize);
> - if (EFI_ERROR (Status)) {
> - goto FreeAndReturn;
> - }
> -
> - Status = LoadLinuxInitializeKernelSetup (SetupBuf);
> - if (EFI_ERROR (Status)) {
> - goto FreeAndReturn;
> - }
> -
> - KernelInitialSize = LoadLinuxGetKernelSize (SetupBuf, KernelSize);
> - if (KernelInitialSize == 0) {
> - Status = EFI_UNSUPPORTED;
> - goto FreeAndReturn;
> - }
> -
> - KernelBuf = LoadLinuxAllocateKernelPages (
> - SetupBuf,
> - EFI_SIZE_TO_PAGES (KernelInitialSize));
> - if (KernelBuf == NULL) {
> - DEBUG ((EFI_D_ERROR, "Unable to allocate memory for kernel!\n"));
> - Status = EFI_OUT_OF_RESOURCES;
> - goto FreeAndReturn;
> - }
> -
> - DEBUG ((EFI_D_INFO, "Kernel size: 0x%x\n", (UINT32) KernelSize));
> - DEBUG ((EFI_D_INFO, "Reading kernel image ..."));
> - QemuFwCfgSelectItem (QemuFwCfgItemKernelData);
> - QemuFwCfgReadBytes (KernelSize, KernelBuf);
> - DEBUG ((EFI_D_INFO, " [done]\n"));
> -
> - QemuFwCfgSelectItem (QemuFwCfgItemCommandLineSize);
> - CommandLineSize = (UINTN) QemuFwCfgRead64 ();
> -
> - if (CommandLineSize > 0) {
> - CommandLine = LoadLinuxAllocateCommandLinePages (
> - EFI_SIZE_TO_PAGES (CommandLineSize));
> - QemuFwCfgSelectItem (QemuFwCfgItemCommandLineData);
> - QemuFwCfgReadBytes (CommandLineSize, CommandLine);
> - } else {
> - CommandLine = NULL;
> - }
> -
> - Status = LoadLinuxSetCommandLine (SetupBuf, CommandLine);
> - if (EFI_ERROR (Status)) {
> - goto FreeAndReturn;
> - }
> -
> - QemuFwCfgSelectItem (QemuFwCfgItemInitrdSize);
> - InitrdSize = (UINTN) QemuFwCfgRead64 ();
> -
> - if (InitrdSize > 0) {
> - InitrdData = LoadLinuxAllocateInitrdPages (
> - SetupBuf,
> - EFI_SIZE_TO_PAGES (InitrdSize)
> - );
> - DEBUG ((EFI_D_INFO, "Initrd size: 0x%x\n", (UINT32) InitrdSize));
> - DEBUG ((EFI_D_INFO, "Reading initrd image ..."));
> - QemuFwCfgSelectItem (QemuFwCfgItemInitrdData);
> - QemuFwCfgReadBytes (InitrdSize, InitrdData);
> - DEBUG ((EFI_D_INFO, " [done]\n"));
> - } else {
> - InitrdData = NULL;
> - }
> -
> - Status = LoadLinuxSetInitrd (SetupBuf, InitrdData, InitrdSize);
> - if (EFI_ERROR (Status)) {
> - goto FreeAndReturn;
> + EFI_HANDLE KernelImageHandle;
> +
> + Status = QemuLoadKernelImage (&KernelImageHandle);
> + if (EFI_ERROR (Status)) {
> + if (Status != EFI_SECURITY_VIOLATION) {
(1) This special treatment should disappear, once QemuLoadKernelImage()
keeps EFI_SECURITY_VIOLATION inside.
> + return Status;
> + }
> + //
> + // From the resource allocation perspective, EFI_SECURITY_VIOLATION means
> + // "success", so we must roll back the image loading.
> + //
> + goto UnloadKernelImage;
> }
>
> //
> - // Signal the EVT_SIGNAL_READY_TO_BOOT event
> + // Signal the EFI_EVENT_GROUP_READY_TO_BOOT event.
> //
> EfiSignalEventReadyToBoot();
>
> REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
>
> - Status = LoadLinux (KernelBuf, SetupBuf);
> + //
> + // Start the image.
> + //
> + Status = QemuStartKernelImage (KernelImageHandle);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((EFI_D_ERROR, "%a: StartImage(): %r\n", __FUNCTION__, Status));
> + }
(2) Please run "PatchCheck.py" on the series; it will reject "EFI_D_ERROR".
Thanks,
Laszlo
>
> -FreeAndReturn:
> - if (SetupBuf != NULL) {
> - FreePages (SetupBuf, EFI_SIZE_TO_PAGES (SetupSize));
> - }
> - if (KernelBuf != NULL) {
> - FreePages (KernelBuf, EFI_SIZE_TO_PAGES (KernelInitialSize));
> - }
> - if (CommandLine != NULL) {
> - FreePages (CommandLine, EFI_SIZE_TO_PAGES (CommandLineSize));
> - }
> - if (InitrdData != NULL) {
> - FreePages (InitrdData, EFI_SIZE_TO_PAGES (InitrdSize));
> - }
> +UnloadKernelImage:
> + QemuUnloadKernelImage (KernelImageHandle);
>
> return Status;
> }
> -
>
next prev parent reply other threads:[~2020-03-03 9:52 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-02 7:29 [PATCH 00/13] Ovmf: use LoadImage/StartImage for loading command line images Ard Biesheuvel
2020-03-02 7:29 ` [PATCH 01/13] OvmfPkg: add GUID for the QEMU kernel loader fs media device path Ard Biesheuvel
2020-03-02 13:22 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 02/13] OvmfPkg: export abstract QEMU blob filesystem in standalone driver Ard Biesheuvel
2020-03-02 13:45 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 03/13] OvmfPkg: introduce QemuLoadImageLib library class Ard Biesheuvel
2020-03-02 14:07 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 04/13] OvmfPkg: provide a generic implementation of QemuLoadImageLib Ard Biesheuvel
2020-03-02 17:12 ` [edk2-devel] " Laszlo Ersek
2020-03-03 7:36 ` Laszlo Ersek
2020-03-02 7:29 ` [PATCH 05/13] ArmVirtPkg: incorporate the new QEMU kernel loader driver and library Ard Biesheuvel
2020-03-02 17:15 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 06/13] ArmVirtPkg/PlatformBootManagerLib: switch to separate QEMU loader Ard Biesheuvel
2020-03-02 17:26 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 07/13] OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command line Ard Biesheuvel
2020-03-02 17:31 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 08/13] OvmfPkg/QemuKernelLoaderFsDxe: add support for the kernel setup block Ard Biesheuvel
2020-03-02 17:58 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 09/13] OvmfPkg: implement QEMU loader library for X86 with legacy fallback Ard Biesheuvel
2020-03-03 9:45 ` [edk2-devel] " Laszlo Ersek
2020-03-03 10:08 ` Ard Biesheuvel
2020-03-03 11:20 ` Laszlo Ersek
2020-03-02 7:29 ` [PATCH 10/13] OvmfPkg: add new QEMU kernel image loader components Ard Biesheuvel
2020-03-03 9:47 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 11/13] OvmfPkg/PlatformBootManagerLib: switch to QemuLoadImageLib Ard Biesheuvel
2020-03-03 9:52 ` Laszlo Ersek [this message]
2020-03-03 9:53 ` [edk2-devel] " Laszlo Ersek
2020-03-02 7:29 ` [PATCH 12/13] OvmfPkg/QemuKernelLoaderFsDxe: add support for new Linux initrd device path Ard Biesheuvel
2020-03-03 10:10 ` [edk2-devel] " Laszlo Ersek
2020-03-03 10:18 ` Ard Biesheuvel
2020-03-03 11:27 ` Laszlo Ersek
2020-03-02 7:29 ` [PATCH 13/13] OvmfPkg: use generic QEMU image loader for secure boot enabled builds Ard Biesheuvel
2020-03-03 10:13 ` [edk2-devel] " 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=f832ad31-0724-ebb7-1a55-e96c797454c1@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