[v3] Ovmf: use LoadImage/StartImage for loading command line images

[edk2-devel] [PATCH v3 00/14] Ovmf: use LoadImage/StartImage for loading command line images

Posted by Ard Biesheuvel 4 years, 1 month ago

On ArmVirtQemu, we require the kernel passed via the QEMU -kernel option
to have a PE/COFF header and an EFI stub, so that it can be loaded and
started using the LoadImage and StartImage boot services, respectively.
This means that, on builds that enable secure boot or measured boot, the
kernel image gets authenticated and/or measured as well.

On X86, for historical reasons, we never use LoadImage or StartImage, which
means that:
- kernel images are never authenticated or measured,
- calling Exit() from within the boot stub will attempt tp terminate the
  calling image, which is likely to end badly.

So instead, split and generalize the code that exists today for ArmVirtQemu,
and wire it up for x86 so that LoadImage and StartImage are used unless
there is a true need for the special Linux boot protocol.

The first 6 patches are only intended to be a refactoring of the existing
code, and should not result in any functional changes for either ArmVirtQemu
or OVMF.

Patch #12 (now #13 adds the new Linux specific initrd loadfile2 protocol that
aims to simplify initrd loading from Linux when booting via the PE stub.

Patch #13 (now #14) is optional, and disables the Linux loader fallback on
builds that have secure boot enabled.

Changes since [v2]:
- rename gX86QemuKernelLoadedImageGuid to gOvmfLoadedX86LinuxKernelProtocolGuid,
  and define the associated struct type OVMF_LOADED_X86_LINUX_KERNEL in the
  protocol header file
- mention that the new protocol is internal ABI and subject to backward
  incompatible change at any time
- align legacy loader logic more closely with the generic one
- modify legacy mixed mode handling to prevent returning a stale handle
- add Laszlo's ack to #4, #6, #8, #12 and #13
- fix up some style issues and out of date/inaccurate comments (Laszlo)

Changes since [v1]:
- handle EFI_SECURITY_VIOLATION return codes from gBS->LoadImage inside the
  QemuLoadImageLib implementation consistently, instead of propagating it
- change the prototype of QemuStartKernelImage () to take the handle by
  reference, allowing the fallback x86 code to reload the image onto a
  fresh handle if needed
- add new patch to declare gX86QemuKernelLoadedImageGuid, and make it a
  true protocol instead of just a GUID
