Re: [PATCH V2 6/9] ArmVirtPkg/QemuFwCfgLib: Relocate QemuFwCfgLib to OvmfPkg

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

Hi Abner,

Thank you for this patch.

I have a minor suggestion marked inline as [SAMI].

Regards,

Sami Mujawar


On 28/09/2021 09:31 AM, Abner Chang wrote:
> Relocate QemuFwCfgLib to OvmfPkg/Library/QemuFwCfgLib and rename
> it to QemuFwCfgLibMMIO, this library is leverage by both ARM and
> RISC-V archs.
>
> Signed-off-by: Abner Chang <abner.chang@hpe.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> ---
>   ArmVirtPkg/ArmVirtQemu.dsc                                 | 2 +-
>   ArmVirtPkg/ArmVirtQemuKernel.dsc                           | 2 +-
>   .../Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf              | 5 ++---
>   .../Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c                | 7 ++++---
>   4 files changed, 8 insertions(+), 8 deletions(-)
>   rename ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf => OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf (87%)
>   rename ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c => OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c (93%)
[SAMI] Is it possible to rename QemuFwCfgLibMMIO.[c|inf] to 
QemuFwCfgMmioLib.[c|inf], please? This would then follow a pattern 
similar to OvmfPkg\Library\XenIoMmioLib\XenIoMmioLib.[c|inf].
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 07f9699c79..6c949fd559 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -59,7 +59,7 @@
>     # Virtio Support
>     VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>     VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
> -  QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> +  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf
>     QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
>     QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>     QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index cf7a2b4463..64035a948d 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -57,7 +57,7 @@
>     # Virtio Support
>     VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>     VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
> -  QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> +  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf
>     QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
>     QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>     QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
> diff --git a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf
> similarity index 87%
> rename from ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> rename to OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf
> index f3cc827907..8101fac03f 100644
> --- a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf
> @@ -23,17 +23,16 @@
>   # The following information is for reference only and not required by the build
>   # tools.
>   #
> -#  VALID_ARCHITECTURES           = ARM AARCH64
> +#  VALID_ARCHITECTURES           = ARM AARCH64 RISCV64
>   #
>   
>   [Sources]
> -  QemuFwCfgLib.c
> +  QemuFwCfgLibMMIO.c
>   
>   [Packages]
>     MdePkg/MdePkg.dec
>     OvmfPkg/OvmfPkg.dec
>     EmbeddedPkg/EmbeddedPkg.dec
> -  ArmVirtPkg/ArmVirtPkg.dec
>   
>   [LibraryClasses]
>     BaseLib
> diff --git a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c
> similarity index 93%
> rename from ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> rename to OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c
> index e2ac4108d1..b953f2eb6c 100644
> --- a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c
> @@ -4,6 +4,7 @@
>   
>     Copyright (C) 2013 - 2014, Red Hat, Inc.
>     Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
> +  (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
>   
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>   **/
> @@ -239,7 +240,7 @@ MmioReadBytes (
>     UINT8 *Ptr;
>     UINT8 *End;
>   
> -#ifdef MDE_CPU_AARCH64
> +#if defined(MDE_CPU_AARCH64) || defined(MDE_CPU_RISCV64)
>     Left = Size & 7;
>   #else
>     Left = Size & 3;
> @@ -249,7 +250,7 @@ MmioReadBytes (
>     Ptr = Buffer;
>     End = Ptr + Size;
>   
> -#ifdef MDE_CPU_AARCH64
> +#if defined(MDE_CPU_AARCH64) || defined(MDE_CPU_RISCV64)
>     while (Ptr < End) {
>       *(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress);
>       Ptr += 8;
> @@ -322,7 +323,7 @@ DmaTransferBytes (
>     //
>     // This will fire off the transfer.
>     //
> -#ifdef MDE_CPU_AARCH64
> +#if defined(MDE_CPU_AARCH64) || defined(MDE_CPU_RISCV64)
>     MmioWrite64 (mFwCfgDmaAddress, SwapBytes64 ((UINT64)&Access));
>   #else
>     MmioWrite32 ((UINT32)(mFwCfgDmaAddress + 4), SwapBytes32 ((UINT32)&Access));