[edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg

Dov Murik posted 11 patches 4 years, 7 months ago
There is a newer version of this series
[edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Dov Murik 4 years, 7 months ago
From: James Bottomley <jejb@linux.ibm.com>

Support QEMU's -kernel option.

OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c is an exact copy
of OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c .

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ashish Kalra <ashish.kalra@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3457
Signed-off-by: James Bottomley <jejb@linux.ibm.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc                                                        |  1 +
 OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf           |  2 ++
 OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h                            | 11 +++++++++++
 OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c                            |  5 +++++
 OvmfPkg/Library/{PlatformBootManagerLib => PlatformBootManagerLibGrub}/QemuKernel.c |  0
 5 files changed, 19 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index a2f1324c40a6..aefdcf881c99 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -281,6 +281,7 @@ [LibraryClasses.common.PEIM]
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
+  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
   PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
 
diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf b/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
index 9a806d17ec45..5f6f73d18470 100644
--- a/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
@@ -23,6 +23,7 @@ [Defines]
 
 [Sources]
   BdsPlatform.c
+  QemuKernel.c
   PlatformData.c
   BdsPlatform.h
 
@@ -46,6 +47,7 @@ [LibraryClasses]
   BootLogoLib
   DevicePathLib
   PciLib
+  QemuLoadImageLib
   UefiLib
   PlatformBmPrintScLib
   Tcg2PhysicalPresenceLib
diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
index 748c63081920..f1d3a2906c00 100644
--- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
+++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
@@ -172,4 +172,15 @@ PlatformInitializeConsole (
   IN PLATFORM_CONSOLE_CONNECT_ENTRY   *PlatformConsole
   );
 
+/**
+  Loads and boots UEFI Linux via the FwCfg interface.
+
+  @retval    EFI_NOT_FOUND - The Linux kernel was not found
+
+**/
+EFI_STATUS
+TryRunningQemuKernel (
+  VOID
+  );
+
 #endif // _PLATFORM_SPECIFIC_BDS_PLATFORM_H_
diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
index 5c92d4fc2b09..7cceeea4879c 100644
--- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
@@ -1315,6 +1315,11 @@ PlatformBootManagerAfterConsole (
   //
   Tcg2PhysicalPresenceLibProcessRequest (NULL);
 
+  //
+  // Process QEMU's -kernel command line option
+  //
+  TryRunningQemuKernel ();
+
   //
   // Perform some platform specific connect sequence
   //
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c b/OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
similarity index 100%
copy from OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
copy to OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77506): https://edk2.groups.io/g/devel/message/77506
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Brijesh Singh via groups.io 4 years, 6 months ago
On 7/6/21 3:54 AM, Dov Murik wrote:
> From: James Bottomley <jejb@linux.ibm.com>
>
> Support QEMU's -kernel option.
>
> OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c is an exact copy
> of OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c .
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ashish Kalra <ashish.kalra@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3457
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>

thanks


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77849): https://edk2.groups.io/g/devel/message/77849
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Christoph Willing 4 years, 6 months ago
I have been using Qemu with -kernel,-initrd,-append options without OVMF for some time in my workflow. When booting with OVMF, I have found that the most recent tag in the edk2 public git repo that supports those options is vUDK2018. Therefore I'm extremely interested in this series of patches, hoping they'll restore the missing functionality, and would like to test them locally. Is there any location from where I could obtain the patches themselves or, better still, access to a repo somewhere that already has them applied?

Thanks,
chris


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77893): https://edk2.groups.io/g/devel/message/77893
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Dov Murik 4 years, 6 months ago
Chris,

On 19/07/2021 7:46, Christoph Willing wrote:
> I have been using Qemu with -kernel,-initrd,-append options without OVMF for some time in my workflow. When booting with OVMF, I have found that the most recent tag in the edk2 public git repo that supports those options is vUDK2018. Therefore I'm extremely interested in this series of patches, hoping they'll restore the missing functionality, and would like to test them locally. Is there any location from where I could obtain the patches themselves or, better still, access to a repo somewhere that already has them applied?
> 

Are you trying to boot a VM with AMD SEV? This patch series is all about enabling
these switches for SEV (memory-encrypted VMs).  For "normal" VMs there should
be no change.