- drop unnecessary 'wrapper' struct around QEMU_LEGACY_LOADED_IMAGE (#10)
- switch to QemuFwCfgRead32() consistently
- fix numerous other minor style and logic issues pointed out by Laszlo
- add Laszlo's ack to #1, #2, #3, #5, #7, #11 and #14 (*)

(*) v2+ numbering

Code can be found here:
https://github.com/ardbiesheuvel/edk2/tree/ovmf-loadimage-startimage-v1
https://github.com/ardbiesheuvel/edk2/tree/ovmf-loadimage-startimage-v2
https://github.com/ardbiesheuvel/edk2/tree/ovmf-loadimage-startimage-v3

[v1] http://mid.mail-archive.com/20200302072936.29221-1-ard.biesheuvel@linaro.org
[v2] http://mid.mail-archive.com/20200304095233.21046-1-ard.biesheuvel@linaro.org

Ard Biesheuvel (14):
  OvmfPkg: add GUID for the QEMU kernel loader fs media device path
  OvmfPkg: export abstract QEMU blob filesystem in standalone driver
  OvmfPkg: introduce QemuLoadImageLib library class
  OvmfPkg: provide a generic implementation of QemuLoadImageLib
  ArmVirtPkg: incorporate the new QEMU kernel loader driver and library
  ArmVirtPkg/PlatformBootManagerLib: switch to separate QEMU loader
  OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command line
  OvmfPkg/QemuKernelLoaderFsDxe: add support for the kernel setup block
  OvmfPkg: create protocol and GUID header for loaded x86 Linux kernels
  OvmfPkg: implement QEMU loader library for X86 with legacy fallback
  OvmfPkg: add new QEMU kernel image loader components
  OvmfPkg/PlatformBootManagerLib: switch to QemuLoadImageLib
  OvmfPkg/QemuKernelLoaderFsDxe: add support for new Linux initrd device
    path
  OvmfPkg: use generic QEMU image loader for secure boot enabled builds

 ArmVirtPkg/ArmVirtQemu.dsc                    |    2 +
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc          |    1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc              |    2 +
 .../PlatformBootManagerLib.inf                |    9 +-
 .../PlatformBootManagerLib/QemuKernel.c       | 1061 +----------------
 .../Include/Guid/QemuKernelLoaderFsMedia.h    |   18 +
 OvmfPkg/Include/Library/QemuLoadImageLib.h    |   84 ++
 .../Protocol/OvmfLoadedX86LinuxKernel.h       |   32 +
 .../GenericQemuLoadImageLib.c                 |  276 +++++
 .../GenericQemuLoadImageLib.inf               |   38 +
 .../PlatformBootManagerLib.inf                |    2 +-
 .../PlatformBootManagerLib/QemuKernel.c       |  144 +--
 .../X86QemuLoadImageLib/X86QemuLoadImageLib.c |  567 +++++++++
 .../X86QemuLoadImageLib.inf                   |   42 +
 OvmfPkg/OvmfPkg.dec                           |   57 +-
 OvmfPkg/OvmfPkgIa32.dsc                       |    6 +
 OvmfPkg/OvmfPkgIa32.fdf                       |    1 +
 OvmfPkg/OvmfPkgIa32X64.dsc                    |    6 +
 OvmfPkg/OvmfPkgIa32X64.fdf                    |    1 +
 OvmfPkg/OvmfPkgX64.dsc                        |    6 +
 OvmfPkg/OvmfPkgX64.fdf                        |    1 +
 .../QemuKernelLoaderFsDxe.c                   |  367 +++---
 .../QemuKernelLoaderFsDxe.inf                 |   50 +
 23 files changed, 1354 insertions(+), 1419 deletions(-)
 create mode 100644 OvmfPkg/Include/Guid/QemuKernelLoaderFsMedia.h
 create mode 100644 OvmfPkg/Include/Library/QemuLoadImageLib.h
 create mode 100644 OvmfPkg/Include/Protocol/OvmfLoadedX86LinuxKernel.h
 create mode 100644 OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c
 create mode 100644 OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
 create mode 100644 OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
 create mode 100644 OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 copy ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c => OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c (77%)
 create mode 100644 OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf

-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55523): https://edk2.groups.io/g/devel/message/55523
Mute This Topic: https://groups.io/mt/71749512/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 00/14] Ovmf: use LoadImage/StartImage for loading command line images

Posted by Bob Feng 4 years, 1 month ago

Hi Ard,

I found this patch set cause Ovmf platform build failure on windows with VS2017. 

The error message is as following:

Generating code
d:\edk2\OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130): error C2220: warning treated as error - no 'object' file generated
d:\edk2\OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130): warning C4132: 'mEfiFileProtocolTemplate': const object should be initialized
TcpTimer.c
Finished generating code
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\cl.exe"' : return code '0x2'
Stop.


build.py...
 : error 7000: Failed to execute command
        C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\nmake.exe /nologo tbuild [d:\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe]


build.py...
 : error F002: Failed to build module
        d:\edk2\OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.inf [IA32, VS2017, DEBUG]


Please help to provide patch to fix it.

Thanks,
Bob


-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ard Biesheuvel
Sent: Thursday, March 5, 2020 9:46 PM
To: devel@edk2.groups.io
Cc: lersek@redhat.com; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [edk2-devel] [PATCH v3 00/14] Ovmf: use LoadImage/StartImage for loading command line images

