* [PATCH v2 1/3] Revert "OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command line"
2021-06-17 9:12 [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Dov Murik
@ 2021-06-17 9:12 ` Dov Murik
2021-06-17 9:12 ` [PATCH v2 2/3] OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs Dov Murik
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Dov Murik @ 2021-06-17 9:12 UTC (permalink / raw)
To: devel; +Cc: Dov Murik
This reverts commit efc52d67e1573ce174d301b52fa1577d552c8441.
Manually fixed conflicts in:
OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
Note that besides re-exposing the kernel command line as a file in the
synthetic filesystem, we also revert back to AllocatePool instead of
AllocatePages.
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
index b09ff6a3590d..c7ddd86f5c75 100644
--- a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
+++ b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
@@ -33,6 +33,7 @@
typedef enum {
KernelBlobTypeKernel,
KernelBlobTypeInitrd,
+ KernelBlobTypeCommandLine,
KernelBlobTypeMax
} KERNEL_BLOB_TYPE;
@@ -59,6 +60,11 @@ STATIC KERNEL_BLOB mKernelBlob[KernelBlobTypeMax] = {
{
{ QemuFwCfgItemInitrdSize, QemuFwCfgItemInitrdData, },
}
+ }, {
+ L"cmdline",
+ {
+ { QemuFwCfgItemCommandLineSize, QemuFwCfgItemCommandLineData, },
+ }
}
};
@@ -948,7 +954,7 @@ FetchBlob (
//
// Read blob.
//
- Blob->Data = AllocatePages (EFI_SIZE_TO_PAGES ((UINTN)Blob->Size));
+ Blob->Data = AllocatePool (Blob->Size);
if (Blob->Data == NULL) {
DEBUG ((DEBUG_ERROR, "%a: failed to allocate %Ld bytes for \"%s\"\n",
__FUNCTION__, (INT64)Blob->Size, Blob->Name));
@@ -1083,8 +1089,7 @@ FreeBlobs:
while (BlobType > 0) {
CurrentBlob = &mKernelBlob[--BlobType];
if (CurrentBlob->Data != NULL) {
- FreePages (CurrentBlob->Data,
- EFI_SIZE_TO_PAGES ((UINTN)CurrentBlob->Size));
+ FreePool (CurrentBlob->Data);
CurrentBlob->Size = 0;
CurrentBlob->Data = NULL;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/3] OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs
2021-06-17 9:12 [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Dov Murik
2021-06-17 9:12 ` [PATCH v2 1/3] Revert "OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command line" Dov Murik
@ 2021-06-17 9:12 ` Dov Murik
2021-06-17 9:12 ` [PATCH v2 3/3] OvmfPkg/X86QemuLoadImageLib: State fw_cfg dependency in file header Dov Murik
2021-06-17 12:01 ` [edk2-devel] [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Ard Biesheuvel
3 siblings, 0 replies; 6+ messages in thread
From: Dov Murik @ 2021-06-17 9:12 UTC (permalink / raw)
To: devel; +Cc: Dov Murik
Remove the QemuFwCfgLib interface used to read the QEMU cmdline
(-append argument) and the initrd size. Instead, use the synthetic
filesystem QemuKernelLoaderFs which has three files: "kernel", "initrd",
and "cmdline".
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 2 +-
OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 145 ++++++++++++++++++--
2 files changed, 133 insertions(+), 14 deletions(-)
diff --git a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
index b262cb926a4d..f462fd6922cf 100644
--- a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
+++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
@@ -27,12 +27,12 @@ [LibraryClasses]
DebugLib
MemoryAllocationLib
PrintLib
- QemuFwCfgLib
UefiBootServicesTableLib
[Protocols]
gEfiDevicePathProtocolGuid
gEfiLoadedImageProtocolGuid
+ gEfiSimpleFileSystemProtocolGuid
[Guids]
gQemuKernelLoaderFsMediaGuid
diff --git a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c
index 114db7e8441f..f520456e3b24 100644
--- a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c
+++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c
@@ -11,9 +11,9 @@
#include <Base.h>
#include <Guid/QemuKernelLoaderFsMedia.h>
#include <Library/DebugLib.h>
+#include <Library/FileHandleLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/PrintLib.h>
-#include <Library/QemuFwCfgLib.h>
#include <Library/QemuLoadImageLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/DevicePath.h>
@@ -30,6 +30,11 @@ typedef struct {
KERNEL_FILE_DEVPATH FileNode;
EFI_DEVICE_PATH_PROTOCOL EndNode;
} KERNEL_VENMEDIA_FILE_DEVPATH;
+
+typedef struct {
+ VENDOR_DEVICE_PATH VenMediaNode;
+ EFI_DEVICE_PATH_PROTOCOL EndNode;
+} SINGLE_VENMEDIA_NODE_DEVPATH;
#pragma pack ()
STATIC CONST KERNEL_VENMEDIA_FILE_DEVPATH mKernelDevicePath = {
@@ -51,6 +56,78 @@ STATIC CONST KERNEL_VENMEDIA_FILE_DEVPATH mKernelDevicePath = {
}
};
+STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH mQemuKernelLoaderFileSystemDevicePath = {
+ {
+ {
+ MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
+ { sizeof (VENDOR_DEVICE_PATH) }
+ },
+ QEMU_KERNEL_LOADER_FS_MEDIA_GUID
+ }, {
+ END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
+ { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
+ }
+};
+
+STATIC
+EFI_STATUS
+GetQemuKernelLoaderBlobSize (
+ IN EFI_FILE_HANDLE Root,
+ IN CHAR16 *FileName,
+ OUT UINTN *Size
+ )
+{
+ EFI_STATUS Status;
+ EFI_FILE_HANDLE FileHandle;
+ UINT64 FileSize;
+
+ Status = Root->Open (Root, &FileHandle, FileName, EFI_FILE_MODE_READ, 0);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ Status = FileHandleGetSize (FileHandle, &FileSize);
+ if (EFI_ERROR (Status)) {
+ goto CloseFile;
+ }
+ *Size = FileSize;
+ Status = EFI_SUCCESS;
+CloseFile:
+ FileHandle->Close (FileHandle);
+ return Status;
+}
+
+STATIC
+EFI_STATUS
+ReadWholeQemuKernelLoaderBlob (
+ IN EFI_FILE_HANDLE Root,
+ IN CHAR16 *FileName,
+ IN UINTN Size,
+ OUT VOID *Buffer
+ )
+{
+ EFI_STATUS Status;
+ EFI_FILE_HANDLE FileHandle;
+ UINTN ReadSize;
+
+ Status = Root->Open (Root, &FileHandle, FileName, EFI_FILE_MODE_READ, 0);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ ReadSize = Size;
+ Status = FileHandle->Read (FileHandle, &ReadSize, Buffer);
+ if (EFI_ERROR (Status)) {
+ goto CloseFile;
+ }
+ if (ReadSize != Size) {
+ Status = EFI_PROTOCOL_ERROR;
+ goto CloseFile;
+ }
+ Status = EFI_SUCCESS;
+CloseFile:
+ FileHandle->Close (FileHandle);
+ return Status;
+}
+
/**
Download the kernel, the initial ramdisk, and the kernel command line from
QEMU's fw_cfg. The kernel will be instructed via its command line to load
@@ -76,12 +153,16 @@ QemuLoadKernelImage (
OUT EFI_HANDLE *ImageHandle
)
{
- EFI_STATUS Status;
- EFI_HANDLE KernelImageHandle;
- EFI_LOADED_IMAGE_PROTOCOL *KernelLoadedImage;
- UINTN CommandLineSize;
- CHAR8 *CommandLine;
- UINTN InitrdSize;
+ EFI_STATUS Status;
+ EFI_HANDLE KernelImageHandle;
+ EFI_LOADED_IMAGE_PROTOCOL *KernelLoadedImage;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
+ EFI_HANDLE FsVolumeHandle;
+ EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *FsProtocol;
+ EFI_FILE_HANDLE Root;
+ UINTN CommandLineSize;
+ CHAR8 *CommandLine;
+ UINTN InitrdSize;
//
// Load the image. This should call back into the QEMU EFI loader file system.
@@ -124,8 +205,38 @@ QemuLoadKernelImage (
);
ASSERT_EFI_ERROR (Status);
- QemuFwCfgSelectItem (QemuFwCfgItemCommandLineSize);
- CommandLineSize = (UINTN)QemuFwCfgRead32 ();
+ //
+ // Open the Qemu Kernel Loader abstract filesystem (volume) which will be
+ // used to read the "initrd" and "cmdline" synthetic files.
+ //
+ DevicePathNode = (EFI_DEVICE_PATH_PROTOCOL *)&mQemuKernelLoaderFileSystemDevicePath;
+ Status = gBS->LocateDevicePath (
+ &gEfiSimpleFileSystemProtocolGuid,
+ &DevicePathNode,
+ &FsVolumeHandle
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = gBS->HandleProtocol (
+ FsVolumeHandle,
+ &gEfiSimpleFileSystemProtocolGuid,
+ (VOID **)&FsProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = FsProtocol->OpenVolume (FsVolumeHandle, &Root);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = GetQemuKernelLoaderBlobSize (Root, L"cmdline", &CommandLineSize);
+ if (EFI_ERROR (Status)) {
+ goto CloseRoot;
+ }
if (CommandLineSize == 0) {
KernelLoadedImage->LoadOptionsSize = 0;
@@ -136,8 +247,11 @@ QemuLoadKernelImage (
goto UnloadImage;
}
- QemuFwCfgSelectItem (QemuFwCfgItemCommandLineData);
- QemuFwCfgReadBytes (CommandLineSize, CommandLine);
+ Status = ReadWholeQemuKernelLoaderBlob (Root, L"cmdline", CommandLineSize,
+ CommandLine);
+ if (EFI_ERROR (Status)) {
+ goto FreeCommandLine;
+ }
//
// Verify NUL-termination of the command line.
@@ -155,8 +269,10 @@ QemuLoadKernelImage (
KernelLoadedImage->LoadOptionsSize = (UINT32)((CommandLineSize - 1) * 2);
}
- QemuFwCfgSelectItem (QemuFwCfgItemInitrdSize);
- InitrdSize = (UINTN)QemuFwCfgRead32 ();
+ Status = GetQemuKernelLoaderBlobSize (Root, L"initrd", &InitrdSize);
+ if (EFI_ERROR (Status)) {
+ goto FreeCommandLine;
+ }
if (InitrdSize > 0) {
//
@@ -193,6 +309,7 @@ QemuLoadKernelImage (
}
*ImageHandle = KernelImageHandle;
+ Root->Close (Root);
return EFI_SUCCESS;
FreeCommandLine:
@@ -201,6 +318,8 @@ FreeCommandLine:
}
UnloadImage:
gBS->UnloadImage (KernelImageHandle);
+CloseRoot:
+ Root->Close (Root);
return Status;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] OvmfPkg/X86QemuLoadImageLib: State fw_cfg dependency in file header
2021-06-17 9:12 [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Dov Murik
2021-06-17 9:12 ` [PATCH v2 1/3] Revert "OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command line" Dov Murik
2021-06-17 9:12 ` [PATCH v2 2/3] OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs Dov Murik
@ 2021-06-17 9:12 ` Dov Murik
2021-06-17 12:01 ` [edk2-devel] [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Ard Biesheuvel
3 siblings, 0 replies; 6+ messages in thread
From: Dov Murik @ 2021-06-17 9:12 UTC (permalink / raw)
To: devel
Cc: Dov Murik, Laszlo Ersek, Ard Biesheuvel, Jordan Justen,
James Bottomley, Tobin Feldman-Fitzthum
Make it clear that X86QemuLoadImageLib relies on fw_cfg; prepare the
ground to add a warning about the incompatibility with boot verification
process.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf | 3 +++
OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
index e1615badd2ba..c7ec041cb706 100644
--- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
+++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
@@ -2,6 +2,9 @@
# X86 specific implementation of QemuLoadImageLib library class interface
# with support for loading mixed mode images and non-EFI stub images
#
+# Note that this implementation reads the cmdline (and possibly kernel, setup
+# data, and initrd in the legacy boot mode) from fw_cfg directly.
+#
# Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
index 1177582ab051..dc9018f4333b 100644
--- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
+++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
@@ -2,6 +2,9 @@
X86 specific implementation of QemuLoadImageLib library class interface
with support for loading mixed mode images and non-EFI stub images
+ Note that this implementation reads the cmdline (and possibly kernel, setup
+ data, and initrd in the legacy boot mode) from fw_cfg directly.
+
Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd
2021-06-17 9:12 [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Dov Murik
` (2 preceding siblings ...)
2021-06-17 9:12 ` [PATCH v2 3/3] OvmfPkg/X86QemuLoadImageLib: State fw_cfg dependency in file header Dov Murik
@ 2021-06-17 12:01 ` Ard Biesheuvel
2021-06-17 12:18 ` Dov Murik
3 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2021-06-17 12:01 UTC (permalink / raw)
To: edk2-devel-groups-io, Dov Murik
Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, James Bottomley,
Tobin Feldman-Fitzthum
On Thu, 17 Jun 2021 at 11:12, Dov Murik <dovmurik@linux.ibm.com> wrote:
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3457
>
> In order to support measured SEV boot with kernel/initrd/cmdline, we'd
> like to have one place that reads those blobs; in the future we'll add
> the measurement and verification in that place.
>
> We already have a synthetic filesystem (QemuKernelLoaderFs) which holds
> three files: "kernel", "initrd", and "cmdline". The kernel is indeed
> read from this filesystem in LoadImage; but the cmdline (and the length
> of initrd) are read from QemuFwCfgLib items.
>
> This patch series modifies GenericQemuLoadImageLib to read cmdline (and
> the initrd size) from the QemuKernelLoaderFs synthetic filesystem, thus
> removing the dependency on QemuFwCfgLib.
>
> Note that X86QemuLoadImageLib is not modified, because it contains a
> QemuLoadLegacyImage() which reads other items of the QemuFwCfg which are
> not available in QemuKernelLoaderFs. Since we don't want to support the
> legacy boot path in the future measured SEV boot, we leave
> X86QemuLoadImageLib as-is (except for a comment addition in patch 3) and
> will force use for GenericQemuLoadImageLib in the measured SEV boot
> implementation.
>
> Relevant discussion threads start in:
> https://edk2.groups.io/g/devel/message/76069
>
> To test this on x86_64, I forced the use of GenericQemuLoadImageLib
> using the following local patch:
>
>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 0a237a905866..46442b543bcf 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -404,7 +404,7 @@ [LibraryClasses.common.DXE_DRIVER]
> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
> - QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
> + QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf # XXX don't commit this or someone will be mad
> !if $(TPM_ENABLE) == TRUE
> Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf
> Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
>
>
> I tested boot with QEMU and OVMF with the following QEMU arguments:
>
> -kernel a
> -kernel a -initrd b
> -kernel a -cmdline c
> -kernel a -initrd b -cmdline c
>
> (and also without -kernel)
>
>
> Code is at
> https://github.com/confidential-containers-demo/edk2/tree/use-synthetic-fs-for-cmdline-v2
>
> v2 changes:
>
> - Add comment to header of X86QemuLoadImageLib.inf
> - Clearer function names in GenericQemuLoadImageLib.c
> - Fix coding style issues
>
> v1: https://edk2.groups.io/g/devel/message/76265
>
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
>
> Dov Murik (3):
> Revert "OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command
> line"
> OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs
> OvmfPkg/X86QemuLoadImageLib: State fw_cfg dependency in file header
>
Please cc me on the entire series.
> OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 2 +-
> OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf | 3 +
> OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 145 ++++++++++++++++++--
> OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 3 +
> OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 11 +-
> 5 files changed, 147 insertions(+), 17 deletions(-)
>
> --
> 2.25.1
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [edk2-devel] [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd
2021-06-17 12:01 ` [edk2-devel] [PATCH v2 0/3] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Ard Biesheuvel
@ 2021-06-17 12:18 ` Dov Murik
0 siblings, 0 replies; 6+ messages in thread
From: Dov Murik @ 2021-06-17 12:18 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel-groups-io
Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, James Bottomley,
Tobin Feldman-Fitzthum
On 17/06/2021 15:01, Ard Biesheuvel wrote:
> On Thu, 17 Jun 2021 at 11:12, Dov Murik <dovmurik@linux.ibm.com> wrote:
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3457
>>
>> In order to support measured SEV boot with kernel/initrd/cmdline, we'd
>> like to have one place that reads those blobs; in the future we'll add
>> the measurement and verification in that place.
>>
>> We already have a synthetic filesystem (QemuKernelLoaderFs) which holds
>> three files: "kernel", "initrd", and "cmdline". The kernel is indeed
>> read from this filesystem in LoadImage; but the cmdline (and the length
>> of initrd) are read from QemuFwCfgLib items.
>>
>> This patch series modifies GenericQemuLoadImageLib to read cmdline (and
>> the initrd size) from the QemuKernelLoaderFs synthetic filesystem, thus
>> removing the dependency on QemuFwCfgLib.
>>
>> Note that X86QemuLoadImageLib is not modified, because it contains a
>> QemuLoadLegacyImage() which reads other items of the QemuFwCfg which are
>> not available in QemuKernelLoaderFs. Since we don't want to support the
>> legacy boot path in the future measured SEV boot, we leave
>> X86QemuLoadImageLib as-is (except for a comment addition in patch 3) and
>> will force use for GenericQemuLoadImageLib in the measured SEV boot
>> implementation.
>>
>> Relevant discussion threads start in:
>> https://edk2.groups.io/g/devel/message/76069
>>
>> To test this on x86_64, I forced the use of GenericQemuLoadImageLib
>> using the following local patch:
>>
>>
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index 0a237a905866..46442b543bcf 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -404,7 +404,7 @@ [LibraryClasses.common.DXE_DRIVER]
>> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>> MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
>> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>> - QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>> + QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf # XXX don't commit this or someone will be mad
>> !if $(TPM_ENABLE) == TRUE
>> Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf
>> Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
>>
>>
>> I tested boot with QEMU and OVMF with the following QEMU arguments:
>>
>> -kernel a
>> -kernel a -initrd b
>> -kernel a -cmdline c
>> -kernel a -initrd b -cmdline c
>>
>> (and also without -kernel)
>>
>>
>> Code is at
>> https://github.com/confidential-containers-demo/edk2/tree/use-synthetic-fs-for-cmdline-v2
>>
>> v2 changes:
>>
>> - Add comment to header of X86QemuLoadImageLib.inf
>> - Clearer function names in GenericQemuLoadImageLib.c
>> - Fix coding style issues
>>
>> v1: https://edk2.groups.io/g/devel/message/76265
>>
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
>>
>> Dov Murik (3):
>> Revert "OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command
>> line"
>> OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs
>> OvmfPkg/X86QemuLoadImageLib: State fw_cfg dependency in file header
>>
>
>
> Please cc me on the entire series.
>
Sorry, my bad. Resent.
-Dov
>
>> OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 2 +-
>> OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf | 3 +
>> OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 145 ++++++++++++++++++--
>> OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 3 +
>> OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 11 +-
>> 5 files changed, 147 insertions(+), 17 deletions(-)
>>
>> --
>> 2.25.1
>>
>>
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread