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


Sami Mujawar
 

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));
>

Join devel@edk2.groups.io to automatically receive all group messages.