On Fri, 26 Apr 2024 at 10:29, Chao Li <[email protected]> wrote:
>
> Added the PEI stage library for QemuFwCfgMmioLib, which uses the FDT to
> find the fw_cfg and parse it.
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755
>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Jiewen Yao <[email protected]>
> Cc: Gerd Hoffmann <[email protected]>
> Co-authored-by: Xianglai Li <[email protected]>
> Signed-off-by: Chao Li <[email protected]>
> ---
> .../Library/QemuFwCfgLib/QemuFwCfgMmioPei.c | 235 ++++++++++++++++++
> .../QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf | 52 ++++
> 2 files changed, 287 insertions(+)
> create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c
> create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c
> new file mode 100644
> index 0000000000..055148de8e
> --- /dev/null
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c
> @@ -0,0 +1,235 @@
> +/** @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/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/QemuFwCfgLib.h>
> +
> +#include <libfdt.h>
> +
> +#include "QemuFwCfgLibMmioInternal.h"
> +
> +/**
> + To get firmware configure selector address.
> +
> + @param VOID
> +
> + @retval firmware configure selector address
> +**/
> +UINTN
> +EFIAPI
> +QemuGetFwCfgSelectorAddress (
> + VOID
> + )
> +{
> + QEMU_FW_CFG_RESOURCE *FwCfgResource;
> +
> + FwCfgResource = QemuGetFwCfgResourceHob ();
> + ASSERT (FwCfgResource != NULL);
> +
> + return FwCfgResource->FwCfgSelectorAddress;
> +}
> +
> +/**
> + To get firmware configure Data address.
> +
> + @param VOID
> +
> + @retval firmware configure data address
> +**/
> +UINTN
> +EFIAPI
> +QemuGetFwCfgDataAddress (
> + VOID
> + )
> +{
> + QEMU_FW_CFG_RESOURCE *FwCfgResource;
> +
> + FwCfgResource = QemuGetFwCfgResourceHob ();
> + ASSERT (FwCfgResource != NULL);
> +
> + return FwCfgResource->FwCfgDataAddress;
> +}
> +
> +/**
> + To get firmware DMA address.
> +
> + @param VOID
> +
> + @retval firmware DMA address
> +**/
> +UINTN
> +EFIAPI
> +QemuGetFwCfgDmaAddress (
> + VOID
> + )
> +{
> + QEMU_FW_CFG_RESOURCE *FwCfgResource;
> +
> + FwCfgResource = QemuGetFwCfgResourceHob ();
> + ASSERT (FwCfgResource != NULL);
> +
> + return FwCfgResource->FwCfgDmaAddress;
> +}
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgInitialize (
> + VOID
> + )
> +{
> + VOID *DeviceTreeBase;
> + INT32 Node;
> + INT32 Prev;
> + CONST CHAR8 *Type;
> + INT32 Len;
> + CONST UINT64 *Reg;
> + UINT64 FwCfgSelectorAddress;
> + UINT64 FwCfgSelectorSize;
> + UINT64 FwCfgDataAddress;
> + UINT64 FwCfgDataSize;
> + UINT64 FwCfgDmaAddress;
> + UINT64 FwCfgDmaSize;
> + QEMU_FW_CFG_RESOURCE *FwCfgResource;
> + VOID *Buffer;
> +
> + //
> + // Check whether the Qemu firmware configure resources HOB has been
> created,
> + // if so use the resources in the HOB.
> + //
> + FwCfgResource = QemuGetFwCfgResourceHob ();
> + if (FwCfgResource != NULL) {
> + return RETURN_SUCCESS;
> + }
> +
> + DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> + ASSERT (DeviceTreeBase != NULL);
> + //
> + // Make sure we have a valid device tree blob
> + //
> + ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> + //
> + // Create resouce memory
> + //
> + Buffer = AllocatePages(EFI_SIZE_TO_PAGES (sizeof (QEMU_FW_CFG_RESOURCE)));
> + ASSERT (Buffer != NULL);
> + ZeroMem (Buffer, sizeof (QEMU_FW_CFG_RESOURCE));
> +
> + FwCfgResource = (QEMU_FW_CFG_RESOURCE *)Buffer;
> +
You will need to respin after all, so please incorporate the fixes I
proposed on v4
> + for (Prev = 0; ; Prev = Node) {
> + Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
> + if (Node < 0) {
> + break;
> + }
> +
> + //
> + // Check for memory node
> + //
> + Type = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
> + if ((Type) &&
and here
> + (AsciiStrnCmp (Type, "qemu,fw-cfg-mmio", Len) == 0))
> + {
> + //
> + // Get the 'reg' property of this node. For now, we will assume
> + // two 8 byte quantities for base and size, respectively.
> + //
> + Reg = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
> + if ((Reg != 0) && (Len == (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);
> +
> + FwCfgResource->FwCfgSelectorAddress = FwCfgSelectorAddress;
> + FwCfgResource->FwCfgDataAddress = 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));
> + FwCfgResource->FwCfgDmaAddress = FwCfgDmaAddress;
> + } else {
> + FwCfgDmaAddress = 0;
> + }
> +
> + if ((FwCfgSelectorAddress != 0) && (FwCfgDataAddress != 0)) {
> + UINT32 Signature;
> +
Please move this declaration to the function scope.
> + //
> + // Select Item Signature
> + //
> + MmioWrite16 (FwCfgSelectorAddress, SwapBytes16
> ((UINT16)QemuFwCfgItemSignature));
> +
> + //
> + // Readout the Signature.
> + //
> + Signature = MmioRead32 (FwCfgDataAddress);
> +
> + if (Signature != SIGNATURE_32 ('Q', 'E', 'M', 'U')) {
> + FwCfgResource->FwCfgDataAddress = 0;
> + FwCfgResource->FwCfgSelectorAddress = 0;
> + FwCfgResource->FwCfgDmaAddress = 0;
> + QemuBuildFwCfgResourceHob (FwCfgResource);
> + }
> +
> + //
> + // Build the firmware configure resource HOB.
> + //
> + QemuBuildFwCfgResourceHob (FwCfgResource);
This logic does not look right at all. What is the intent here? Should
the HOB only be built if the signature check passes?
> + }
> +
> + break;
> + } else {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to parse FDT QemuCfg node\n",
> + __func__
> + ));
> + break;
> + }
> + }
> + }
> +
> + return RETURN_SUCCESS;
> +}
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf
> new file mode 100644
> index 0000000000..a3dc9a03da
> --- /dev/null
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf
> @@ -0,0 +1,52 @@
> +## @file
> +#
> +# Stateful, implicitly initialized fw_cfg library.
> +#
> +# 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
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 1.29
> + BASE_NAME = QemuFwCfgPeiLib
> + FILE_GUID = CDF9A9D5-7422-4DCB-B41D-607151AD320B
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = QemuFwCfgLib|PEIM
> +
> + CONSTRUCTOR = QemuFwCfgInitialize
> +
> +#
> +# The following information is for reference only and not required by the
> build
> +# tools.
> +#
> +# VALID_ARCHITECTURES = LOONGARCH64
> +#
> +
You can drop the comment block above
> +[Sources]
> + QemuFwCfgLibMmio.c
> + QemuFwCfgMmioPei.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + OvmfPkg/OvmfPkg.dec
> + EmbeddedPkg/EmbeddedPkg.dec
> +
Please put in alphabetical order
> +[LibraryClasses]
> + BaseLib
> + BaseMemoryLib
> + DebugLib
> + HobLib
> + IoLib
> + MemoryAllocationLib
> + PcdLib
> +
> +[Pcd]
> + gUefiOvmfPkgTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> +
> +[Guids]
> + gQemuFirmwareResourceHobGuid
> --
> 2.27.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118388): https://edk2.groups.io/g/devel/message/118388
Mute This Topic: https://groups.io/mt/105746793/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
