From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web09.10679.1583404847634155714 for ; Thu, 05 Mar 2020 02:40:48 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=kWASNH77; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f66.google.com with SMTP id z15so6391895wrl.1 for ; Thu, 05 Mar 2020 02:40:47 -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; bh=1NQ5F39fHuxYhGBkzIARVC9X7glPrF5UU5lb3OEztpw=; b=kWASNH77JVHEM+8ulvilCEVs1ST4I+fQ2Rs0wJc3RxsjkXIa73c9YOMsovFH6AQuD4 NONU0x0ZaGDw+5p/Ek6ZCWrvYMXKpsgK77YuXgcNprWurQ0i2O0h8FYnZdx9/SjhlSki ho5NeK4s5WnREe4wVAj/CA4a+7lETw8mPwfhbGpmWaoqo/wTQampBJm3T8IuB/A42+C/ yLGZBhBzPTLEADmqytczBRBvy8X6eqjLg+koYB/UgNraJfD9Sc6LCc8yXYP9djMXGae0 hBfoK5bJ8bbH6X3u2QgK0fgPfCDd8WiQaGbgDGb/xCzUnhs3X31KWH8ouFlqNTINqfhw lB4w== 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; bh=1NQ5F39fHuxYhGBkzIARVC9X7glPrF5UU5lb3OEztpw=; b=P1slyzeEBoqmW+SBgAtesGhvuULIj1jZS0iVC6qgikpEhPzBug6ugFSkwF5xzlVJWD cJfoevHVabeCYQPzAdPM9F/sDQMMJYBB9TAY5dXG04bz4aifXKL1rxzVRQQQo7FfYeKq 5eHScvSI1O+u1Vq+HbiTM6BsRpVv6yjcP9bBamg99zNeVO4qMXev8pubbx0HIK0x0oBP L8ovde+zU7z85eXD9cj1WbVJRUIFyI3Axng14huXRQaXHP/TolJ9lMXlkGg7qW/M1SAm JHihT/dgdGGe9T2lVjPhhuBTtu+SUL5Cb+n2E6dYJ8kOC6wUCzxQ7DvzxHYD9+q/0nrq CUeA== X-Gm-Message-State: ANhLgQ26C2YH2N2XRzqqXeEZNFOZ86Ls218D2CLCevRw+rvaLk6sD2Ly HqRLKbI8XZQy/e5JwRTYVykuiTOxqeiyZeJDfrvxNTY0IADbTw== X-Google-Smtp-Source: ADFU+vsZQRae0BPqsQMoH1cqZlHlH15McvI4OUnSa2f3jPnWzCcTMQxJbc4Baf0eZAk46v6knQNKWhrLyzISfZ9SNr0= X-Received: by 2002:a05:6000:110b:: with SMTP id z11mr9698609wrw.252.1583404845655; Thu, 05 Mar 2020 02:40:45 -0800 (PST) MIME-Version: 1.0 References: <20200304095233.21046-1-ard.biesheuvel@linaro.org> <20200304095233.21046-10-ard.biesheuvel@linaro.org> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 5 Mar 2020 11:40:34 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 09/14] OvmfPkg: create protocol and GUID header for legacy loaded images To: edk2-devel-groups-io , Laszlo Ersek Content-Type: text/plain; charset="UTF-8" On Thu, 5 Mar 2020 at 11:31, Laszlo Ersek wrote: > > On 03/04/20 10:52, Ard Biesheuvel wrote: > > In preparation of moving the legacy x86 loading to an implementation > > of the QEMU load image library class, introduce a protocol header > > and GUID that we will use to identify legacy loaded images in the > > protocol database. > > > > Signed-off-by: Ard Biesheuvel > > --- > > OvmfPkg/Include/Protocol/X86QemuKernelLoadedImage.h | 19 +++++++++++++++++++ > > OvmfPkg/OvmfPkg.dec | 1 + > > 2 files changed, 20 insertions(+) > > > > diff --git a/OvmfPkg/Include/Protocol/X86QemuKernelLoadedImage.h b/OvmfPkg/Include/Protocol/X86QemuKernelLoadedImage.h > > new file mode 100644 > > index 000000000000..7e1bebaa6a07 > > --- /dev/null > > +++ b/OvmfPkg/Include/Protocol/X86QemuKernelLoadedImage.h > > @@ -0,0 +1,19 @@ > > +/** @file > > + Protocol/GUID definition to describe a kernel image loaded by the legacy X86 > > + loader from the file specified on the QEMU command line via the -kernel > > + option. > > (1) Please add a comment that the interface structure associated with > this protocol GUID is subject to change, and should not be used outside > of the EDK II tree. > > (I'm requesting this comment regardless of point (5) below.) > Now that we're bikeshedding: :-) Given the internal nature of this protocol, perhaps the name should reflect it? And if we're changing it, perhaps make it more precise? It is internal to OVMF so gOvmfLoadedX86LinuxKernelProtocolGuid and for the type OVMF_LOADED_X86_LINUX_KERNEL (given that the only thing it can represent is a loaded x86 Linux kernel, but that is not specific to QEMU in principle) This ignores the initrd aspect as well as the command line, but EFI's loaded image subsumes the command line as well, so I think that is fine. > > + > > + Copyright (c) 2020, Arm, Ltd. All rights reserved.
> > + > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > +**/ > > + > > +#ifndef X86_QEMU_KERNEL_LOADED_IMAGE_GUID_H__ > > +#define X86_QEMU_KERNEL_LOADED_IMAGE_GUID_H__ > > (2) Please drop "_GUID" from the guard macro's name. > > > + > > +#define X86_QEMU_KERNEL_LOADED_IMAGE_GUID \ > > + {0xa3edc05d, 0xb618, 0x4ff6, {0x95, 0x52, 0x76, 0xd7, 0x88, 0x63, 0x43, 0xc8}} > > (3) Please replace "_GUID" with "_PROTOCOL_GUID" in the initializer > macro's name. > > > + > > +extern EFI_GUID gX86QemuKernelLoadedImageGuid; > > (4) Please replace "Guid" with "ProtocolGuid" in the variable name. > > > + > > +#endif > > (5) Please consider moving the QEMU_LEGACY_LOADED_IMAGE typedef here, > from the next patch. It's not a technical necessity at all, but edk2 > protocol headers always include the interface typedef too. And due to > (1) above, I think it's safe too. > > If you don't like the idea, I won't insist. :) > > > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > > index 055caaa43041..06ffd4198d44 100644 > > --- a/OvmfPkg/OvmfPkg.dec > > +++ b/OvmfPkg/OvmfPkg.dec > > @@ -112,6 +112,7 @@ [Protocols] > > gEfiLegacyBiosPlatformProtocolGuid = {0x783658a3, 0x4172, 0x4421, {0xa2, 0x99, 0xe0, 0x09, 0x07, 0x9c, 0x0c, 0xb4}} > > gEfiLegacyInterruptProtocolGuid = {0x31ce593d, 0x108a, 0x485d, {0xad, 0xb2, 0x78, 0xf2, 0x1f, 0x29, 0x66, 0xbe}} > > gEfiVgaMiniPortProtocolGuid = {0xc7735a2f, 0x88f5, 0x4882, {0xae, 0x63, 0xfa, 0xac, 0x8c, 0x8b, 0x86, 0xb3}} > > + gX86QemuKernelLoadedImageGuid = {0xa3edc05d, 0xb618, 0x4ff6, {0x95, 0x52, 0x76, 0xd7, 0x88, 0x63, 0x43, 0xc8}} > > (6) Same as (4). > > > > > [PcdsFixedAtBuild] > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|0x0|UINT32|0 > > > > With (1)-(4) and (6) fixed, and with (5) optionally implemented: > > Reviewed-by: Laszlo Ersek > > Thanks! > Laszlo > > > >