Re: [edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files
Hi Ard,
Thanks,
Chao
On 2024/4/25 20:58, Ard Biesheuvel wrote:
On Thu, 25 Apr 2024 at 14:13, Chao Li 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
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Cc: Leif Lindholm
Cc: Sami Mujawar
Cc: Sunil V L
Cc: Andrei Warkentin
Signed-off-by: Chao Li
---
.../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.
(C) Copyright 2021 Hewlett Packard Enterprise Development LP
+ Copyright (c) 2024 Loongson Technology Corporation Limited. All rights
reserved.
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
-STATIC UINTN mFwCfgSelectorAddress;
-STATIC UINTN mFwCfgDataAddress;
-STATIC UINTN mFwCfgDmaAddress;
-
-/**
- Reads firmware configuration bytes into a buffer
-
- @param[in] SizeSize 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] SizeSize 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;
- UINTNAddressCells, SizeCells;
- UINT64 FwCfgSelectorAddress;
- UINT64 FwCfgSelectorSize;
- UINT64 FwCfgDataAddress;
- UINT64 FwCfgDataSize;
- UINT64 FwCfgDmaAddress;
- UINT64 FwCfgDmaSize;
-
- Status = gBS->LocateProtocol (
- ,
- NULL,
- (VOID **)
- );
- ASSERT_EFI_ERROR (Status);
-
- Status = FdtClient->FindCompatibleNodeReg (
-FdtClient,
-"qemu,fw-cfg-mmio",
-(CONST VOID **),
-,
-,
-
-);
- 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;
-
Re: [edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files
On Thu, 25 Apr 2024 at 14:13, Chao Li 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
> Cc: Jiewen Yao
> Cc: Gerd Hoffmann
> Cc: Leif Lindholm
> Cc: Sami Mujawar
> Cc: Sunil V L
> Cc: Andrei Warkentin
> Signed-off-by: Chao Li
> ---
> .../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.
>(C) Copyright 2021 Hewlett Packard Enterprise Development LP
> + Copyright (c) 2024 Loongson Technology Corporation Limited. All rights
> reserved.
>
Please only claim copyright for code that you wrote, not for code that
you just moved between files.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
> @@ -20,63 +19,7 @@
>
> #include
>
> -STATIC UINTN mFwCfgSelectorAddress;
> -STATIC UINTN mFwCfgDataAddress;
> -STATIC UINTN mFwCfgDmaAddress;
> -
> -/**
> - Reads firmware configuration bytes into a buffer
> -
> - @param[in] SizeSize 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] SizeSize 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;
> - UINTNAddressCells, SizeCells;
> - UINT64 FwCfgSelectorAddress;
> - UINT64 FwCfgSelectorSize;
> - UINT64 FwCfgDataAddress;
> - UINT64 FwCfgDataSize;
> - UINT64 FwCfgDmaAddress;
> - UINT64 FwCfgDmaSize;
> -
> - Status = gBS->LocateProtocol (
> - ,
> - NULL,
> - (VOID **)
> - );
> - ASSERT_EFI_ERROR (Status);
> -
> - Status = FdtClient->FindCompatibleNodeReg (
> -FdtClient,
> -"qemu,fw-cfg-mmio",
> -(CONST VOID **),
> -,
> -,
> -
> -);
> - 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
[edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files
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
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Cc: Leif Lindholm
Cc: Sami Mujawar
Cc: Sunil V L
Cc: Andrei Warkentin
Signed-off-by: Chao Li
---
.../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.
(C) Copyright 2021 Hewlett Packard Enterprise Development LP
+ Copyright (c) 2024 Loongson Technology Corporation Limited. All rights
reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -20,63 +19,7 @@
#include
-STATIC UINTN mFwCfgSelectorAddress;
-STATIC UINTN mFwCfgDataAddress;
-STATIC UINTN mFwCfgDmaAddress;
-
-/**
- Reads firmware configuration bytes into a buffer
-
- @param[in] SizeSize 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] SizeSize 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;
- UINTNAddressCells, SizeCells;
- UINT64 FwCfgSelectorAddress;
- UINT64 FwCfgSelectorSize;
- UINT64 FwCfgDataAddress;
- UINT64 FwCfgDataSize;
- UINT64 FwCfgDmaAddress;
- UINT64 FwCfgDmaSize;
-
- Status = gBS->LocateProtocol (
- ,
- NULL,
- (VOID **)
- );
- ASSERT_EFI_ERROR (Status);
-
- Status = FdtClient->FindCompatibleNodeReg (
-FdtClient,
-"qemu,fw-cfg-mmio",
-(CONST VOID **),
-,
-,
-
-);
- 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