On ArmVirtQemu, we require the kernel passed via the QEMU -kernel option to have a PE/COFF header and an EFI stub, so that it can be loaded and started using the LoadImage and StartImage boot services, respectively.
This means that, on builds that enable secure boot or measured boot, the kernel image gets authenticated and/or measured as well.

On X86, for historical reasons, we never use LoadImage or StartImage, which means that:
- kernel images are never authenticated or measured,
- calling Exit() from within the boot stub will attempt tp terminate the
  calling image, which is likely to end badly.

So instead, split and generalize the code that exists today for ArmVirtQemu, and wire it up for x86 so that LoadImage and StartImage are used unless there is a true need for the special Linux boot protocol.

The first 6 patches are only intended to be a refactoring of the existing code, and should not result in any functional changes for either ArmVirtQemu or OVMF.

Patch #12 (now #13 adds the new Linux specific initrd loadfile2 protocol that aims to simplify initrd loading from Linux when booting via the PE stub.

Patch #13 (now #14) is optional, and disables the Linux loader fallback on builds that have secure boot enabled.

Changes since [v2]:
- rename gX86QemuKernelLoadedImageGuid to gOvmfLoadedX86LinuxKernelProtocolGuid,
  and define the associated struct type OVMF_LOADED_X86_LINUX_KERNEL in the
  protocol header file
- mention that the new protocol is internal ABI and subject to backward
  incompatible change at any time
- align legacy loader logic more closely with the generic one
- modify legacy mixed mode handling to prevent returning a stale handle
- add Laszlo's ack to #4, #6, #8, #12 and #13
- fix up some style issues and out of date/inaccurate comments (Laszlo)

Changes since [v1]:
- handle EFI_SECURITY_VIOLATION return codes from gBS->LoadImage inside the
  QemuLoadImageLib implementation consistently, instead of propagating it
- change the prototype of QemuStartKernelImage () to take the handle by
  reference, allowing the fallback x86 code to reload the image onto a
  fresh handle if needed
- add new patch to declare gX86QemuKernelLoadedImageGuid, and make it a
  true protocol instead of just a GUID
