From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id C0B3A780091 for ; Tue, 30 Apr 2024 02:15:14 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=ExnW26xhWQdX6Q/i9jfhZKTPh/I6yJTCwrxATJwoDoM=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:From:To:Cc:Reply-To:References:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20240206; t=1714443313; v=1; b=XElp9Ciofl5W2l5IxXh9b3fiFY1XxfkWKJdCr7v34aFMSoyMDR7/PYiqhGDFn2WIkp0AhR1+ nl0iWM6msr5foz90Gs6OlQKsprzM3ACaTII0XlYuoYeC7PZol36oxR/5xJOumUiiFzEQI/ghBuF +Ov32zojzhSYgd7HbJfVIQypSFB8jHNQ0SpXQ+MKdKWcD9y0R5Ze5iWZs8kes9iyldujyWAMTHY q32OwsAErsY8Y4y/vCdyeQCujeNQ3tYdUC2v/f5vNnqAZqSNYLzt2PQPU0HCPAEiLcYBuEauAuj LWlJyTMlkg8HvW+Q1K3aYbUvGO6k/sWXCQnCU+YUzR6PA== X-Received: by 127.0.0.2 with SMTP id hw8UYY7687511xh7gIY1UflV; Mon, 29 Apr 2024 19:15:13 -0700 X-Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by mx.groups.io with SMTP id smtpd.web10.5824.1714443311058587260 for ; Mon, 29 Apr 2024 19:15:12 -0700 X-Received: from loongson.cn (unknown [10.40.24.149]) by gateway (Coremail) with SMTP id _____8Dx2ekqVDBmljwFAA--.5144S3; Tue, 30 Apr 2024 10:15:06 +0800 (CST) X-Received: from [10.40.24.149] (unknown [10.40.24.149]) by localhost.localdomain (Coremail) with SMTP id AQAAf8CxX1UlVDBmUMIKAA--.9006S3; Tue, 30 Apr 2024 10:15:01 +0800 (CST) Message-ID: <6178f421-dd75-4e02-9890-36e8c64c8a25@loongson.cn> Date: Tue, 30 Apr 2024 10:15:01 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v4 4/8] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version From: "Chao Li" To: devel@edk2.groups.io, ardb@kernel.org Cc: Ard Biesheuvel , Jiewen Yao , Gerd Hoffmann , Xianglai Li Reply-To: devel@edk2.groups.io,lichao@loongson.cn References: <20240426082827.68489-1-lichao@loongson.cn> <20240426082920.68922-1-lichao@loongson.cn> <17CAEA1048BCCAB8.26557@groups.io> In-Reply-To: <17CAEA1048BCCAB8.26557@groups.io> X-CM-TRANSID: AQAAf8CxX1UlVDBmUMIKAA--.9006S3 X-CM-SenderInfo: xolfxt3r6o00pqjv00gofq/1tbiAQADCGYvCpMLygAAsb X-Coremail-Antispam: 1Uk129KBjDUn29KB7ZKAUJUUUUU529EdanIXcx71UUUUU7KY7 ZEXasCq-sGcSsGvfJ3UbIjqfuFe4nvWSU5nxnvy29KBjDU0xBIdaVrnUUvcSsGvfC2Kfnx nUUI43ZEXa7xR_UUUUUUUUU== Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Mon, 29 Apr 2024 19:15:12 -0700 Resent-From: lichao@loongson.cn List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: VqP6e0VCrw1jbARbwyvMB5xfx7686176AA= Content-Type: multipart/alternative; boundary="------------lLrnN5JHIhu0eBo0LuV0KB0i" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=XElp9Cio; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io --------------lLrnN5JHIhu0eBo0LuV0KB0i Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Hi Ard, Thanks, Chao On 2024/4/30 09:19, Chao Li wrote: > > Hi Ard, > > OK, I will submit the V5 today and make the adjustments according to=20 > your suggestions. > > On 2024/4/29 21:11, Ard Biesheuvel wrote: >> On Fri, 26 Apr 2024 at 10:29, Chao Li 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=3D4755 >>> >>> Cc: Ard Biesheuvel >>> Cc: Jiewen Yao >>> Cc: Gerd Hoffmann >>> Co-authored-by: Xianglai Li >>> Signed-off-by: Chao Li >>> --- >>> .../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.i= nf >>> >>> 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. >>> + (C) Copyright 2021 Hewlett Packard Enterprise Development LP
>>> + Copyright (c) 2024 Loongson Technology Corporation Limited. All righ= ts reserved.
>>> + >>> + SPDX-License-Identifier: BSD-2-Clause-Patent >>> +**/ >>> + >>> +#include >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#include >>> + >>> +#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 =3D QemuGetFwCfgResourceHob (); >>> + ASSERT (FwCfgResource !=3D 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 =3D QemuGetFwCfgResourceHob (); >>> + ASSERT (FwCfgResource !=3D 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 =3D QemuGetFwCfgResourceHob (); >>> + ASSERT (FwCfgResource !=3D 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 =3D QemuGetFwCfgResourceHob (); >>> + if (FwCfgResource !=3D NULL) { >>> + return RETURN_SUCCESS; >>> + } >>> + >>> + DeviceTreeBase =3D (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBase= Address); >>> + ASSERT (DeviceTreeBase !=3D NULL); >>> + // >>> + // Make sure we have a valid device tree blob >>> + // >>> + ASSERT (fdt_check_header (DeviceTreeBase) =3D=3D 0); >>> + >>> + // >>> + // Create resouce memory >>> + // >>> + Buffer =3D AllocatePages(EFI_SIZE_TO_PAGES (sizeof (QEMU_FW_CFG_RESO= URCE))); >>> + ASSERT (Buffer !=3D NULL); >>> + ZeroMem (Buffer, sizeof (QEMU_FW_CFG_RESOURCE)); >>> + >>> + FwCfgResource =3D (QEMU_FW_CFG_RESOURCE *)Buffer; >>> + >> You will need to respin after all, so please incorporate the fixes I >> proposed on v4 >> >>> + for (Prev =3D 0; ; Prev =3D Node) { >>> + Node =3D fdt_next_node (DeviceTreeBase, Prev, NULL); >>> + if (Node < 0) { >>> + break; >>> + } >>> + >>> + // >>> + // Check for memory node >>> + // >>> + Type =3D fdt_getprop (DeviceTreeBase, Node, "compatible", &Len); >>> + if ((Type) && >> and here >> >>> + (AsciiStrnCmp (Type, "qemu,fw-cfg-mmio", Len) =3D=3D 0)) >>> + { >>> + // >>> + // Get the 'reg' property of this node. For now, we will assume >>> + // two 8 byte quantities for base and size, respectively. >>> + // >>> + Reg =3D fdt_getprop (DeviceTreeBase, Node, "reg", &Len); >>> + if ((Reg !=3D 0) && (Len =3D=3D (2 * sizeof (UINT64)))) { >>> + FwCfgDataAddress =3D SwapBytes64 (Reg[0]); >>> + FwCfgDataSize =3D 8; >>> + FwCfgSelectorAddress =3D FwCfgDataAddress + FwCfgDataSize; >>> + FwCfgSelectorSize =3D 2; >>> + >>> + // >>> + // The following ASSERT()s express >>> + // >>> + // Address + Size - 1 <=3D MAX_UINTN >>> + // >>> + // for both registers, that is, that the last byte in each MMI= O range is >>> + // expressible as a MAX_UINTN. The form below is mathematicall= y >>> + // equivalent, and it also prevents any unsigned overflow befo= re the >>> + // comparison. >>> + // >>> + ASSERT (FwCfgSelectorAddress <=3D MAX_UINTN - FwCfgSelectorSiz= e + 1); >>> + ASSERT (FwCfgDataAddress <=3D MAX_UINTN - FwCfgDataSize = + 1); >>> + >>> + FwCfgResource->FwCfgSelectorAddress =3D FwCfgSelectorAddress; >>> + FwCfgResource->FwCfgDataAddress =3D FwCfgDataAddress; >>> + >>> + DEBUG (( >>> + DEBUG_INFO, >>> + "Found FwCfg @ 0x%Lx/0x%Lx\n", >>> + FwCfgSelectorAddress, >>> + FwCfgDataAddress >>> + )); >>> + >>> + if (SwapBytes64 (Reg[1]) >=3D 0x18) { >>> + FwCfgDmaAddress =3D FwCfgDataAddress + 0x10; >>> + FwCfgDmaSize =3D 0x08; >>> + >>> + // >>> + // See explanation above. >>> + // >>> + ASSERT (FwCfgDmaAddress <=3D MAX_UINTN - FwCfgDmaSize + 1); >>> + >>> + DEBUG ((DEBUG_INFO, "Found FwCfg DMA @ 0x%Lx\n", FwCfgDmaAdd= ress)); >>> + FwCfgResource->FwCfgDmaAddress =3D FwCfgDmaAddress; >>> + } else { >>> + FwCfgDmaAddress =3D 0; >>> + } >>> + >>> + if ((FwCfgSelectorAddress !=3D 0) && (FwCfgDataAddress !=3D 0)= ) { >>> + UINT32 Signature; >>> + >> Please move this declaration to the function scope. > OK >>> + // >>> + // Select Item Signature >>> + // >>> + MmioWrite16 (FwCfgSelectorAddress, SwapBytes16 ((UINT16)Qemu= FwCfgItemSignature)); >>> + >>> + // >>> + // Readout the Signature. >>> + // >>> + Signature =3D MmioRead32 (FwCfgDataAddress); >>> + >>> + if (Signature !=3D SIGNATURE_32 ('Q', 'E', 'M', 'U')) { >>> + FwCfgResource->FwCfgDataAddress =3D 0; >>> + FwCfgResource->FwCfgSelectorAddress =3D 0; >>> + FwCfgResource->FwCfgDmaAddress =3D 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? > I think it should check passes and then built the HOB, after all this=20 > is a QEMU lib, if you don't think so, I can move the built HOB out of=20 > this check. I get it, there are some logic error, I will fix in V5. I think it=20 should be more better to built the HOB when the 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/Ovm= fPkg/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 rig= hts reserved.
>>> +# >>> +# SPDX-License-Identifier: BSD-2-Clause-Patent >>> +# >>> +## >>> + >>> +[Defines] >>> + INF_VERSION =3D 1.29 >>> + BASE_NAME =3D QemuFwCfgPeiLib >>> + FILE_GUID =3D CDF9A9D5-7422-4DCB-B41D-607151AD3= 20B >>> + MODULE_TYPE =3D BASE >>> + VERSION_STRING =3D 1.0 >>> + LIBRARY_CLASS =3D QemuFwCfgLib|PEIM >>> + >>> + CONSTRUCTOR =3D QemuFwCfgInitialize >>> + >>> +# >>> +# The following information is for reference only and not required by = the build >>> +# tools. >>> +# >>> +# VALID_ARCHITECTURES =3D LOONGARCH64 >>> +# >>> + >> You can drop the comment block above > OK >>> +[Sources] >>> + QemuFwCfgLibMmio.c >>> + QemuFwCfgMmioPei.c >>> + >>> +[Packages] >>> + MdePkg/MdePkg.dec >>> + OvmfPkg/OvmfPkg.dec >>> + EmbeddedPkg/EmbeddedPkg.dec >>> + >> Please put in alphabetical order > OK >>> +[LibraryClasses] >>> + BaseLib >>> + BaseMemoryLib >>> + DebugLib >>> + HobLib >>> + IoLib >>> + MemoryAllocationLib >>> + PcdLib >>> + >>> +[Pcd] >>> + gUefiOvmfPkgTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress >>> + >>> +[Guids] >>> + gQemuFirmwareResourceHobGuid >>> -- >>> 2.27.0 >>> >=20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118400): https://edk2.groups.io/g/devel/message/118400 Mute This Topic: https://groups.io/mt/105746793/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --------------lLrnN5JHIhu0eBo0LuV0KB0i Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Hi Ard,


