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 735007803DA for ; Mon, 29 Apr 2024 13:11:52 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=e3oj9k2ifS+V+YO9TOyoy/hGgC9sAcHQr2zufTk/Vls=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc: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=1714396311; v=1; b=QUHil0qRdm4USHkLUq/3YH6Pt9FHPZECrPLEX8a/ua4rwolkFXCX0z38iFY3FpYO533n/kY2 M4q9+Vkge9wvIB8+zzNxddQUUa3A89PwKVULec4Jw9+uuISn6ibOxVcFTnAoFD8pg0HCSFdGYWe lfwnzWqK+2PSFgQV0AghVlI/jDeIuivQlHmNNZ9MLPLq3D2UdwTeCwR3IS0ufGdIfGErRe7vGs1 qi4wnQI1YydPwXkC1X/MWaFFMf/LRdTDWCvuOBJwJaMAZlPPGye/eSAD9g644sfMQRu4WFHp93Q 68zV+ofQnnvJYfeD7Dtlgqzol9+hwR1JvtJHQnJrGFZGg== X-Received: by 127.0.0.2 with SMTP id XP6rYY7687511x96Lq78JmfL; Mon, 29 Apr 2024 06:11:51 -0700 X-Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by mx.groups.io with SMTP id smtpd.web10.20935.1714396309761172534 for ; Mon, 29 Apr 2024 06:11:50 -0700 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id B9A45CE0B76 for ; Mon, 29 Apr 2024 13:11:46 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id E0378C116B1 for ; Mon, 29 Apr 2024 13:11:45 +0000 (UTC) X-Received: by mail-lj1-f181.google.com with SMTP id 38308e7fff4ca-2dfb4ea2bbfso23060421fa.2 for ; Mon, 29 Apr 2024 06:11:45 -0700 (PDT) X-Gm-Message-State: edatYuvRzfscoVzp7rKr5Smdx7686176AA= X-Google-Smtp-Source: AGHT+IHf0Rd0B0wWN5/oGpy6PNHXf6Pd71a9h2AnaPdTy/nPv0AKEGUpFWK3MN8f7LGmbzCqwXJ9t9OueYeeeJFG6S8= X-Received: by 2002:a2e:be24:0:b0:2df:98c3:95dd with SMTP id z36-20020a2ebe24000000b002df98c395ddmr6201540ljq.22.1714396304282; Mon, 29 Apr 2024 06:11:44 -0700 (PDT) MIME-Version: 1.0 References: <20240426082827.68489-1-lichao@loongson.cn> <20240426082920.68922-1-lichao@loongson.cn> In-Reply-To: <20240426082920.68922-1-lichao@loongson.cn> From: "Ard Biesheuvel" Date: Mon, 29 Apr 2024 15:11:33 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v4 4/8] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version To: Chao Li Cc: devel@edk2.groups.io, Ard Biesheuvel , Jiewen Yao , Gerd Hoffmann , Xianglai Li 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 06:11:50 -0700 Resent-From: ardb@kernel.org Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=QUHil0qR; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=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 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=4755 > > 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.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.
> + (C) Copyright 2021 Hewlett Packard Enterprise Development LP
> + Copyright (c) 2024 Loongson Technology Corporation Limited. All rights 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 = 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.
> +# Copyright (c) 2024 Loongson Technology Corporation Limited. All rights reserved.
> +# > +# 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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-