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 898C7940694 for ; Tue, 30 Apr 2024 01:19:30 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=pdgNu8AT4YaeBPd7wFbrnomyuhHFg0dbDqH0SPTldh0=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20240206; t=1714439969; v=1; b=D/vA/DZC/4zUNRc0rwY+lR8PiIUvJS2UdV7cYGbFZY8aOBkxmU7YSvV66ve/07YGg5t6wEvq aw695189OCIXELpiN8em0EmClGVD+yTn/ol9LlMXmqKI5HP1IfAWpignWC81sNrkrGa+iWglNYb i/TNsjMzfyuB3DQBW1plkZLOfkcS94kyMikQ6UxjbnGxCyzHS2zPRKD7EHaN9UjCRgXbiHH4LDj KCakK2BBVGDiZoj0iO1rdamBmKd4/glQM+7DxdeO86+YuEOkUR5WZZ/rWn3MX69b2sbt9D3xTG6 HxEa6JhxUoHm/wDmjI4jGiS/3dRSV70y7+BM7X099Fh5g== X-Received: by 127.0.0.2 with SMTP id 89YZYY7687511xk9Q2qGnVaz; Mon, 29 Apr 2024 18:19:29 -0700 X-Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by mx.groups.io with SMTP id smtpd.web10.4792.1714439962432987157 for ; Mon, 29 Apr 2024 18:19:23 -0700 X-Received: from loongson.cn (unknown [10.40.24.149]) by gateway (Coremail) with SMTP id _____8BxN+kVRzBmzzgFAA--.5144S3; Tue, 30 Apr 2024 09:19:17 +0800 (CST) X-Received: from [10.40.24.149] (unknown [10.40.24.149]) by localhost.localdomain (Coremail) with SMTP id AQAAf8DxtFYQRzBmqbcKAA--.9181S3; Tue, 30 Apr 2024 09:19:12 +0800 (CST) Message-ID: <8fa560a1-0eba-4f7d-9c42-1a592752e015@loongson.cn> Date: Tue, 30 Apr 2024 09:19:12 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v4 4/8] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version To: devel@edk2.groups.io, ardb@kernel.org Cc: Ard Biesheuvel , Jiewen Yao , Gerd Hoffmann , Xianglai Li References: <20240426082827.68489-1-lichao@loongson.cn> <20240426082920.68922-1-lichao@loongson.cn> From: "Chao Li" In-Reply-To: X-CM-TRANSID: AQAAf8DxtFYQRzBmqbcKAA--.9181S3 X-CM-SenderInfo: xolfxt3r6o00pqjv00gofq/1tbiAQADCGYvCpMKTAAAsc 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 18:19:23 -0700 Resent-From: lichao@loongson.cn Reply-To: devel@edk2.groups.io,lichao@loongson.cn List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: NZgeH5hCQIS4CXbKoUEL82Qsx7686176AA= Content-Type: multipart/alternative; boundary="------------SBikQkzmQS0FcRoViFx3Nky3" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b="D/vA/DZC"; 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 --------------SBikQkzmQS0FcRoViFx3Nky3 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Hi Ard, OK, I will submit the V5 today and make the adjustments according to=20 your suggestions. Thanks, Chao 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.in= f >> >> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c b/OvmfPkg/L= ibrary/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 right= s 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 c= reated, >> + // if so use the resources in the HOB. >> + // >> + FwCfgResource =3D QemuGetFwCfgResourceHob (); >> + if (FwCfgResource !=3D NULL) { >> + return RETURN_SUCCESS; >> + } >> + >> + DeviceTreeBase =3D (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseA= ddress); >> + 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_RESOU= RCE))); >> + 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 MMIO= range is >> + // expressible as a MAX_UINTN. The form below is mathematically >> + // equivalent, and it also prevents any unsigned overflow befor= e the >> + // 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", FwCfgDmaAddr= ess)); >> + 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)QemuF= wCfgItemSignature)); >> + >> + // >> + // 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 is=20 a QEMU lib, if you don't think so, I can move the built HOB out of this=20 check. > >> + } >> + >> + 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/Ovmf= Pkg/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. >> +# Copyright (c) 2024 Loongson Technology Corporation Limited. All righ= ts 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-607151AD32= 0B >> + 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 t= he 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 (#118396): https://edk2.groups.io/g/devel/message/118396 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- --------------SBikQkzmQS0FcRoViFx3Nky3 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Hi Ard,

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


=
Thanks,
Chao
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 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 <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Co-authored-by: Xianglai Li <lixianglai@loongson.cn>
Signed-off-by: Chao Li <lichao@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 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 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 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 is a QEMU lib, if you don't think so, I can move the built HOB out of this check.


+        }
+
+        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 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
_._,_._,_
Groups.io Links:

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

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

_._,_._,_
--------------SBikQkzmQS0FcRoViFx3Nky3--