=
Thanks,
Chao
On 2024/4/30 09:19, Chao Li wrote:

Hi Ard,

OK, I will submit the V5 today and make the adjustments according to your suggestions.

On 2024/4/29 21:11, Ard Biesheuvel wrote:
On Fri, 26 Apr 2024 at 10:29=
, Chao Li <lichao@loongson.cn> wrote:

Added the PEI stage librar=
y for QemuFwCfgMmioLib, which uses the FDT to
find the fw_cfg and parse it.

BZ: https://bugzilla.tianocore.org/show_bug.=
cgi?id=3D4755

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <=
;jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kr=
axel@redhat.com>
Co-authored-by: Xianglai Li &=
lt;lixianglai@loongson.cn>
Signed-off-by: Chao Li <l=
ichao@loongson.cn>
---
 .../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/Libr=
ary/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 r=
eserved.<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 =3D QemuGetFwCfgResourceHob ();
+  ASSERT (FwCfgResource !=3D 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 =3D QemuGetFwCfgResourceHob ();
+  ASSERT (FwCfgResource !=3D 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 =3D QemuGetFwCfgResourceHob ();
+  ASSERT (FwCfgResource !=3D 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 crea=
ted,
+  // if so use the resources in the HOB.
+  //
+  FwCfgResource =3D QemuGetFwCfgResourceHob ();
+  if (FwCfgResource !=3D NULL) {
+    return RETURN_SUCCESS;
+  }
+
+  DeviceTreeBase =3D (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddr=
ess);
+  ASSERT (DeviceTreeBase !=3D NULL);
+  //
+  // Make sure we have a valid device tree blob
+  //
+  ASSERT (fdt_check_header (DeviceTreeBase) =3D=3D 0);
+
+  //
+  // Create resouce memory
+  //
+  Buffer =3D AllocatePages(EFI_SIZE_TO_PAGES (sizeof (QEMU_FW_CFG_RESOURCE=
)));
+  ASSERT (Buffer !=3D NULL);
+  ZeroMem (Buffer, sizeof (QEMU_FW_CFG_RESOURCE));
+
+  FwCfgResource =3D (QEMU_FW_CFG_RESOURCE *)Buffer;
+

You will need to respin afte=
r all, so please incorporate the fixes I
proposed on v4


+  for (Prev =3D 0; ; Prev=
 =3D Node) {
+    Node =3D fdt_next_node (DeviceTreeBase, Prev, NULL);
+    if (Node < 0) {
+      break;
+    }
+
+    //
+    // Check for memory node
+    //
+    Type =3D fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
+    if ((Type) &&

and here


+        (AsciiStrnCmp (Ty=
pe, "qemu,fw-cfg-mmio", Len) =3D=3D 0))
+    {
+      //
+      // Get the 'reg' property of this node. For now, we will assume
+      // two 8 byte quantities for base and size, respectively.
+      //
+      Reg =3D fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
+      if ((Reg !=3D 0) && (Len =3D=3D (2 * sizeof (UINT64)))) {
+        FwCfgDataAddress     =3D SwapBytes64 (Reg[0]);
+        FwCfgDataSize        =3D 8;
+        FwCfgSelectorAddress =3D FwCfgDataAddress + FwCfgDataSize;
+        FwCfgSelectorSize    =3D 2;
+
+        //
+        // The following ASSERT()s express
+        //
+        //   Address + Size - 1 <=3D MAX_UINTN
+        //
+        // for both registers, that is, that the last byte in each MMIO ra=
nge is
+        // expressible as a MAX_UINTN. The form below is mathematically
+        // equivalent, and it also prevents any unsigned overflow before t=
he
+        // comparison.
+        //
+        ASSERT (FwCfgSelectorAddress <=3D MAX_UINTN - FwCfgSelectorSize=
 + 1);
+        ASSERT (FwCfgDataAddress     <=3D MAX_UINTN - FwCfgDataSize    =
 + 1);
+
+        FwCfgResource->FwCfgSelectorAddress =3D FwCfgSelectorAddress;
+        FwCfgResource->FwCfgDataAddress     =3D FwCfgDataAddress;
+
+        DEBUG ((
+          DEBUG_INFO,
+          "Found FwCfg @ 0x%Lx/0x%Lx\n",
+          FwCfgSelectorAddress,
+          FwCfgDataAddress
+          ));
+
+        if (SwapBytes64 (Reg[1]) >=3D 0x18) {
+          FwCfgDmaAddress =3D FwCfgDataAddress + 0x10;
+          FwCfgDmaSize    =3D 0x08;
+
+          //
+          // See explanation above.
+          //
+          ASSERT (FwCfgDmaAddress <=3D MAX_UINTN - FwCfgDmaSize + 1);
+
+          DEBUG ((DEBUG_INFO, "Found FwCfg DMA @ 0x%Lx\n", FwCfgDmaAddress=
));
+          FwCfgResource->FwCfgDmaAddress  =3D FwCfgDmaAddress;
+        } else {
+          FwCfgDmaAddress =3D 0;
+        }
+
+        if ((FwCfgSelectorAddress !=3D 0) && (FwCfgDataAddress !=
=3D 0)) {
+          UINT32  Signature;
+

Please move this declaration=
 to the function scope.
OK
+          //
+          // Select Item Signature
+          //
+          MmioWrite16 (FwCfgSelectorAddress, SwapBytes16 ((UINT16)QemuFwCf=
gItemSignature));
+
+          //
+          // Readout the Signature.
+          //
+          Signature =3D MmioRead32 (FwCfgDataAddress);
+
+          if (Signature !=3D SIGNATURE_32 ('Q', 'E', 'M', 'U')) {
+            FwCfgResource->FwCfgDataAddress     =3D 0;
+            FwCfgResource->FwCfgSelectorAddress =3D 0;
+            FwCfgResource->FwCfgDmaAddress      =3D 0;
+            QemuBuildFwCfgResourceHob (FwCfgResource);
+          }
+
+          //
+          // Build the firmware configure resource HOB.
+          //
+          QemuBuildFwCfgResourceHob (FwCfgResource);

This logic does not look rig=
ht at all. What is the intent here? Should
the HOB only be built if the signature check passes?
I think it should check passes and then built the HOB, after all this is a QEMU lib, if you don't think so, I can move the built HOB out of this check.
I get it, there are some logic error, I will fix in V5. I think it should be more better to built the HOB when the 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.<B=
R>
+#  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights =
reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    =3D 1.29
+  BASE_NAME                      =3D QemuFwCfgPeiLib
+  FILE_GUID                      =3D CDF9A9D5-7422-4DCB-B41D-607151AD320B
+  MODULE_TYPE                    =3D BASE
+  VERSION_STRING                 =3D 1.0
+  LIBRARY_CLASS                  =3D QemuFwCfgLib|PEIM
+
+  CONSTRUCTOR                    =3D QemuFwCfgInitialize
+
+#
+# The following information is for reference only and not required by the =
build
+# tools.
+#
+#  VALID_ARCHITECTURES           =3D LOONGARCH64
+#
+

You can drop the comment blo=
ck above
OK
+[Sources]
+  QemuFwCfgLibMmio.c
+  QemuFwCfgMmioPei.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+

Please put in alphabetical o=
rder
OK
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  HobLib
+  IoLib
+  MemoryAllocationLib
+  PcdLib
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
+
+[Guids]
+  gQemuFirmwareResourceHobGuid
--
2.27.0
=20
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#118400) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------lLrnN5JHIhu0eBo0LuV0KB0i--