1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v2 00/14] Ovmf: use LoadImage/StartImage for loading command line images
  4. View patch

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

Ard Biesheuvel posted 14 patches 5 years, 11 months ago
There is a newer version of this series
[edk2-devel] [PATCH v2 09/14] OvmfPkg: create protocol and GUID header for legacy loaded images
Posted by Ard Biesheuvel 5 years, 11 months ago 
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.
+
+  Copyright (c) 2020, Arm, Ltd. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef X86_QEMU_KERNEL_LOADED_IMAGE_GUID_H__
+#define X86_QEMU_KERNEL_LOADED_IMAGE_GUID_H__
+
+#define X86_QEMU_KERNEL_LOADED_IMAGE_GUID \
+  {0xa3edc05d, 0xb618, 0x4ff6, {0x95, 0x52, 0x76, 0xd7, 0x88, 0x63, 0x43, 0xc8}}
+
+extern EFI_GUID gX86QemuKernelLoadedImageGuid;
+
+#endif
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}}
 
 [PcdsFixedAtBuild]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|0x0|UINT32|0
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55390): https://edk2.groups.io/g/devel/message/55390
Mute This Topic: https://groups.io/mt/71722803/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 09/14] OvmfPkg: create protocol and GUID header for legacy loaded images
Posted by Laszlo Ersek 5 years, 11 months ago 
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.)

> +
> +  Copyright (c) 2020, Arm, Ltd. All rights reserved.<BR>
> +
> +  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 <lersek@redhat.com>

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55497): https://edk2.groups.io/g/devel/message/55497
Mute This Topic: https://groups.io/mt/71722803/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 09/14] OvmfPkg: create protocol and GUID header for legacy loaded images
Posted by Ard Biesheuvel 5 years, 11 months ago 
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: :-)

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.<BR>
> > +
> > +  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 <lersek@redhat.com>
>
> Thanks!
> Laszlo
>
>
> 
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55498): https://edk2.groups.io/g/devel/message/55498
Mute This Topic: https://groups.io/mt/71722803/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 09/14] OvmfPkg: create protocol and GUID header for legacy loaded images
Posted by Laszlo Ersek 5 years, 11 months ago 
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55542): https://edk2.groups.io/g/devel/message/55542
Mute This Topic: https://groups.io/mt/71722803/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.