Hi Ard,
I will save the real HOB data in the next version.Hi Ard,
On 2024/4/25 21:02, Ard Biesheuvel wrote:
On Thu, 25 Apr 2024 at 14:13, Chao Li <lichao@loongson.cn> wrote:Added the HOB methods to load and store the QEMU firmware configure address, data address and DMA address, which are not enabled during the DXE stage. 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 | 81 +++++++++++++++-- .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf | 1 + .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h | 74 +++++++++++++++ .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c | 91 ++++++++++++++++--- 4 files changed, 226 insertions(+), 21 deletions(-) diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c index dc949c8e26..b5dbc5e4b5 100644 --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c @@ -8,11 +8,16 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ +#include <Base.h> #include <Uefi.h> +#include <Pi/PiBootMode.h> +#include <Pi/PiHob.h> + #include <Library/BaseLib.h> #include <Library/BaseMemoryLib.h> #include <Library/DebugLib.h> +#include <Library/HobLib.h> #include <Library/IoLib.h> #include <Library/QemuFwCfgLib.h> #include <Library/UefiBootServicesTableLib.h> @@ -21,6 +26,62 @@ #include "QemuFwCfgLibMmioInternal.h" +EFI_GUID mFwCfgResourceGuid = FW_CONFIG_RESOURCE_HOB_GUID; + +/** + Build firmware configure resource address HOB. + + @param[in] FwCfgResource A pointer to firmware configure resource. + + @retval NULL +**/ +VOID +QemuBuildFwCfgResourceHob ( + IN QEMU_FW_CFG_RESOURCE *FwCfgResource + ) +{ + UINT64 Data64; + + Data64 = (UINT64)(UINTN)FwCfgResource; + + BuildGuidDataHob ( + &mFwCfgResourceGuid, + (VOID *)&Data64,This looks wrong: why are you taking the address of the stack variable rather than the address of the resource descriptor?It only saves the pointer of FwCfgResource, and the memory space has been created in the PEI constructor. Do you mean saving all contents of FwCfgResource in the HOB?
The following line is indeed wrong. if only save the pointer, the size should be "sizeof (UINT64)".
All right.+ sizeof (QEMU_FW_CFG_RESOURCE) + ); +} + +/** + Get firmware configure resource in HOB. + + @param VOID + + @retval FwCfgResource The firmware configure resouce in HOB.resource
OK.+ NULL The firmware configure resouce not found. +**/ +QEMU_FW_CFG_RESOURCE * +QemuGetFwCfgResourceHob ( + VOID + ) +{ + EFI_HOB_GUID_TYPE *GuidHob; + VOID *DataInHob; + QEMU_FW_CFG_RESOURCE *FwCfgResource; + + GuidHob = NULL; + DataInHob = NULL; + + GuidHob = GetFirstGuidHob (&mFwCfgResourceGuid);Please define this GUID in the package .DEC file and add it to the [Guids] section in the .INF so that you can refer to its name directly.
+ if (GuidHob == NULL) { + return NULL; + } + + DataInHob = GET_GUID_HOB_DATA (GuidHob); + FwCfgResource = (QEMU_FW_CFG_RESOURCE *)(*(UINTN *)DataInHob); + + return FwCfgResource; +} + /** Returns a boolean indicating if the firmware configuration interface is available or not. @@ -37,7 +98,7 @@ QemuFwCfgIsAvailable ( VOID ) { - return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0); + return (BOOLEAN)(QemuGetFwCfgSelectorAddress () != 0 && QemuGetFwCfgDataAddress () != 0); } /** @@ -56,7 +117,7 @@ QemuFwCfgSelectItem ( ) { if (QemuFwCfgIsAvailable ()) { - MmioWrite16 (mFwCfgSelectorAddress, SwapBytes16 ((UINT16)QemuFwCfgItem)); + MmioWrite16 (QemuGetFwCfgSelectorAddress (), SwapBytes16 ((UINT16)QemuFwCfgItem)); } } @@ -86,30 +147,30 @@ MmioReadBytes ( #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined (MDE_CPU_LOONGARCH64) while (Ptr < End) { - *(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress); + *(UINT64 *)Ptr = MmioRead64 (QemuGetFwCfgDataAddress ()); Ptr += 8; } if (Left & 4) { - *(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress); + *(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ()); Ptr += 4; } #else while (Ptr < End) { - *(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress); + *(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ()); Ptr += 4; } #endif if (Left & 2) { - *(UINT16 *)Ptr = MmioRead16 (mFwCfgDataAddress); + *(UINT16 *)Ptr = MmioRead16 (QemuGetFwCfgDataAddress ()); Ptr += 2; } if (Left & 1) { - *Ptr = MmioRead8 (mFwCfgDataAddress); + *Ptr = MmioRead8 (QemuGetFwCfgDataAddress ()); } } @@ -162,9 +223,9 @@ DmaTransferBytes ( // This will fire off the transfer. // #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined (MDE_CPU_LOONGARCH64) - MmioWrite64 (mFwCfgDmaAddress, SwapBytes64 ((UINT64)&Access)); + MmioWrite64 (QemuGetFwCfgDmaAddress (), SwapBytes64 ((UINT64)&Access)); #else - MmioWrite32 ((UINT32)(mFwCfgDmaAddress + 4), SwapBytes32 ((UINT32)&Access)); + MmioWrite32 ((UINT32)(QemuGetFwCfgDmaAddress () + 4), SwapBytes32 ((UINT32)&Access)); #endif // @@ -233,7 +294,7 @@ MmioWriteBytes ( UINTN Idx; for (Idx = 0; Idx < Size; ++Idx) { - MmioWrite8 (mFwCfgDataAddress, ((UINT8 *)Buffer)[Idx]); + MmioWrite8 (QemuGetFwCfgDataAddress (), ((UINT8 *)Buffer)[Idx]); } } diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf index f2596f270e..8e191f2d22 100644 --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf @@ -40,6 +40,7 @@ [LibraryClasses] BaseLib BaseMemoryLib DebugLib + HobLib IoLib UefiBootServicesTableLib diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h index d7d645f700..6101e15b21 100644 --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h @@ -16,6 +16,17 @@ extern UINTN mFwCfgSelectorAddress; extern UINTN mFwCfgDataAddress; extern UINTN mFwCfgDmaAddress; +#define FW_CONFIG_RESOURCE_HOB_GUID \ + { \ + 0x3cc47b04, 0x0d3e, 0xaa64, { 0x06, 0xa6, 0x4b, 0xdc, 0x9a, 0x2c, 0x61, 0x19 } \ + } + +typedef struct { + UINTN FwCfgSelectorAddress; + UINTN FwCfgDataAddress; + UINTN FwCfgDmaAddress; +} QEMU_FW_CFG_RESOURCE; + /** Reads firmware configuration bytes into a buffer @@ -96,6 +107,69 @@ EFIAPI IN UINTN Size ); +/** + Build firmware configure resource HOB. + + @param[in] FwCfgResource A pointer to firmware configure resource. + + @retval NULL +**/ +VOID +QemuBuildFwCfgResourceHob ( + IN QEMU_FW_CFG_RESOURCE *FwCfgResource + ); + +/** + Get firmware configure resource HOB. + + @param VOID + + @retval FwCfgResource The firmware configure resouce in HOB. +**/ +QEMU_FW_CFG_RESOURCE * +QemuGetFwCfgResourceHob ( + VOID + ); + +/** + To get firmware configure selector address. + + @param VOID + + @retval firmware configure selector address +**/ +UINTN +EFIAPI +QemuGetFwCfgSelectorAddress ( + VOID + ); + +/** + To get firmware configure Data address. + + @param VOID + + @retval firmware configure data address +**/ +UINTN +EFIAPI +QemuGetFwCfgDataAddress ( + VOID + ); + +/** + To get firmware DMA address. + + @param VOID + + @retval firmware DMA address +**/ +UINTN +EFIAPI +QemuGetFwCfgDmaAddress ( + VOID + ); + /** Slow READ_BYTES_FUNCTION. **/ diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c index 4844a42a36..2d5055a76e 100644 --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c @@ -32,23 +32,92 @@ READ_BYTES_FUNCTION *InternalQemuFwCfgReadBytes = MmioReadBytes; WRITE_BYTES_FUNCTION *InternalQemuFwCfgWriteBytes = MmioWriteBytes; SKIP_BYTES_FUNCTION *InternalQemuFwCfgSkipBytes = MmioSkipBytes; +/** + To get firmware configure selector address. + + @param VOID + + @retval firmware configure selector address +**/ +UINTN +EFIAPI +QemuGetFwCfgSelectorAddress ( + VOID + ) +{ + return mFwCfgSelectorAddress; +} + +/** + To get firmware configure Data address. + + @param VOID + + @retval firmware configure data address +**/ +UINTN +EFIAPI +QemuGetFwCfgDataAddress ( + VOID + ) +{ + return mFwCfgDataAddress; +} + +/** + To get firmware DMA address. + + @param VOID + + @retval firmware DMA address +**/ +UINTN +EFIAPI +QemuGetFwCfgDmaAddress ( + VOID + ) +{ + return mFwCfgDmaAddress; +} + + 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; + 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; + QEMU_FW_CFG_RESOURCE *FwCfgResource; + + // + // Check whether the Qemu firmware configure resources HOB has been created, + // if so use the resources in the HOB. + // + FwCfgResource = QemuGetFwCfgResourceHob (); + if (FwCfgResource != NULL) { + mFwCfgSelectorAddress = FwCfgResource->FwCfgSelectorAddress; + mFwCfgDataAddress = FwCfgResource->FwCfgDataAddress; + mFwCfgDmaAddress = FwCfgResource->FwCfgDmaAddress; + + if (mFwCfgDmaAddress != 0) { + InternalQemuFwCfgReadBytes = DmaReadBytes; + InternalQemuFwCfgWriteBytes = DmaWriteBytes; + InternalQemuFwCfgSkipBytes = DmaSkipBytes; + } + + return RETURN_SUCCESS; + } Status = gBS->LocateProtocol ( &gFdtClientProtocolGuid, -- 2.27.0