If you indeed use SEV guests, this patch series is available in 
https://github.com/confidential-containers-demo/edk2/tree/sev-hashes-v2

but also requires the corresponding QEMU patches in
https://lore.kernel.org/qemu-devel/20210624102040.2015280-1-dovmurik@linux.ibm.com/ 
which are not yet upstream.


-Dov


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77891): https://edk2.groups.io/g/devel/message/77891
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Christoph Willing 4 years, 6 months ago
Thanks for the clarification Dov.

I've been trying with just "normal" VMs, not SEV. I did already find and try the confidential-containers-demo sev-hashes-v2 branch but it didn't help - not surprising if it's not relevant to normal VMs.

Do you know whether this functionality (-kernel, -initrd, -append options) is actually supposed to work in normal VMs at the moment? The only conditions under which it works here with qemu-6.0.0 is with vUDK2017 & 2018 and an old ovmf binary package from kraxel.og dated 2017. Anything built from the edk2 master branch has failed when using those qemu options, although all the same builds work perfectly using the VMs' internal kernels & initrds. I've also extracted OVMF files from the current kraxel.org package as well as Ubuntu's (hirsute) package and these also fail the same way i.e. kernel boots and initrd works (loads modules) but then the VM filesystem doesn't seem to be found (no /dev/sdX exists to mount the filesystem root).

I guess this could be a qemu problem but since it works with some (old) udk/edk2 versions, I thought I'd look here first.

Thanks for any help or pointers,
chris


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77896): https://edk2.groups.io/g/devel/message/77896
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Dov Murik 4 years, 6 months ago

On 19/07/2021 15:56, Christoph Willing wrote:
> Thanks for the clarification Dov.
> 
> I've been trying with just "normal" VMs, not SEV. I did already find and try the confidential-containers-demo sev-hashes-v2 branch but it didn't help - not surprising if it's not relevant to normal VMs.
> 
> Do you know whether this functionality (-kernel, -initrd, -append options) is actually supposed to work in normal VMs at the moment? The only conditions under which it works here with qemu-6.0.0 is with vUDK2017 & 2018 and an old ovmf binary package from kraxel.og dated 2017. Anything built from the edk2 master branch has failed when using those qemu options, although all the same builds work perfectly using the VMs' internal kernels & initrds. I've also extracted OVMF files from the current kraxel.org package as well as Ubuntu's (hirsute) package and these also fail the same way i.e. kernel boots and initrd works (loads modules) but then the VM filesystem doesn't seem to be found (no /dev/sdX exists to mount the filesystem root).
> 
> I guess this could be a qemu problem but since it works with some (old) udk/edk2 versions, I thought I'd look here first.
> 


Can you please try with edk2 commit d1fc3d7ef3cb - just before we did
some changes around this QEMU-interop code in OVMF?

Thanks,
Dov

> Thanks for any help or pointers,
> chris
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77914): https://edk2.groups.io/g/devel/message/77914
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Christoph Willing 4 years, 6 months ago
On 20/7/21 3:58 am, Dov Murik wrote:
> 
> 
> On 19/07/2021 15:56, Christoph Willing wrote:
>> Thanks for the clarification Dov.
>>
>> I've been trying with just "normal" VMs, not SEV. I did already find and try the confidential-containers-demo sev-hashes-v2 branch but it didn't help - not surprising if it's not relevant to normal VMs.
>>
>> Do you know whether this functionality (-kernel, -initrd, -append options) is actually supposed to work in normal VMs at the moment? The only conditions under which it works here with qemu-6.0.0 is with vUDK2017 & 2018 and an old ovmf binary package from kraxel.og dated 2017. Anything built from the edk2 master branch has failed when using those qemu options, although all the same builds work perfectly using the VMs' internal kernels & initrds. I've also extracted OVMF files from the current kraxel.org package as well as Ubuntu's (hirsute) package and these also fail the same way i.e. kernel boots and initrd works (loads modules) but then the VM filesystem doesn't seem to be found (no /dev/sdX exists to mount the filesystem root).
>>
>> I guess this could be a qemu problem but since it works with some (old) udk/edk2 versions, I thought I'd look here first.
>>
> 
> 
> Can you please try with edk2 commit d1fc3d7ef3cb - just before we did
> some changes around this QEMU-interop code in OVMF?
> 

