On 03/03/20 11:18, Ard Biesheuvel wrote: > On Tue, 3 Mar 2020 at 11:10, Laszlo Ersek <ler...@redhat.com> wrote: >> >> On 03/02/20 08:29, Ard Biesheuvel wrote: >>> Linux v5.7 will introduce a new method to load the initial ramdisk >>> (initrd) from the loader, using the LoadFile2 protocol installed on a >>> special vendor GUIDed media device path. >>> >>> Add support for this to our QEMU command line kernel/initrd loader. >>> >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 >>> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> >>> --- >>> OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 79 >>> ++++++++++++++++++++ >>> OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf | 2 + >>> 2 files changed, 81 insertions(+) >>> >>> diff --git a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c >>> b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c >>> index 77d8fedb738a..415946edf22a 100644 >>> --- a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c >>> +++ b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c >>> @@ -13,15 +13,18 @@ >>> #include <Guid/FileInfo.h> >>> #include <Guid/FileSystemInfo.h> >>> #include <Guid/FileSystemVolumeLabelInfo.h> >>> +#include <Guid/LinuxEfiInitrdMedia.h> >>> #include <Guid/QemuKernelLoaderFsMedia.h> >>> #include <Library/BaseLib.h> >>> #include <Library/BaseMemoryLib.h> >>> #include <Library/DebugLib.h> >>> +#include <Library/DevicePathLib.h> >>> #include <Library/MemoryAllocationLib.h> >>> #include <Library/QemuFwCfgLib.h> >>> #include <Library/UefiBootServicesTableLib.h> >>> #include <Library/UefiRuntimeServicesTableLib.h> >>> #include <Protocol/DevicePath.h> >>> +#include <Protocol/LoadFile2.h> >>> #include <Protocol/SimpleFileSystem.h> >>> >>> // >>> @@ -84,6 +87,19 @@ STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH >>> mFileSystemDevicePath = { >>> } >>> }; >>> >>> +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH mInitrdDevicePath = { >>> + { >>> + { >>> + MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP, >>> + { sizeof (VENDOR_DEVICE_PATH) } >>> + }, >>> + LINUX_EFI_INITRD_MEDIA_GUID >>> + }, { >>> + END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE, >>> + { sizeof (EFI_DEVICE_PATH_PROTOCOL) } >>> + } >>> +}; >>> + >>> // >>> // The "file in the EFI stub filesystem" abstraction. >>> // >>> @@ -839,6 +855,48 @@ STATIC CONST EFI_SIMPLE_FILE_SYSTEM_PROTOCOL >>> mFileSystem = { >>> StubFileSystemOpenVolume >>> }; >>> >>> +STATIC >>> +EFI_STATUS >>> +EFIAPI >>> +InitrdLoadFile2 ( >>> + IN EFI_LOAD_FILE2_PROTOCOL *This, >>> + IN EFI_DEVICE_PATH_PROTOCOL *FilePath, >>> + IN BOOLEAN BootPolicy, >>> + IN OUT UINTN *BufferSize, >>> + IN VOID *Buffer OPTIONAL >>> + ) >>> +{ >>> + CONST KERNEL_BLOB *InitrdBlob = &mKernelBlob[KernelBlobTypeInitrd]; >>> + >>> + ASSERT (InitrdBlob->Size > 0); >>> + >>> + if (BootPolicy) { >>> + return EFI_UNSUPPORTED; >>> + } >>> + >>> + if (BufferSize == NULL || !IsDevicePathValid (FilePath, 0)) { >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + if (FilePath->Type != END_DEVICE_PATH_TYPE || >>> + FilePath->SubType != END_ENTIRE_DEVICE_PATH_SUBTYPE) { >>> + return EFI_NOT_FOUND; >>> + } >>> + >>> + if (Buffer == NULL || *BufferSize < InitrdBlob->Size) { >>> + *BufferSize = InitrdBlob->Size; >>> + return EFI_BUFFER_TOO_SMALL; >>> + } >>> + >>> + CopyMem (Buffer, InitrdBlob->Data, InitrdBlob->Size); >>> + >>> + *BufferSize = InitrdBlob->Size; >>> + return EFI_SUCCESS; >>> +} >>> + >>> +STATIC CONST EFI_LOAD_FILE2_PROTOCOL mInitrdLoadFile2 = { >>> + InitrdLoadFile2, >>> +}; >>> >>> // >>> // Utility functions. >>> @@ -945,6 +1003,7 @@ QemuKernelLoaderFsDxeEntrypoint ( >>> KERNEL_BLOB *KernelBlob; >>> EFI_STATUS Status; >>> EFI_HANDLE FileSystemHandle; >>> + EFI_HANDLE InitrdLoadFile2Handle; >>> >>> if (!QemuFwCfgIsAvailable ()) { >>> return EFI_NOT_FOUND; >>> @@ -989,8 +1048,28 @@ QemuKernelLoaderFsDxeEntrypoint ( >>> goto FreeBlobs; >>> } >>> >>> + if (KernelBlob[KernelBlobTypeInitrd].Size > 0) { >>> + InitrdLoadFile2Handle = NULL; >>> + Status = gBS->InstallMultipleProtocolInterfaces >>> (&InitrdLoadFile2Handle, >>> + &gEfiDevicePathProtocolGuid, &mInitrdDevicePath, >>> + &gEfiLoadFile2ProtocolGuid, &mInitrdLoadFile2, >>> + NULL); >>> + if (EFI_ERROR (Status)) { >>> + DEBUG ((DEBUG_ERROR, "%a: InstallMultipleProtocolInterfaces(): %r\n", >>> + __FUNCTION__, Status)); >>> + goto UninstallFileSystemHandle; >>> + } >>> + } >>> + >>> return EFI_SUCCESS; >>> >>> +UninstallFileSystemHandle: >>> + Status = gBS->UninstallMultipleProtocolInterfaces (FileSystemHandle, >>> + &gEfiDevicePathProtocolGuid, >>> &mFileSystemDevicePath, >>> + &gEfiSimpleFileSystemProtocolGuid, &mFileSystem, >>> + NULL); >>> + ASSERT_EFI_ERROR (Status); >>> + >>> FreeBlobs: >>> while (BlobType > 0) { >>> CurrentBlob = &mKernelBlob[--BlobType]; >>> diff --git a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf >>> b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf >>> index f4b50c265027..737f9b87a7c7 100644 >>> --- a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf >>> +++ b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf >>> @@ -28,6 +28,7 @@ [LibraryClasses] >>> BaseLib >>> BaseMemoryLib >>> DebugLib >>> + DevicePathLib >>> MemoryAllocationLib >>> UefiBootServicesTableLib >>> QemuFwCfgLib >>> @@ -42,6 +43,7 @@ [Guids] >>> >>> [Protocols] >>> gEfiDevicePathProtocolGuid ## PRODUCES >>> + gEfiLoadFile2ProtocolGuid ## PRODUCES >>> gEfiSimpleFileSystemProtocolGuid ## PRODUCES >>> >>> [Depex] >>> >> >> I think this patch conflicts with your other patch: >> >> [edk2-devel] [PATCH v3 2/6] OvmfPkg: add 'initrd' shell command to >> expose Linux initrd via device path >> >> There should not be two different handles in the handle database that >> carry the same device path (I mean "same device path" by contents, not >> by reference). >> >> I believe that it's possible that the command-line specified -kernel / >> -initrd are attempted first, but the kernel's stub returns for some >> reason. Then the user can still go to the UEFI shell, and use the new >> dynamic shell command. In that case I think the device path will cease >> being unique (by contents). >> >> I think you can solve this as follows: >> >> - in both modules (this driver, and the dynamic shell command driver), >> install a NULL protocol interface with GUID = gEfiCallerIdGuid on the >> same handle that carries the device path and LoadFile2 >> >> - in both modules, before you touch the handle that carries the device >> path, try to locate it first. If it exists but doesn't carry the subject >> module's FILE_GUID as a NULL protocol interface, we know the device path >> was created by the "other" module, and we can bail. >> >> Just an idea. >> > > Thanks. > > I did wonder about the rules regarding duplicates in the device path > space. If the DXE core doesn't catch that, this seems like a gross > oversight to me. > > But it raises an interesting point: the idea is that any boot stage > could elect to provide this interface, not just UEFI or the shell > itself, but also shim, grub or whatever executes in between. That > means that, in general, it should probably be the responsibility of > the code that installs the protocol to ensure that it doesn't exist > already. > > For this code, it means it could simply install it, since it > guarantees to execute first. > > For the initrd implementation, it means we should check whether any > other implementation exists already, which could simply be done [in > that particular case] by locating the protocol and checking the > address against the statically allocated one in the driver. > > I am aware that both are DXE drivers, but since the dynamic shell > command is not invoked by any driver that is invoked before EndOfDxe, > I think the above should be sufficient. >
Sounds OK to me. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55315): https://edk2.groups.io/g/devel/message/55315 Mute This Topic: https://groups.io/mt/71669026/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
