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

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

Hi Jiewen,

Thank you for clarifying the library naming convention.
I could not find any references/examples as such in https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/v/release%2F2.20/4_naming_conventions/42_file_names and therefore had suggested following the file naming as done for Xen.

Regards,

Sami Mujawar

On 29/09/2021, 11:04, "Yao, Jiewen" <jiewen.yao@intel.com> wrote:

    hi
    I think the original name is correct.

    The naming convention is : <LibClassName>Lib<InstanceName>

    thank you!
    Yao, Jiewen


    > 在 2021年9月29日，下午5:45，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));
    >