Re: [edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files

["Chao Li" <lichao@loongson.cn>]

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

Hi Ard,


Thanks,
Chao
On 2024/4/25 20:58, Ard Biesheuvel wrote:
> On Thu, 25 Apr 2024 at 14:13, Chao Li<lichao@loongson.cn>  wrote:
>> Separate QemuFwCfgLibMmio.c into two files named QemuFwCfgLibMmio.c and
>> QemuFwCfgLibMmioDxe.c, added a new header named
>> QemuFwCfgLibMmioInternal.h for MMIO version.
>>
>> Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc").
>>
>> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4755
>>
>> Cc: Ard Biesheuvel<ardb+tianocore@kernel.org>
>> Cc: Jiewen Yao<jiewen.yao@intel.com>
>> Cc: Gerd Hoffmann<kraxel@redhat.com>
>> Cc: Leif Lindholm<quic_llindhol@quicinc.com>
>> Cc: Sami Mujawar<sami.mujawar@arm.com>
>> Cc: Sunil V L<sunilvl@ventanamicro.com>
>> Cc: Andrei Warkentin<andrei.warkentin@intel.com>
>> Signed-off-by: Chao Li<lichao@loongson.cn>
>> ---
>>   .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 194 +-----------------
>>   .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |   4 +-
>>   .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 179 ++++++++++++++++
>>   .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 153 ++++++++++++++
>>   4 files changed, 340 insertions(+), 190 deletions(-)
>>   create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h
>>   create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c
>>
>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
>> index 115a210759..dc949c8e26 100644
>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
>> @@ -1,10 +1,9 @@
>>   /** @file
>>
>> -  Stateful and implicitly initialized fw_cfg library implementation.
>> -
>>     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>
>> +  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights reserved.<BR>
>>
> Please only claim copyright for code that you wrote, not for code that
> you just moved between files.
OK, I will remove Loongson copyright in this patch and add it in patch 2 
or 3.
>
>>     SPDX-License-Identifier: BSD-2-Clause-Patent
>>   **/
>> @@ -20,63 +19,7 @@
>>
>>   #include <Protocol/FdtClient.h>
>>
>> -STATIC UINTN  mFwCfgSelectorAddress;
>> -STATIC UINTN  mFwCfgDataAddress;
>> -STATIC UINTN  mFwCfgDmaAddress;
>> -
>> -/**
>> -  Reads firmware configuration bytes into a buffer
>> -
>> -  @param[in] Size    Size in bytes to read
>> -  @param[in] Buffer  Buffer to store data into  (OPTIONAL if Size is 0)
>> -
>> -**/
>> -typedef
>> -VOID(EFIAPI READ_BYTES_FUNCTION)(
>> -  IN UINTN Size,
>> -  IN VOID  *Buffer OPTIONAL
>> -  );
>> -
>> -/**
>> -  Writes bytes from a buffer to firmware configuration
>> -
>> -  @param[in] Size    Size in bytes to write
>> -  @param[in] Buffer  Buffer to transfer data from (OPTIONAL if Size is 0)
>> -
>> -**/
>> -typedef
>> -VOID(EFIAPI WRITE_BYTES_FUNCTION)(
>> -  IN UINTN Size,
>> -  IN VOID  *Buffer OPTIONAL
>> -  );
>> -
>> -/**
>> -  Skips bytes in firmware configuration
>> -
>> -  @param[in] Size  Size in bytes to skip
>> -
>> -**/
>> -typedef
>> -VOID(EFIAPI SKIP_BYTES_FUNCTION)(
>> -  IN UINTN Size
>> -  );
>> -
>> -//
>> -// Forward declaration of the two implementations we have.
>> -//
>> -STATIC READ_BYTES_FUNCTION   MmioReadBytes;
>> -STATIC WRITE_BYTES_FUNCTION  MmioWriteBytes;
>> -STATIC SKIP_BYTES_FUNCTION   MmioSkipBytes;
>> -STATIC READ_BYTES_FUNCTION   DmaReadBytes;
>> -STATIC WRITE_BYTES_FUNCTION  DmaWriteBytes;
>> -STATIC SKIP_BYTES_FUNCTION   DmaSkipBytes;
>> -
>> -//
>> -// These correspond to the implementation we detect at runtime.
>> -//
>> -STATIC READ_BYTES_FUNCTION   *InternalQemuFwCfgReadBytes  = MmioReadBytes;
>> -STATIC WRITE_BYTES_FUNCTION  *InternalQemuFwCfgWriteBytes = MmioWriteBytes;
>> -STATIC SKIP_BYTES_FUNCTION   *InternalQemuFwCfgSkipBytes  = MmioSkipBytes;
>> +#include "QemuFwCfgLibMmioInternal.h"
>>
>>   /**
>>     Returns a boolean indicating if the firmware configuration interface
>> @@ -97,126 +40,6 @@ QemuFwCfgIsAvailable (
>>     return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0);
>>   }
>>
>> -RETURN_STATUS
>> -EFIAPI
>> -QemuFwCfgInitialize (
>> -  VOID
>> -  )
>> -{
>> -  EFI_STATUS           Status;
>> -  FDT_CLIENT_PROTOCOL  *FdtClient;
>> -  CONST UINT64         *Reg;
>> -  UINT32               RegSize;
>> -  UINTN                AddressCells, SizeCells;
>> -  UINT64               FwCfgSelectorAddress;
>> -  UINT64               FwCfgSelectorSize;
>> -  UINT64               FwCfgDataAddress;
>> -  UINT64               FwCfgDataSize;
>> -  UINT64               FwCfgDmaAddress;
>> -  UINT64               FwCfgDmaSize;
>> -
>> -  Status = gBS->LocateProtocol (
>> -                  &gFdtClientProtocolGuid,
>> -                  NULL,
>> -                  (VOID **)&FdtClient
>> -                  );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  Status = FdtClient->FindCompatibleNodeReg (
>> -                        FdtClient,
>> -                        "qemu,fw-cfg-mmio",
>> -                        (CONST VOID **)&Reg,
>> -                        &AddressCells,
>> -                        &SizeCells,
>> -                        &RegSize
>> -                        );
>> -  if (EFI_ERROR (Status)) {
>> -    DEBUG ((
>> -      DEBUG_WARN,
>> -      "%a: No 'qemu,fw-cfg-mmio' compatible DT node found (Status == %r)\n",
>> -      __func__,
>> -      Status
>> -      ));
>> -    return EFI_SUCCESS;
>> -  }
>> -
>> -  ASSERT (AddressCells == 2);
>> -  ASSERT (SizeCells == 2);
>> -  ASSERT (RegSize == 2 * sizeof (UINT64));
>> -
>> -  FwCfgDataAddress     = SwapBytes64 (Reg[0]);
>> -  FwCfgDataSize        = 8;
>> -  FwCfgSelectorAddress = FwCfgDataAddress + FwCfgDataSize;
>> -  FwCfgSelectorSize    = 2;
>> -
>> -  //
>> -  // The following ASSERT()s express
>> -  //
>> -  //   Address + Size - 1 <= MAX_UINTN
>> -  //
>> -  // for both registers, that is, that the last byte in each MMIO range is
>> -  // expressible as a MAX_UINTN. The form below is mathematically
>> -  // equivalent, and it also prevents any unsigned overflow before the
>> -  // comparison.
>> -  //
>> -  ASSERT (FwCfgSelectorAddress <= MAX_UINTN - FwCfgSelectorSize + 1);
>> -  ASSERT (FwCfgDataAddress     <= MAX_UINTN - FwCfgDataSize     + 1);
>> -
>> -  mFwCfgSelectorAddress = FwCfgSelectorAddress;
>> -  mFwCfgDataAddress     = FwCfgDataAddress;
>> -
>> -  DEBUG ((
>> -    DEBUG_INFO,
>> -    "Found FwCfg @ 0x%Lx/0x%Lx\n",
>> -    FwCfgSelectorAddress,
>> -    FwCfgDataAddress
>> -    ));
>> -
>> -  if (SwapBytes64 (Reg[1]) >= 0x18) {
>> -    FwCfgDmaAddress = FwCfgDataAddress + 0x10;
>> -    FwCfgDmaSize    = 0x08;
>> -
>> -    //
>> -    // See explanation above.
>> -    //
>> -    ASSERT (FwCfgDmaAddress <= MAX_UINTN - FwCfgDmaSize + 1);
>> -
>> -    DEBUG ((DEBUG_INFO, "Found FwCfg DMA @ 0x%Lx\n", FwCfgDmaAddress));
>> -  } else {
>> -    FwCfgDmaAddress = 0;
>> -  }
>> -
>> -  if (QemuFwCfgIsAvailable ()) {
>> -    UINT32  Signature;
>> -
>> -    QemuFwCfgSelectItem (QemuFwCfgItemSignature);
>> -    Signature = QemuFwCfgRead32 ();
>> -    if (Signature == SIGNATURE_32 ('Q', 'E', 'M', 'U')) {
>> -      //
>> -      // For DMA support, we require the DTB to advertise the register, and the
>> -      // feature bitmap (which we read without DMA) to confirm the feature.
>> -      //
>> -      if (FwCfgDmaAddress != 0) {
>> -        UINT32  Features;
>> -
>> -        QemuFwCfgSelectItem (QemuFwCfgItemInterfaceVersion);
>> -        Features = QemuFwCfgRead32 ();
>> -        if ((Features & FW_CFG_F_DMA) != 0) {
>> -          mFwCfgDmaAddress            = FwCfgDmaAddress;
>> -          InternalQemuFwCfgReadBytes  = DmaReadBytes;
>> -          InternalQemuFwCfgWriteBytes = DmaWriteBytes;
>> -          InternalQemuFwCfgSkipBytes  = DmaSkipBytes;
>> -        }
>> -      }
>> -    } else {
>> -      mFwCfgSelectorAddress = 0;
>> -      mFwCfgDataAddress     = 0;
>> -    }
>> -  }
>> -
>> -  return RETURN_SUCCESS;
>> -}
>> -
>>   /**
>>     Selects a firmware configuration item for reading.
>>
>> @@ -240,7 +63,6 @@ QemuFwCfgSelectItem (
>>   /**
>>     Slow READ_BYTES_FUNCTION.
>>   **/
>> -STATIC
>>   VOID
>>   EFIAPI
>>   MmioReadBytes (
>> @@ -252,7 +74,7 @@ MmioReadBytes (
>>     UINT8  *Ptr;
>>     UINT8  *End;
>>
>> - #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64)
>> + #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined (MDE_CPU_LOONGARCH64)
>>     Left = Size & 7;
>>    #else
>>     Left = Size & 3;
>> @@ -262,7 +84,7 @@ MmioReadBytes (
>>     Ptr   = Buffer;
>>     End   = Ptr + Size;
>>
>> - #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64)
>> + #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined (MDE_CPU_LOONGARCH64)
>>     while (Ptr < End) {
>>       *(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress);
>>       Ptr           += 8;
>> @@ -306,7 +128,6 @@ MmioReadBytes (
>>                             FW_CFG_DMA_CTL_READ  - read from fw_cfg into Buffer.
>>                             FW_CFG_DMA_CTL_SKIP  - skip bytes in fw_cfg.
>>   **/
>> -STATIC
>>   VOID
>>   DmaTransferBytes (
>>     IN     UINTN   Size,
>> @@ -340,7 +161,7 @@ DmaTransferBytes (
>>     //
>>     // This will fire off the transfer.
>>     //
>> - #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64)
>> + #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined (MDE_CPU_LOONGARCH64)
>>     MmioWrite64 (mFwCfgDmaAddress, SwapBytes64 ((UINT64)&Access));
>>    #else
>>     MmioWrite32 ((UINT32)(mFwCfgDmaAddress + 4), SwapBytes32 ((UINT32)&Access));
>> @@ -365,7 +186,6 @@ DmaTransferBytes (
>>   /**
>>     Fast READ_BYTES_FUNCTION.
>>   **/
>> -STATIC
>>   VOID
>>   EFIAPI
>>   DmaReadBytes (
>> @@ -403,7 +223,6 @@ QemuFwCfgReadBytes (
>>   /**
>>     Slow WRITE_BYTES_FUNCTION.
>>   **/
>> -STATIC
>>   VOID
>>   EFIAPI
>>   MmioWriteBytes (
>> @@ -421,7 +240,6 @@ MmioWriteBytes (
>>   /**
>>     Fast WRITE_BYTES_FUNCTION.
>>   **/
>> -STATIC
>>   VOID
>>   EFIAPI
>>   DmaWriteBytes (
>> @@ -457,7 +275,6 @@ QemuFwCfgWriteBytes (
>>   /**
>>     Slow SKIP_BYTES_FUNCTION.
>>   **/
>> -STATIC
>>   VOID
>>   EFIAPI
>>   MmioSkipBytes (
>> @@ -484,7 +301,6 @@ MmioSkipBytes (
>>   /**
>>     Fast SKIP_BYTES_FUNCTION.
>>   **/
>> -STATIC
>>   VOID
>>   EFIAPI
>>   DmaSkipBytes (
>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
>> index 4b0dfbcb0d..f2596f270e 100644
>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
>> @@ -4,6 +4,7 @@
>>   #
>>   #  Copyright (C) 2013 - 2014, Red Hat, Inc.
>>   #  Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.<BR>
>> +#  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights reserved.<BR>
>>   #
>>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>>   #
>> @@ -23,11 +24,12 @@ [Defines]
>>   # The following information is for reference only and not required by the build
>>   # tools.
>>   #
>> -#  VALID_ARCHITECTURES           = ARM AARCH64 RISCV64
>> +#  VALID_ARCHITECTURES           = ARM AARCH64 RISCV64 LOONGARCH64
>>   #
>>
>>   [Sources]
>>     QemuFwCfgLibMmio.c
>> +  QemuFwCfgMmioDxe.c
>>
>>   [Packages]
>>     MdePkg/MdePkg.dec
>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h
>> new file mode 100644
>> index 0000000000..d7d645f700
>> --- /dev/null
>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h
>> @@ -0,0 +1,179 @@
>> +/** @file
>> +  Internal interfaces specific to the QemuFwCfgLibMmio instances in OvmfPkg.
>> +
>> +  Copyright (C) 2016, Red Hat, Inc.
>> +  Copyright (C) 2017, Advanced Micro Devices. All rights reserved
>> +  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#ifndef QEMU_FW_CFG_LIB_MMIO_INTERNAL_H_
>> +#define QEMU_FW_CFG_LIB_MMIO_INTERNAL_H_
>> +
>> +extern UINTN  mFwCfgSelectorAddress;
>> +extern UINTN  mFwCfgDataAddress;
>> +extern UINTN  mFwCfgDmaAddress;
>> +
> These can remain STATIC and live in the DXE version of the library, no?
OK, the DXE version will keep it be STATIC, I will fix it on my next commit.
>
>> +/**
>> +  Reads firmware configuration bytes into a buffer
>> +
>> +  @param[in] Size    Size in bytes to read
>> +  @param[in] Buffer  Buffer to store data into  (OPTIONAL if Size is 0)
>> +
>> +**/
>> +typedef
>> +VOID(EFIAPI READ_BYTES_FUNCTION)(
>> +  IN UINTN Size,
>> +  IN VOID  *Buffer OPTIONAL
>> +  );
>> +
>> +/**
>> +  Writes bytes from a buffer to firmware configuration
>> +
>> +  @param[in] Size    Size in bytes to write
>> +  @param[in] Buffer  Buffer to transfer data from (OPTIONAL if Size is 0)
>> +
>> +**/
>> +typedef
>> +VOID(EFIAPI WRITE_BYTES_FUNCTION)(
>> +  IN UINTN Size,
>> +  IN VOID  *Buffer OPTIONAL
>> +  );
>> +
>> +/**
>> +  Skips bytes in firmware configuration
>> +
>> +  @param[in] Size  Size in bytes to skip
>> +
>> +**/
>> +typedef
>> +VOID(EFIAPI SKIP_BYTES_FUNCTION)(
>> +  IN UINTN Size
>> +  );
>> +
>> +/**
>> +  Reads firmware configuration bytes into a buffer
>> +
>> +  @param[in] Size    Size in bytes to read
>> +  @param[in] Buffer  Buffer to store data into  (OPTIONAL if Size is 0)
>> +
>> +**/
>> +extern
>> +VOID
>> +EFIAPI
>> +(*InternalQemuFwCfgReadBytes) (
>> +  IN UINTN  Size,
>> +  IN VOID   *Buffer  OPTIONAL
>> +  );
>> +
>> +/**
>> +  Writes bytes from a buffer to firmware configuration
>> +
>> +  @param[in] Size    Size in bytes to write
>> +  @param[in] Buffer  Buffer to transfer data from (OPTIONAL if Size is 0)
>> +
>> +**/
>> +extern
>> +VOID
>> +EFIAPI
>> +(*InternalQemuFwCfgWriteBytes) (
>> +  IN UINTN  Size,
>> +  IN VOID   *Buffer  OPTIONAL
>> +  );
>> +
>> +/**
>> +  Skips bytes in firmware configuration
>> +
>> +  @param[in] Size  Size in bytes to skip
>> +
>> +**/
>> +extern
>> +VOID
>> +EFIAPI
>> +(*InternalQemuFwCfgSkipBytes) (
>> +  IN UINTN  Size
>> +  );
>> +
>> +/**
>> +  Slow READ_BYTES_FUNCTION.
>> +**/
>> +VOID
>> +EFIAPI
>> +MmioReadBytes (
>> +  IN UINTN  Size,
>> +  IN VOID   *Buffer OPTIONAL
>> +  );
>> +
>> +/**
>> +  Slow WRITE_BYTES_FUNCTION.
>> +**/
>> +VOID
>> +EFIAPI
>> +MmioWriteBytes (
>> +  IN UINTN  Size,
>> +  IN VOID   *Buffer OPTIONAL
>> +  );
>> +
>> +/**
>> +  Slow SKIP_BYTES_FUNCTION.
>> +**/
>> +VOID
>> +EFIAPI
>> +MmioSkipBytes (
>> +  IN UINTN  Size
>> +  );
>> +
>> +/**
>> +  Fast READ_BYTES_FUNCTION.
>> +**/
>> +VOID
>> +EFIAPI
>> +DmaReadBytes (
>> +  IN UINTN  Size,
>> +  IN VOID   *Buffer OPTIONAL
>> +  );
>> +
>> +/**
>> +  Fast WRITE_BYTES_FUNCTION.
>> +**/
>> +VOID
>> +EFIAPI
>> +DmaWriteBytes (
>> +  IN UINTN  Size,
>> +  IN VOID   *Buffer OPTIONAL
>> +  );
>> +
>> +/**
>> +  Fast SKIP_BYTES_FUNCTION.
>> +**/
>> +VOID
>> +EFIAPI
>> +DmaSkipBytes (
>> +  IN UINTN  Size
>> +  );
>> +
>> +/**
>> +  Transfer an array of bytes, or skip a number of bytes, using the DMA
>> +  interface.
>> +
>> +  @param[in]     Size     Size in bytes to transfer or skip.
>> +
>> +  @param[in,out] Buffer   Buffer to read data into or write data from. Ignored,
>> +                          and may be NULL, if Size is zero, or Control is
>> +                          FW_CFG_DMA_CTL_SKIP.
>> +
>> +  @param[in]     Control  One of the following:
>> +                          FW_CFG_DMA_CTL_WRITE - write to fw_cfg from Buffer.
>> +                          FW_CFG_DMA_CTL_READ  - read from fw_cfg into Buffer.
>> +                          FW_CFG_DMA_CTL_SKIP  - skip bytes in fw_cfg.
>> +**/
>> +VOID
>> +DmaTransferBytes (
>> +  IN     UINTN   Size,
>> +  IN OUT VOID    *Buffer OPTIONAL,
>> +  IN     UINT32  Control
>> +  );
>> +
>> +#endif
>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c
>> new file mode 100644
>> index 0000000000..4844a42a36
>> --- /dev/null
>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c
>> @@ -0,0 +1,153 @@
>> +/** @file
>> +
>> +  Stateful and implicitly initialized fw_cfg library implementation.
>> +
>> +  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>
>> +  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +
>> +#include <Uefi.h>
>> +
>> +#include <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/QemuFwCfgLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +
>> +#include <Protocol/FdtClient.h>
>> +
>> +#include "QemuFwCfgLibMmioInternal.h"
>> +
>> +UINTN  mFwCfgSelectorAddress;
>> +UINTN  mFwCfgDataAddress;
>> +UINTN  mFwCfgDmaAddress;
>> +
>> +//
>> +// These correspond to the implementation we detect at runtime.
>> +//
>> +READ_BYTES_FUNCTION   *InternalQemuFwCfgReadBytes  = MmioReadBytes;
>> +WRITE_BYTES_FUNCTION  *InternalQemuFwCfgWriteBytes = MmioWriteBytes;
>> +SKIP_BYTES_FUNCTION   *InternalQemuFwCfgSkipBytes  = MmioSkipBytes;
>> +
> These are duplicated later in the PEI version of the library, right?
> So they can live in the shared .c file
Yes, them should be in the shared.c file, let me try.
>
>> +RETURN_STATUS
>> +EFIAPI
>> +QemuFwCfgInitialize (
>> +  VOID
>> +  )
>> +{
>> +  EFI_STATUS           Status;
>> +  FDT_CLIENT_PROTOCOL  *FdtClient;
>> +  CONST UINT64         *Reg;
>> +  UINT32               RegSize;
>> +  UINTN                AddressCells, SizeCells;
>> +  UINT64               FwCfgSelectorAddress;
>> +  UINT64               FwCfgSelectorSize;
>> +  UINT64               FwCfgDataAddress;
>> +  UINT64               FwCfgDataSize;
>> +  UINT64               FwCfgDmaAddress;
>> +  UINT64               FwCfgDmaSize;
>> +
>> +  Status = gBS->LocateProtocol (
>> +                  &gFdtClientProtocolGuid,
>> +                  NULL,
>> +                  (VOID **)&FdtClient
>> +                  );
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  Status = FdtClient->FindCompatibleNodeReg (
>> +                        FdtClient,
>> +                        "qemu,fw-cfg-mmio",
>> +                        (CONST VOID **)&Reg,
>> +                        &AddressCells,
>> +                        &SizeCells,
>> +                        &RegSize
>> +                        );
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((
>> +      DEBUG_WARN,
>> +      "%a: No 'qemu,fw-cfg-mmio' compatible DT node found (Status == %r)\n",
>> +      __func__,
>> +      Status
>> +      ));
>> +    return EFI_SUCCESS;
>> +  }
>> +
>> +  ASSERT (AddressCells == 2);
>> +  ASSERT (SizeCells == 2);
>> +  ASSERT (RegSize == 2 * sizeof (UINT64));
>> +
>> +  FwCfgDataAddress     = SwapBytes64 (Reg[0]);
>> +  FwCfgDataSize        = 8;
>> +  FwCfgSelectorAddress = FwCfgDataAddress + FwCfgDataSize;
>> +  FwCfgSelectorSize    = 2;
>> +
>> +  //
>> +  // The following ASSERT()s express
>> +  //
>> +  //   Address + Size - 1 <= MAX_UINTN
>> +  //
>> +  // for both registers, that is, that the last byte in each MMIO range is
>> +  // expressible as a MAX_UINTN. The form below is mathematically
>> +  // equivalent, and it also prevents any unsigned overflow before the
>> +  // comparison.
>> +  //
>> +  ASSERT (FwCfgSelectorAddress <= MAX_UINTN - FwCfgSelectorSize + 1);
>> +  ASSERT (FwCfgDataAddress     <= MAX_UINTN - FwCfgDataSize     + 1);
>> +
>> +  mFwCfgSelectorAddress = FwCfgSelectorAddress;
>> +  mFwCfgDataAddress     = FwCfgDataAddress;
>> +
>> +  DEBUG ((
>> +    DEBUG_INFO,
>> +    "Found FwCfg @ 0x%Lx/0x%Lx\n",
>> +    FwCfgSelectorAddress,
>> +    FwCfgDataAddress
>> +    ));
>> +
>> +  if (SwapBytes64 (Reg[1]) >= 0x18) {
>> +    FwCfgDmaAddress = FwCfgDataAddress + 0x10;
>> +    FwCfgDmaSize    = 0x08;
>> +
>> +    //
>> +    // See explanation above.
>> +    //
>> +    ASSERT (FwCfgDmaAddress <= MAX_UINTN - FwCfgDmaSize + 1);
>> +
>> +    DEBUG ((DEBUG_INFO, "Found FwCfg DMA @ 0x%Lx\n", FwCfgDmaAddress));
>> +  } else {
>> +    FwCfgDmaAddress = 0;
>> +  }
>> +
>> +  if (QemuFwCfgIsAvailable ()) {
>> +    UINT32  Signature;
>> +
>> +    QemuFwCfgSelectItem (QemuFwCfgItemSignature);
>> +    Signature = QemuFwCfgRead32 ();
>> +    if (Signature == SIGNATURE_32 ('Q', 'E', 'M', 'U')) {
>> +      //
>> +      // For DMA support, we require the DTB to advertise the register, and the
>> +      // feature bitmap (which we read without DMA) to confirm the feature.
>> +      //
>> +      if (FwCfgDmaAddress != 0) {
>> +        UINT32  Features;
>> +
>> +        QemuFwCfgSelectItem (QemuFwCfgItemInterfaceVersion);
>> +        Features = QemuFwCfgRead32 ();
>> +        if ((Features & FW_CFG_F_DMA) != 0) {
>> +          mFwCfgDmaAddress            = FwCfgDmaAddress;
>> +          InternalQemuFwCfgReadBytes  = DmaReadBytes;
>> +          InternalQemuFwCfgWriteBytes = DmaWriteBytes;
>> +          InternalQemuFwCfgSkipBytes  = DmaSkipBytes;
>> +        }
>> +      }
>> +    } else {
>> +      mFwCfgSelectorAddress = 0;
>> +      mFwCfgDataAddress     = 0;
>> +    }
>> +  }
>> +
>> +  return RETURN_SUCCESS;
>> +}
>> --
>> 2.27.0
>>
>
> 
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118306): https://edk2.groups.io/g/devel/message/118306
Mute This Topic: https://groups.io/mt/105728766/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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