- drop unnecessary 'wrapper' struct around QEMU_LEGACY_LOADED_IMAGE (#10)
- switch to QemuFwCfgRead32() consistently
- fix numerous other minor style and logic issues pointed out by Laszlo
- add Laszlo's ack to #1, #2, #3, #5, #7, #11 and #14 (*)

(*) v2+ numbering

Code can be found here:
https://github.com/ardbiesheuvel/edk2/tree/ovmf-loadimage-startimage-v1
https://github.com/ardbiesheuvel/edk2/tree/ovmf-loadimage-startimage-v2
https://github.com/ardbiesheuvel/edk2/tree/ovmf-loadimage-startimage-v3

[v1] http://mid.mail-archive.com/20200302072936.29221-1-ard.biesheuvel@linaro.org
[v2] http://mid.mail-archive.com/20200304095233.21046-1-ard.biesheuvel@linaro.org

Ard Biesheuvel (14):
  OvmfPkg: add GUID for the QEMU kernel loader fs media device path
  OvmfPkg: export abstract QEMU blob filesystem in standalone driver
  OvmfPkg: introduce QemuLoadImageLib library class
  OvmfPkg: provide a generic implementation of QemuLoadImageLib
  ArmVirtPkg: incorporate the new QEMU kernel loader driver and library
  ArmVirtPkg/PlatformBootManagerLib: switch to separate QEMU loader
  OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command line
  OvmfPkg/QemuKernelLoaderFsDxe: add support for the kernel setup block
  OvmfPkg: create protocol and GUID header for loaded x86 Linux kernels
  OvmfPkg: implement QEMU loader library for X86 with legacy fallback
  OvmfPkg: add new QEMU kernel image loader components
  OvmfPkg/PlatformBootManagerLib: switch to QemuLoadImageLib
  OvmfPkg/QemuKernelLoaderFsDxe: add support for new Linux initrd device
    path
  OvmfPkg: use generic QEMU image loader for secure boot enabled builds

 ArmVirtPkg/ArmVirtQemu.dsc                    |    2 +
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc          |    1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc              |    2 +
 .../PlatformBootManagerLib.inf                |    9 +-
 .../PlatformBootManagerLib/QemuKernel.c       | 1061 +----------------
 .../Include/Guid/QemuKernelLoaderFsMedia.h    |   18 +
 OvmfPkg/Include/Library/QemuLoadImageLib.h    |   84 ++
 .../Protocol/OvmfLoadedX86LinuxKernel.h       |   32 +
 .../GenericQemuLoadImageLib.c                 |  276 +++++
 .../GenericQemuLoadImageLib.inf               |   38 +
 .../PlatformBootManagerLib.inf                |    2 +-
 .../PlatformBootManagerLib/QemuKernel.c       |  144 +--
 .../X86QemuLoadImageLib/X86QemuLoadImageLib.c |  567 +++++++++
 .../X86QemuLoadImageLib.inf                   |   42 +
 OvmfPkg/OvmfPkg.dec                           |   57 +-
 OvmfPkg/OvmfPkgIa32.dsc                       |    6 +
 OvmfPkg/OvmfPkgIa32.fdf                       |    1 +
 OvmfPkg/OvmfPkgIa32X64.dsc                    |    6 +
 OvmfPkg/OvmfPkgIa32X64.fdf                    |    1 +
 OvmfPkg/OvmfPkgX64.dsc                        |    6 +
 OvmfPkg/OvmfPkgX64.fdf                        |    1 +
 .../QemuKernelLoaderFsDxe.c                   |  367 +++---
 .../QemuKernelLoaderFsDxe.inf                 |   50 +
 23 files changed, 1354 insertions(+), 1419 deletions(-)  create mode 100644 OvmfPkg/Include/Guid/QemuKernelLoaderFsMedia.h
 create mode 100644 OvmfPkg/Include/Library/QemuLoadImageLib.h
 create mode 100644 OvmfPkg/Include/Protocol/OvmfLoadedX86LinuxKernel.h
 create mode 100644 OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c
 create mode 100644 OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
 create mode 100644 OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
 create mode 100644 OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 copy ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c => OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c (77%)  create mode 100644 OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf

--
2.17.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55565): https://edk2.groups.io/g/devel/message/55565
Mute This Topic: https://groups.io/mt/71749512/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 00/14] Ovmf: use LoadImage/StartImage for loading command line images

Posted by Ard Biesheuvel 4 years, 1 month ago

On Fri, 6 Mar 2020 at 03:01, Feng, Bob C <bob.c.feng@intel.com> wrote:
>
> Hi Ard,
>
> I found this patch set cause Ovmf platform build failure on windows with VS2017.
>
> The error message is as following:
>
> Generating code
> d:\edk2\OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130): error C2220: warning treated as error - no 'object' file generated
> d:\edk2\OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130): warning C4132: 'mEfiFileProtocolTemplate': const object should be initialized
> TcpTimer.c
> Finished generating code
> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\cl.exe"' : return code '0x2'
> Stop.
>
>
> build.py...
>  : error 7000: Failed to execute command
>         C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\nmake.exe /nologo tbuild [d:\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe]
>
>
> build.py...
>  : error F002: Failed to build module
>         d:\edk2\OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.inf [IA32, VS2017, DEBUG]
>
>
> Please help to provide patch to fix it.
>

Interesting. That code was moved from an ARM only source file to one
that is shared between all architectures.

GCC and Clang have no problem with that, and I think a tentative
definitive of a const object is permitted by the spec.

Anyway, I sent a patch to fix it. Please confirm whether it works for you.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55589): https://edk2.groups.io/g/devel/message/55589
Mute This Topic: https://groups.io/mt/71749512/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-