Re: [edk2-devel] [PATCH v2 09/14] OvmfPkg: create protocol and GUID header for legacy loaded images

["Laszlo Ersek" <lersek@redhat.com>]

On 03/05/20 11:40, Ard Biesheuvel wrote:
> On Thu, 5 Mar 2020 at 11:31, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org>
>>> ---
>>>  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: :-)

I don't feel we are :) All the things I've asked for are existing
practice in edk2. In particular (1) would mirror the note in
"VirtioDevice.h".

> 
> 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.

Great suggestions, I like them.

Thanks!
Laszlo