On Thu, 25 Apr 2024 at 14:13, Chao Li <lic...@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+tianoc...@kernel.org> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Leif Lindholm <quic_llind...@quicinc.com> > Cc: Sami Mujawar <sami.muja...@arm.com> > Cc: Sunil V L <suni...@ventanamicro.com> > Cc: Andrei Warkentin <andrei.warken...@intel.com> > Signed-off-by: Chao Li <lic...@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? > + sizeof (QEMU_FW_CFG_RESOURCE) > + ); > +} > + > +/** > + Get firmware configure resource in HOB. > + > + @param VOID > + > + @retval FwCfgResource The firmware configure resouce in HOB. resource > + 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 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118300): https://edk2.groups.io/g/devel/message/118300 Mute This Topic: https://groups.io/mt/105728768/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-