From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web10.11269.1583408278802930493 for ; Thu, 05 Mar 2020 03:37:59 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=odo2ZLI4; spf=pass (domain: linaro.org, ip: 209.85.221.65, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f65.google.com with SMTP id 6so1165131wre.4 for ; Thu, 05 Mar 2020 03:37:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hjQ8i0zlqK8uGd8k41dKW6Sc7xSYeqcBzbkJsCA8vPg=; b=odo2ZLI41/5kkOuyFYehR+cZsu2vJZ6DdElsOlc4brwdVRe1ceR40MC/wlitXJp/fE jhxWg2e+LDvIcQeLdtFO+IybK2/xn92CaNq+R9ASsPvOgz1/rdNEtTNOE865TR2wdPw7 iD43lqaSmTgI3SkeBRZ5tJHNM8CsTKcXbmUXk7ydFkskbcb0jEy/I2kxHCCzpKnd/6Pf 1z4M6zDBmX7QoD2qStQd34X7oBvEp3/6eFpYtD5ifS8oqH/sovtszSa+Nh0G1zDSqIlq JSzqHSjqu62H67dpS0ScXRzSfqp3ig5ukH5bCzt1Q5n8sdIGRhb+KRqAyZcSzfE9Uj6h zxHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hjQ8i0zlqK8uGd8k41dKW6Sc7xSYeqcBzbkJsCA8vPg=; b=irWiQHo7DhHnV8Gw5fIlXMV9aukhgJLG0w+6MOHHQw0UijAksjYgolAj5/4D4B1mOH BAGRHn/CsQzcLpsQ0tVzLgnXZiGLqla5TYdLOXygczz/jA/ICv+ZG6KTsLuB3jGYe0ur NJA+w8taioW7UemIPCkmmW8R5xxXrsHNhzxFJWRwNmjz6lWJI5S4vTBiTfeTC5gIn24U jGJWnBUyXZnUj7PmMHNcfD3r9tlk/XzCbmJE7Z1fYbdlTRlySAEz7ftUvXKykeajHDkT 8Ais3t2JPR/D2Hv+XD9LVvsNpBA6cE1+IksAnF1wtim53y3bUCIhahijVe0YOwpZYkBu MxBg== X-Gm-Message-State: ANhLgQ3H13M4TXfv2HQt97kOsMSRoK1nAhb17u7nbNgieqoIQfAdXrXz NWWNBXw2O7OHwp4g0CESrLdLO74nQ2UCjVB7ao66YQ== X-Google-Smtp-Source: ADFU+vvTH5/oVn8euCVu5j+GrgLNmZ1lc7xieM/MAfNHmEjU9/ZOK1IARRWivK8hU3JcoxOUv3Sx4I+oxbR5hn3mxNo= X-Received: by 2002:adf:f84a:: with SMTP id d10mr10021715wrq.208.1583408276922; Thu, 05 Mar 2020 03:37:56 -0800 (PST) MIME-Version: 1.0 References: <20200304095233.21046-1-ard.biesheuvel@linaro.org> <20200304095233.21046-5-ard.biesheuvel@linaro.org> <1da5be78-4fe4-a672-9f57-d029a99cc606@redhat.com> <6c1d2510-7714-9eb7-c99e-bc5f59b411ac@redhat.com> In-Reply-To: <6c1d2510-7714-9eb7-c99e-bc5f59b411ac@redhat.com> From: "Ard Biesheuvel" Date: Thu, 5 Mar 2020 12:37:46 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 04/14] OvmfPkg: provide a generic implementation of QemuLoadImageLib To: Laszlo Ersek Cc: edk2-devel-groups-io Content-Type: text/plain; charset="UTF-8" On Thu, 5 Mar 2020 at 12:29, Laszlo Ersek 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 > >> --- > >> OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 278 ++++++++++++++++++++ > >> OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 38 +++ > >> 2 files changed, 316 insertions(+) > > > > Reviewed-by: Laszlo Ersek > > 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.
> >> + > >> + SPDX-License-Identifier: BSD-2-Clause-Patent > >> +**/ > >> + > >> +#include > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +#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 :-)