Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

["Sami Mujawar" <sami.mujawar@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 3828 bytes --]

Hi Jianyong,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 16/09/2022 03:46 am, Jianyong Wu wrote:
> As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.
>
> Signed-off-by: Jianyong Wu<jianyong.wu@arm.com>
> ---
>   ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-
>   ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +
>   .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -
>   3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
> index 7ca7a391d9..92ccd4ef12 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.dsc
> +++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
> @@ -37,13 +37,15 @@
>     # Virtio Support
>     VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>     VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
> +  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
[SAMI] How does this work for CloudHv?
> +  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>   
>     ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
>   
>     TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
>     CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>     BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> -  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

[SAMI] I believe this brings the dependency on QemuBootOrderLib which 
requires QemuFwCfgLib. Is there a way to to implement 
QemuBootOrderLibNull to remove the dependency on QemuFwCfgLib? The 
QemuBootOrderLib APIs 
ConnectDevicesFromQem(),StoreQemuBootOrder(),SetBootOrderFromQemu () and 
GetFrontPageTimeoutFromQemu ()  could return something like 
EFI_UNSUPPORTED in QemuBootOrderLibNull.

Can you check, please?

[/SAMI]

>     PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
>     CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
>     FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
> @@ -330,6 +332,10 @@
>         NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
>         NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
>     }
> +  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {
> +    <LibraryClasses>
> +      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf
> +  }
>   
>     #
>     # SCSI Bus and Disk Driver
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf
> index 81c539590a..15b9c13c59 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.fdf
> +++ b/ArmVirtPkg/ArmVirtCloudHv.fdf
> @@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE
>     INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
>     INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>     INF MdeModulePkg/Application/UiApp/UiApp.inf
> +  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
>   
>     #
>     # SCSI Bus and Disk Driver
> diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> index b7aa6ebb4e..f7b53d0747 100644
> --- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> +++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> @@ -24,7 +24,6 @@
>     EmbeddedPkg/EmbeddedPkg.dec
>     MdePkg/MdePkg.dec
>     OvmfPkg/OvmfPkg.dec
> -  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
[SAMI] This should be part of patch 1/1.
>   
>   [LibraryClasses]
>     BaseLib

[-- Attachment #2: Type: text/html, Size: 5052 bytes --]