I just tried a build at d1fc3d7ef3cb... with the same result. Works with
VM's internal kernel & initrd but not with external (using -kernel,
-initrd & -append options).

As soon as I revert to OVMF files (CODE & VARS) from vUDK2018, all works
as expected with external kernel & initrd.

Since this problem seems to go back to around 2018, is it better to
report in bugzilla?

Thanks,
chris



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77929): https://edk2.groups.io/g/devel/message/77929
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Dov Murik 4 years, 6 months ago

On 20/07/2021 1:36, Christoph Willing wrote:
> On 20/7/21 3:58 am, Dov Murik wrote:
>>
>>
>> On 19/07/2021 15:56, Christoph Willing wrote:
>>> Thanks for the clarification Dov.
>>>
>>> I've been trying with just "normal" VMs, not SEV. I did already find and try the confidential-containers-demo sev-hashes-v2 branch but it didn't help - not surprising if it's not relevant to normal VMs.
>>>
>>> Do you know whether this functionality (-kernel, -initrd, -append options) is actually supposed to work in normal VMs at the moment? The only conditions under which it works here with qemu-6.0.0 is with vUDK2017 & 2018 and an old ovmf binary package from kraxel.og dated 2017. Anything built from the edk2 master branch has failed when using those qemu options, although all the same builds work perfectly using the VMs' internal kernels & initrds. I've also extracted OVMF files from the current kraxel.org package as well as Ubuntu's (hirsute) package and these also fail the same way i.e. kernel boots and initrd works (loads modules) but then the VM filesystem doesn't seem to be found (no /dev/sdX exists to mount the filesystem root).
>>>
>>> I guess this could be a qemu problem but since it works with some (old) udk/edk2 versions, I thought I'd look here first.
>>>
>>
>>
>> Can you please try with edk2 commit d1fc3d7ef3cb - just before we did
>> some changes around this QEMU-interop code in OVMF?
>>
> 
> I just tried a build at d1fc3d7ef3cb... with the same result. Works with
> VM's internal kernel & initrd but not with external (using -kernel,
> -initrd & -append options).
> 
> As soon as I revert to OVMF files (CODE & VARS) from vUDK2018, all works
> as expected with external kernel & initrd.
> 
> Since this problem seems to go back to around 2018, is it better to
> report in bugzilla?

I think so.

Be sure to include full logs as much as possible and details about the
image you're trying to start; it seems to me that if the kernel starts
and initrd is mounted etc then both QEMU and OVMF are doing their part,
and there's something else that fails (but then again, reverting to an
old OVMF does solve it... IDK).

-Dov

> 
> Thanks,
> chris
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77945): https://edk2.groups.io/g/devel/message/77945
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Lendacky, Thomas via groups.io 4 years, 6 months ago
On 7/6/21 3:54 AM, Dov Murik wrote:
> From: James Bottomley <jejb@linux.ibm.com>
> 
> Support QEMU's -kernel option.
> 
> OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c is an exact copy
> of OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c .

Just a nit, but this confused me initially. Maybe it should say something
along the lines of create a QemuKernel.c for PlatformBootManagerLibGrub
that is an exact copy of the file from PlatformBootManagerLib.

Is there any way that the two libraries can use the same file rather than
making an exact copy?

Thanks,
Tom

> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ashish Kalra <ashish.kalra@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3457
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> ---
>  OvmfPkg/AmdSev/AmdSevX64.dsc                                                        |  1 +
>  OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf           |  2 ++
>  OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h                            | 11 +++++++++++
>  OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c                            |  5 +++++
>  OvmfPkg/Library/{PlatformBootManagerLib => PlatformBootManagerLibGrub}/QemuKernel.c |  0
>  5 files changed, 19 insertions(+)
> 
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index a2f1324c40a6..aefdcf881c99 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -281,6 +281,7 @@ [LibraryClasses.common.PEIM]
>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
> +  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>    PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>    QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>  
> diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf b/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
> index 9a806d17ec45..5f6f73d18470 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
> +++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
> @@ -23,6 +23,7 @@ [Defines]
>  
>  [Sources]
>    BdsPlatform.c
> +  QemuKernel.c
>    PlatformData.c
>    BdsPlatform.h
>  
> @@ -46,6 +47,7 @@ [LibraryClasses]
>    BootLogoLib
>    DevicePathLib
>    PciLib
> +  QemuLoadImageLib
>    UefiLib
>    PlatformBmPrintScLib
>    Tcg2PhysicalPresenceLib
> diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
> index 748c63081920..f1d3a2906c00 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
> +++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
> @@ -172,4 +172,15 @@ PlatformInitializeConsole (
>    IN PLATFORM_CONSOLE_CONNECT_ENTRY   *PlatformConsole
>    );
>  
> +/**
> +  Loads and boots UEFI Linux via the FwCfg interface.
> +
> +  @retval    EFI_NOT_FOUND - The Linux kernel was not found
> +
> +**/
> +EFI_STATUS
> +TryRunningQemuKernel (
> +  VOID
> +  );
> +
>  #endif // _PLATFORM_SPECIFIC_BDS_PLATFORM_H_
> diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
> index 5c92d4fc2b09..7cceeea4879c 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
> @@ -1315,6 +1315,11 @@ PlatformBootManagerAfterConsole (
>    //
>    Tcg2PhysicalPresenceLibProcessRequest (NULL);
>  
> +  //
> +  // Process QEMU's -kernel command line option
> +  //
> +  TryRunningQemuKernel ();
> +
>    //
>    // Perform some platform specific connect sequence
>    //
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c b/OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
> similarity index 100%
> copy from OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
> copy to OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77901): https://edk2.groups.io/g/devel/message/77901
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Dov Murik 4 years, 6 months ago

On 19/07/2021 18:21, Tom Lendacky wrote:
> On 7/6/21 3:54 AM, Dov Murik wrote:
>> From: James Bottomley <jejb@linux.ibm.com>
>>
>> Support QEMU's -kernel option.
>>
>> OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c is an exact copy
>> of OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c .
> 
> Just a nit, but this confused me initially. Maybe it should say something
> along the lines of create a QemuKernel.c for PlatformBootManagerLibGrub
> that is an exact copy of the file from PlatformBootManagerLib.
> 

You're right; I'll write it clearer.


> Is there any way that the two libraries can use the same file rather than
> making an exact copy?

I guess it's possible by extracting the file into its own library?  I'll
need to take a deeper look.

-Dov


> 
> Thanks,
> Tom
> 
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ashish Kalra <ashish.kalra@amd.com>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Erdem Aktas <erdemaktas@google.com>
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3457
>> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
>> ---
>>  OvmfPkg/AmdSev/AmdSevX64.dsc                                                        |  1 +
>>  OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf           |  2 ++
>>  OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h                            | 11 +++++++++++
>>  OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c                            |  5 +++++
>>  OvmfPkg/Library/{PlatformBootManagerLib => PlatformBootManagerLibGrub}/QemuKernel.c |  0
>>  5 files changed, 19 insertions(+)
>>
>> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
>> index a2f1324c40a6..aefdcf881c99 100644
>> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
>> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
>> @@ -281,6 +281,7 @@ [LibraryClasses.common.PEIM]
>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
>>    MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
>> +  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>>    PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>>    QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>>  
>> diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf b/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
>> index 9a806d17ec45..5f6f73d18470 100644
>> --- a/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
>> +++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
>> @@ -23,6 +23,7 @@ [Defines]
>>  
>>  [Sources]
>>    BdsPlatform.c
>> +  QemuKernel.c
>>    PlatformData.c
>>    BdsPlatform.h
>>  
>> @@ -46,6 +47,7 @@ [LibraryClasses]
>>    BootLogoLib
>>    DevicePathLib
>>    PciLib
>> +  QemuLoadImageLib
>>    UefiLib
>>    PlatformBmPrintScLib
>>    Tcg2PhysicalPresenceLib
>> diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
>> index 748c63081920..f1d3a2906c00 100644
>> --- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
>> +++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h
>> @@ -172,4 +172,15 @@ PlatformInitializeConsole (
>>    IN PLATFORM_CONSOLE_CONNECT_ENTRY   *PlatformConsole
>>    );
>>  
>> +/**
>> +  Loads and boots UEFI Linux via the FwCfg interface.
>> +
>> +  @retval    EFI_NOT_FOUND - The Linux kernel was not found
>> +
>> +**/
>> +EFI_STATUS
>> +TryRunningQemuKernel (
>> +  VOID
>> +  );
>> +
>>  #endif // _PLATFORM_SPECIFIC_BDS_PLATFORM_H_
>> diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
>> index 5c92d4fc2b09..7cceeea4879c 100644
>> --- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
>> +++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
>> @@ -1315,6 +1315,11 @@ PlatformBootManagerAfterConsole (
>>    //
>>    Tcg2PhysicalPresenceLibProcessRequest (NULL);
>>  
>> +  //
>> +  // Process QEMU's -kernel command line option
>> +  //
>> +  TryRunningQemuKernel ();
>> +
>>    //
>>    // Perform some platform specific connect sequence
>>    //
>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c b/OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
>> similarity index 100%
>> copy from OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
>> copy to OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77916): https://edk2.groups.io/g/devel/message/77916
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Dov Murik 4 years, 6 months ago

