On Thu, 5 Mar 2020 at 12:29, Laszlo Ersek <ler...@redhat.com> wrote: > > On 03/05/20 10:51, Laszlo Ersek wrote: > > On 03/04/20 10:52, Ard Biesheuvel wrote: > >> Implement QemuLoadImageLib, and make it load the image provided by the > >> QEMU_EFI_LOADER_FS_MEDIA_GUID/kernel device path that we implemented > >> in a preceding patch in a separate DXE driver, using only the standard > >> LoadImage and StartImage boot services. > >> > >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 > >> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > >> --- > >> OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 278 > >> ++++++++++++++++++++ > >> OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 38 > >> +++ > >> 2 files changed, 316 insertions(+) > > > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> > > One request though: > > > > >> diff --git > >> a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c > >> b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c > >> new file mode 100644 > >> index 000000000000..f5edb43cc0b9 > >> --- /dev/null > >> +++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c > >> @@ -0,0 +1,278 @@ > >> +/** @file > >> + Generic implementation of QemuLoadImageLib library class interface. > >> + > >> + Copyright (c) 2020, ARM Ltd. All rights reserved.<BR> > >> + > >> + SPDX-License-Identifier: BSD-2-Clause-Patent > >> +**/ > >> + > >> +#include <Uefi.h> > >> + > >> +#include <Base.h> > >> +#include <Guid/QemuKernelLoaderFsMedia.h> > >> +#include <Library/DebugLib.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> > >> +#include <Protocol/LoadedImage.h> > >> + > >> +#pragma pack (1) > >> +typedef struct { > >> + EFI_DEVICE_PATH_PROTOCOL FilePathHeader; > >> + CHAR16 FilePath[ARRAY_SIZE (L"kernel")]; > >> +} KERNEL_FILE_DEVPATH; > >> + > >> +typedef struct { > >> + VENDOR_DEVICE_PATH VenMediaNode; > >> + KERNEL_FILE_DEVPATH FileNode; > >> + EFI_DEVICE_PATH_PROTOCOL EndNode; > >> +} KERNEL_VENMEDIA_FILE_DEVPATH; > >> +#pragma pack () > >> + > >> +STATIC CONST KERNEL_VENMEDIA_FILE_DEVPATH mKernelDevicePath = { > >> + { > >> + { > >> + MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP, > >> + { sizeof (VENDOR_DEVICE_PATH) } > >> + }, > >> + QEMU_KERNEL_LOADER_FS_MEDIA_GUID > >> + }, { > >> + { > >> + MEDIA_DEVICE_PATH, MEDIA_FILEPATH_DP, > >> + { sizeof (KERNEL_FILE_DEVPATH) } > >> + }, > >> + L"kernel", > >> + }, { > >> + END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE, > >> + { sizeof (EFI_DEVICE_PATH_PROTOCOL) } > >> + } > >> +}; > >> + > >> +/** > >> + 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 > >> + the initrd from the same Simple FileSystem where the kernel was loaded > >> from. > >> + > >> + @param[out] ImageHandle The image handle that was allocated for > >> + loading the image > >> + @param[out] LoadedImage The loaded image protocol that was > >> installed > >> + on ImageHandle by the LoadImage boot > >> service. > > (1) Please remove this parameter. (I've noticed this now, after diffing > the two implementations of this function, including leading comments.) > > The R-b stands. >
Ah yes - that param went out of date a while ago :-) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55501): https://edk2.groups.io/g/devel/message/55501 Mute This Topic: https://groups.io/mt/71722797/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
