Hi Ard, Thanks, Chao On 2024/4/26 09:20, Chao Li wrote: > > Hi Ard, > > On 2024/4/25 21:02, Ard Biesheuvel wrote: >> On Thu, 25 Apr 2024 at 14:13, Chao Li 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 >>> 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 | 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 >>> #include >>> >>> +#include >>> +#include >>> + >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -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)". > I will save the real HOB data in the next version. > >>> + sizeof (QEMU_FW_CFG_RESOURCE) >>> + ); >>> +} >>> + >>> +/** >>> + Get firmware configure resource in HOB. >>> + >>> + @param VOID >>> + >>> + @retval FwCfgResource The firmware configure resouce in HOB. >> resource > All right. >>> + 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. > OK. >>> + 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 >>> >>> >>> >>> >>> >>> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118309): https://edk2.groups.io/g/devel/message/118309 Mute This Topic: https://groups.io/mt/105728768/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-