On 19/07/2021 22:14, Dov Murik wrote:
> 
> 
> On 19/07/2021 18:21, Tom Lendacky wrote:
>> On 7/6/21 3:54 AM, Dov Murik wrote:
>>> From: James Bottomley <jejb@linux.ibm.com>
>>>
>>> Support QEMU's -kernel option.
>>>
>>> OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c is an exact copy
>>> of OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c .
>>
>> Just a nit, but this confused me initially. Maybe it should say something
>> along the lines of create a QemuKernel.c for PlatformBootManagerLibGrub
>> that is an exact copy of the file from PlatformBootManagerLib.
>>
> 
> You're right; I'll write it clearer.
> 
> 
>> Is there any way that the two libraries can use the same file rather than
>> making an exact copy?
> 
> I guess it's possible by extracting the file into its own library?  I'll
> need to take a deeper look.
> 


With this patch we'll have two identical files:

  OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
  OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c

but there's another QemuKernel.c, which is *almost* identical:

  ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c

so a proper fix should consolidate all three into one library used by
all three libs.

I suggest postponing this to a separate refactoring series.

Thanks,
-Dov




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77951): https://edk2.groups.io/g/devel/message/77951
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 03/11] OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
Posted by Ard Biesheuvel 4 years, 6 months ago
On Tue, 20 Jul 2021 at 09:33, Dov Murik <dovmurik@linux.ibm.com> wrote:
>
>
>
> On 19/07/2021 22:14, Dov Murik wrote:
> >
> >
> > On 19/07/2021 18:21, Tom Lendacky wrote:
> >> On 7/6/21 3:54 AM, Dov Murik wrote:
> >>> From: James Bottomley <jejb@linux.ibm.com>
> >>>
> >>> Support QEMU's -kernel option.
> >>>
> >>> OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c is an exact copy
> >>> of OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c .
> >>
> >> Just a nit, but this confused me initially. Maybe it should say something
> >> along the lines of create a QemuKernel.c for PlatformBootManagerLibGrub
> >> that is an exact copy of the file from PlatformBootManagerLib.
> >>
> >
> > You're right; I'll write it clearer.
> >
> >
> >> Is there any way that the two libraries can use the same file rather than
> >> making an exact copy?
> >
> > I guess it's possible by extracting the file into its own library?  I'll
> > need to take a deeper look.
> >
>
>
> With this patch we'll have two identical files:
>
>   OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
>   OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
>
> but there's another QemuKernel.c, which is *almost* identical:
>
>   ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
>
> so a proper fix should consolidate all three into one library used by
> all three libs.
>
> I suggest postponing this to a separate refactoring series.
>

That is fine with me.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77953): https://edk2.groups.io/g/devel/message/77953
Mute This Topic: https://groups.io/mt/84016356/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-