Re: [edk2-devel] [PATCH v3 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
Hi Ard, Thanks, Chao On 2024/4/26 09:20, Chao Li wrote: Hi Ard, On 2024/4/25 21:02, Ard Biesheuvel wrote: On Thu, 25 Apr 2024 at 14:13, Chao Li 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 Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Leif Lindholm Cc: Sami Mujawar Cc: Sunil V L Cc: Andrei Warkentin Signed-off-by: Chao Li --- .../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 #include +#include +#include + #include #include #include +#include #include #include #include @@ -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 ( +, +(VOID *), This looks wrong: why are you taking the address of the stack variable rather than the address of the resource descriptor? It only saves the pointer of FwCfgResource, and the memory space has been created in the PEI constructor. Do you mean saving all contents of FwCfgResource in the HOB? The following line is indeed wrong. if only save the pointer, the size should be "sizeof (UINT64)". I will save the real HOB data in the next version. +sizeof (QEMU_FW_CFG_RESOURCE) +); +} + +/** + Get firmware configure resource in HOB. + + @param VOID + + @retval FwCfgResourceThe firmware configure resouce in HOB. resource All right. + 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 (); 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. OK. + 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)));
Re: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature based on PFP 1.06 spec
Hi EDKII stewards, Could you please review the libspdm license? The libspdm(https://github.com/DMTF/libspdm) is a implementation that follows the DMTF SPDM(https://www.dmtf.org/standards/spdm) spec. And the libspdm library is under DMTF repo. The license is: https://github.com/DMTF/libspdm/blob/main/LICENSE.md Thanks, Wenxing -Original Message- From: Yao, Jiewen Sent: Sunday, April 21, 2024 10:31 AM To: Hou, Wenxing ; devel@edk2.groups.io; Andrew Fish ; Leif Lindholm ; Kinney, Michael D ; Liming Gao ; Sean Brogan ; Joey Vagedes ; Liu, Zhiguang ; Kumar, Rahul R Subject: RE: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature based on PFP 1.06 spec All series: Reviewed-by: Jiewen Yao Dear Steward member Do you have any concern on adding libspdm (https://github.com/DMTF/libspdm) as one more submodule? Thank you Yao, Jiewen > -Original Message- > From: Hou, Wenxing > Sent: Thursday, April 18, 2024 6:16 PM > To: devel@edk2.groups.io; Andrew Fish ; Leif Lindholm > ; Kinney, Michael D > ; Liming Gao ; > Sean Brogan ; Joey Vagedes > ; Liu, Zhiguang ; > Kumar, Rahul R ; Yao, Jiewen > > Subject: RE: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature > based on PFP 1.06 spec > > Dear EDKII reviewers: > > Thank you for your previous review of this patch set. > Currently, five patches have been reviewed by. > > But there are five patches need review. > Patch1: MdePkg: Add SPDM1.2 support. > Patch2: MdePkg: Add TCG PFP 1.06 support. > Patch4: MdeModulePkg/Variable: Add TCG SPDM device measurement > update > Patch8: .gitmodule: Add libspdm submodule for EDKII > Patch10: ReadMe.rst: Add libspdm submodule license > > Could you please review the PATCH v4? > > PS: Jiewen has reviewed all the PATCH. And I have fixed his feedback in PATCH > v4. > Jiewen has no questions about all the patches anymore. > > Thanks, > Wenxing > > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Wenxing > Hou > Sent: Thursday, April 18, 2024 5:28 PM > To: devel@edk2.groups.io > Cc: Andrew Fish ; Leif Lindholm > ; Kinney, Michael D > ; Liming Gao ; > Sean Brogan ; Joey Vagedes > ; Liu, Zhiguang ; > Kumar, Rahul R ; Yao, Jiewen > > Subject: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature > based on PFP > 1.06 spec > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2479 > > In PFP spec 1.06, platform firmware records the device certificate and > device measurement for each SPDM responder. > This PATCH set implement the DeviceSecurityLib to support spdm device > Authentication and Measurement. > > Libspdm as submodule is to support DeviceSecurity feature: > https://github.com/DMTF/libspdm > > TCG PFP spec 1.06: > https://trustedcomputinggroup.org/resource/pc-client-specific-platform > - > firmware-profile-specification/ > > The POC branch: > https://github.com/tianocore/edk2-staging/tree/DeviceSecurity > > And the PATCH set has passed the EDKII CI: > https://github.com/tianocore/edk2/pull/5508 > > v2 changes: > - Fix typo: PcdEnableSpdmDeviceAuthenticaion -> > PcdEnableSpdmDeviceAuthentication > v3 changes: > - Add new patch 10: Update ReadMe.rst for libspdm submodule license > v4 changes: > - Update submodule libspdm to latest tag > > PATCH 3: Reviewed-by: Liming Gao PATCH 5: > Reviewed-by: Jiewen Yao PATCH 6: Reviewed-by: > Jiewen Yao PATCH 7: Reviewed-by: Joey Vagedes > PATCH 9: Reviewed-by: Jiewen Yao > > > Cc: Andrew Fish > Cc: Leif Lindholm > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Sean Brogan > Cc: Joey Vagedes > Cc: Zhiguang Liu > Cc: Rahul Kumar > Cc: Jiewen Yao > Signed-off-by: Wenxing Hou > > Wenxing Hou (10): > MdePkg: Add SPDM1.2 support. > MdePkg: Add TCG PFP 1.06 support. > MdePkg: Add devAuthBoot GlobalVariable > MdeModulePkg/Variable: Add TCG SPDM device measurement update > SecurityPkg: Add TCG PFP 1.06 support. > SecurityPkg: add DeviceSecurity support > .pytool/CISettings.py: add libspdm submodule. > .gitmodule: Add libspdm submodule for EDKII > SecurityPkg: Add libspdm submodule > ReadMe.rst: Add libspdm submodule license > > .gitmodules |3 + > .pytool/CISettings.py |2 + > MdeModulePkg/MdeModulePkg.dec |5 + > .../Variable/RuntimeDxe/Measurement.c | 38 +- > .../RuntimeDxe/VariableRuntimeDxe.inf |3 + > .../RuntimeDxe/VariableSmmRuntimeDxe.inf |3 + > MdePkg/Include/Guid/GlobalVariable.h |8 +- > MdePkg/Include/Guid/ImageAuthentication.h |5 +- > MdePkg/Include/IndustryStandard/Spdm.h| 1112 - > .../IndustryStandard/UefiTcgPlatform.h| 186 ++- > ReadMe.rst|1 + > .../OsStub/CryptlibWrapper/CryptlibWrapper.c | 970 ++ > .../CryptlibWrapper/CryptlibWrapper.inf | 38 + >
Re: [edk2-devel] [PATCH v3 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
Hi Ard, Thanks, Chao On 2024/4/25 21:02, Ard Biesheuvel wrote: On Thu, 25 Apr 2024 at 14:13, Chao Li 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 Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Leif Lindholm Cc: Sami Mujawar Cc: Sunil V L Cc: Andrei Warkentin Signed-off-by: Chao Li --- .../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 #include +#include +#include + #include #include #include +#include #include #include #include @@ -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 ( +, +(VOID *), This looks wrong: why are you taking the address of the stack variable rather than the address of the resource descriptor? It only saves the pointer of FwCfgResource, and the memory space has been created in the PEI constructor. Do you mean saving all contents of FwCfgResource in the HOB? The following line is indeed wrong. if only save the pointer, the size should be "sizeof (UINT64)". +sizeof (QEMU_FW_CFG_RESOURCE) +); +} + +/** + Get firmware configure resource in HOB. + + @param VOID + + @retval FwCfgResourceThe firmware configure resouce in HOB. resource All right. + 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 (); 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. OK. + 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))); + MmioWrite64 (QemuGetFwCfgDmaAddress (), SwapBytes64 ((UINT64))); #else - MmioWrite32
Re: [edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files
Hi Ard, Thanks, Chao On 2024/4/25 20:58, Ard Biesheuvel wrote: On Thu, 25 Apr 2024 at 14:13, Chao Li wrote: Separate QemuFwCfgLibMmio.c into two files named QemuFwCfgLibMmio.c and QemuFwCfgLibMmioDxe.c, added a new header named QemuFwCfgLibMmioInternal.h for MMIO version. Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc"). BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4755 Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Leif Lindholm Cc: Sami Mujawar Cc: Sunil V L Cc: Andrei Warkentin Signed-off-by: Chao Li --- .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c | 194 +- .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf | 4 +- .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h | 179 .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c | 153 ++ 4 files changed, 340 insertions(+), 190 deletions(-) create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c index 115a210759..dc949c8e26 100644 --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c @@ -1,10 +1,9 @@ /** @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. Please only claim copyright for code that you wrote, not for code that you just moved between files. OK, I will remove Loongson copyright in this patch and add it in patch 2 or 3. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -20,63 +19,7 @@ #include -STATIC UINTN mFwCfgSelectorAddress; -STATIC UINTN mFwCfgDataAddress; -STATIC UINTN mFwCfgDmaAddress; - -/** - Reads firmware configuration bytes into a buffer - - @param[in] SizeSize in bytes to read - @param[in] Buffer Buffer to store data into (OPTIONAL if Size is 0) - -**/ -typedef -VOID(EFIAPI READ_BYTES_FUNCTION)( - IN UINTN Size, - IN VOID *Buffer OPTIONAL - ); - -/** - Writes bytes from a buffer to firmware configuration - - @param[in] SizeSize in bytes to write - @param[in] Buffer Buffer to transfer data from (OPTIONAL if Size is 0) - -**/ -typedef -VOID(EFIAPI WRITE_BYTES_FUNCTION)( - IN UINTN Size, - IN VOID *Buffer OPTIONAL - ); - -/** - Skips bytes in firmware configuration - - @param[in] Size Size in bytes to skip - -**/ -typedef -VOID(EFIAPI SKIP_BYTES_FUNCTION)( - IN UINTN Size - ); - -// -// Forward declaration of the two implementations we have. -// -STATIC READ_BYTES_FUNCTION MmioReadBytes; -STATIC WRITE_BYTES_FUNCTION MmioWriteBytes; -STATIC SKIP_BYTES_FUNCTION MmioSkipBytes; -STATIC READ_BYTES_FUNCTION DmaReadBytes; -STATIC WRITE_BYTES_FUNCTION DmaWriteBytes; -STATIC SKIP_BYTES_FUNCTION DmaSkipBytes; - -// -// These correspond to the implementation we detect at runtime. -// -STATIC READ_BYTES_FUNCTION *InternalQemuFwCfgReadBytes = MmioReadBytes; -STATIC WRITE_BYTES_FUNCTION *InternalQemuFwCfgWriteBytes = MmioWriteBytes; -STATIC SKIP_BYTES_FUNCTION *InternalQemuFwCfgSkipBytes = MmioSkipBytes; +#include "QemuFwCfgLibMmioInternal.h" /** Returns a boolean indicating if the firmware configuration interface @@ -97,126 +40,6 @@ QemuFwCfgIsAvailable ( return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0); } -RETURN_STATUS -EFIAPI -QemuFwCfgInitialize ( - VOID - ) -{ - EFI_STATUS Status; - FDT_CLIENT_PROTOCOL *FdtClient; - CONST UINT64 *Reg; - UINT32 RegSize; - UINTNAddressCells, SizeCells; - UINT64 FwCfgSelectorAddress; - UINT64 FwCfgSelectorSize; - UINT64 FwCfgDataAddress; - UINT64 FwCfgDataSize; - UINT64 FwCfgDmaAddress; - UINT64 FwCfgDmaSize; - - Status = gBS->LocateProtocol ( - , - NULL, - (VOID **) - ); - ASSERT_EFI_ERROR (Status); - - Status = FdtClient->FindCompatibleNodeReg ( -FdtClient, -"qemu,fw-cfg-mmio", -(CONST VOID **), -, -, - -); - if (EFI_ERROR (Status)) { -DEBUG (( - DEBUG_WARN, - "%a: No 'qemu,fw-cfg-mmio' compatible DT node found (Status == %r)\n", - __func__, - Status - )); -return EFI_SUCCESS; - } - - ASSERT (AddressCells == 2); - ASSERT (SizeCells == 2); - ASSERT (RegSize == 2 * sizeof (UINT64)); - - FwCfgDataAddress = SwapBytes64 (Reg[0]); - FwCfgDataSize= 8; -
Re: [edk2-devel] 回复: [PATCH 0/7] General Updates based on UEFI 2.10 and PI 1.8 Specification
Hi Felix/ Liming, Thank you for your comments. Patch 6 has been updated to only focus on consolidating the revision macros. Patch Link: https://edk2.groups.io/g/devel/message/118246?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105721897 PR Link: https://github.com/tianocore/edk2/pull/5569 Thank you, Sachin. -Original Message- From: Felix Polyudov Sent: Tuesday, April 23, 2024 10:58 PM To: gaoliming ; Sachin Ganesh ; devel@edk2.groups.io Cc: zhiguang@intel.com; michael.d.kin...@intel.com; ardb+tianoc...@kernel.org; kra...@redhat.com; jiewen@intel.com; erdemak...@google.com; min.m...@intel.com; thomas.lenda...@amd.com; Dhanaraj V Subject: RE: [EXTERNAL] 回复: [PATCH 0/7] General Updates based on UEFI 2.10 and PI 1.8 Specification I think patch 6 can be updated to introduce unified PI specification versioning macros without incrementing the minor revision, by changing PI_SPECIFICATION_MINOR_REVISION back to 70. This will ensure compliance with the versioning schema introduced in PI 1.7B without changing the PI support level. (the macros were introduced by PIWG mantis 2101) -Original Message- From: gaoliming Sent: Tuesday, April 23, 2024 10:49 AM To: Sachin Ganesh ; devel@edk2.groups.io Cc: zhiguang@intel.com; michael.d.kin...@intel.com; ardb+tianoc...@kernel.org; kra...@redhat.com; jiewen@intel.com; erdemak...@google.com; min.m...@intel.com; thomas.lenda...@amd.com; Felix Polyudov ; Dhanaraj V Subject: [EXTERNAL] 回复: [PATCH 0/7] General Updates based on UEFI 2.10 and PI 1.8 Specification **CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.** Except for Patch 6/7, others are good to me. Reviewed-by: Liming Gao I suggest to merge others first. The patch 6/7 to update PI version from 1.7 to 1.8 can be discussed first. Thanks Liming > -邮件原件- > 发件人: Sachin Ganesh > 发送时间: 2024年4月20日 5:46 > 收件人: devel@edk2.groups.io > 抄送: gaolim...@byosoft.com.cn; zhiguang@intel.com; > michael.d.kin...@intel.com; ardb+tianoc...@kernel.org; > kra...@redhat.com; jiewen@intel.com; erdemak...@google.com; > min.m...@intel.com; thomas.lenda...@amd.com; Felix Polyudov > ; Dhanaraj V ; Sachin Ganesh > > 主题: [PATCH 0/7] General Updates based on UEFI 2.10 and PI 1.8 > Specification > > This series of patches are for general updates to MdePkg and > MdeModulePkg based on UEFI 2.10 and PI 1.8 Specifications > > Sachin Ganesh (7): > MdePkg: Add definition for NVMe Over Fabric Device Path > MdePkg: Add new Resource Attributes defined in PI 1.8 Spec > MdePkg: Define Unaccepted Memory Type > MdeModulePkg: Use newly defined Unaccepted Memory Type > MdePkg: Update Delayed Dispatch PPI as per PI 1.8 Spec > MdePkg: Update to PI 1.8 Revision > OvmfPkg: Use newly defined Unaccepted Memory Type > > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 10 +++--- > MdeModulePkg/Core/Dxe/Mem/Page.c | 38 > ++-- > MdeModulePkg/Include/Pi/PrePiDxeCis.h| 25 - > MdeModulePkg/Include/Pi/PrePiHob.h | 20 --- > MdePkg/Include/Pi/PiDxeCis.h | 19 +- > MdePkg/Include/Pi/PiHob.h| 14 +++- > MdePkg/Include/Pi/PiMmCis.h | 6 ++-- > MdePkg/Include/Pi/PiMultiPhase.h | 6 > MdePkg/Include/Pi/PiPeiCis.h | 6 ++-- > MdePkg/Include/Pi/PiSmmCis.h | 2 +- > MdePkg/Include/Ppi/DelayedDispatch.h | 24 - > MdePkg/Include/Protocol/DevicePath.h | 22 > OvmfPkg/AmdSevDxe/AmdSevDxe.c| 4 +-- > OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c | 8 ++--- > OvmfPkg/Library/PeilessStartupLib/Hob.c | 4 +-- > OvmfPkg/Library/PlatformInitLib/IntelTdx.c | 8 ++--- > OvmfPkg/PlatformPei/AmdSev.c | 4 +-- > 17 files changed, 108 insertions(+), 112 deletions(-) delete mode > 100644 MdeModulePkg/Include/Pi/PrePiDxeCis.h > delete mode 100644 MdeModulePkg/Include/Pi/PrePiHob.h > > -- > 2.24.1.windows.2 > -The information contained in this message may be confidential and > proprietary to American Megatrends (AMI). This communication is > intended to > be read only by the individual or entity to whom it is addressed or by their > designee. If the reader of this message is not the intended recipient, > you are > on notice that any distribution of this message, in any form, is > strictly prohibited. Please promptly notify the sender by reply e-mail > or by telephone > at 770-246-8600, and then delete or destroy all copies of the transmission= -The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended
[edk2-devel] Cannot access SerialPortPoll function
Hello Everyone, I'm working on a project with edk2, and these guys are trying to port edk2 to STM32MP25 platforms. I have a problem with the module SerialPortLib, specifically the function *SerialPortPoll*. I made a test that adds a line of debug in this function and adds a MmioWrite8 to a forbidden address of my platform but I don't see anything. This demonstrates that I cannot access this function (this function is not loaded). Do you know why and how to solve this? Here is my function SerialPortPoll BOOLEAN EFIAPI SerialPortPoll ( VOID ) { DEBUG ((DEBUG_INFO, "In SerialPortPo\n")); MmioWrite8(0x8000, 0xff); return FALSE; } And here is my function SerialPortWrite that works well because I can see the boot log message UINTN EFIAPI SerialPortWrite ( IN UINT8 *Buffer, IN UINTN NumberOfBytes ) { UINT8 *CONST Final = [NumberOfBytes]; while (Buffer < Final) { // Wait until UART able to accept another char while (!(MmioRead32 (USART2_BASE + R_UART_ISR) & (1<<7))) { } MmioWrite8 (USART2_BASE + R_UART_TDR, *Buffer++); } DEBUG ((DEBUG_INFO, "end SerialPortWrite\n")); return NumberOfBytes; } Thanks -- PHAN Ba Gia Bao Etudiant en 5A STI - INSA Centre Val de Loire -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118302): https://edk2.groups.io/g/devel/message/118302 Mute This Topic: https://groups.io/mt/105739991/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Synchronous Exception at ArmGicDxe
It is near to the GicDistributorBase (0x4ac1) of STM32MP25 Le jeu. 25 avr. 2024 à 02:21, Andrew (EFI) Fish a écrit : > The fault address is 0x0004AC14. Is that in the address range of > the GIC for this platform? What does that Physical address map to you on > the STM32MP25? > > Thanks, > > Andrew Fish > > On Apr 21, 2024, at 10:07 PM, Ba Gia Bao Phan > wrote: > > Hi Everyone, > > I'm working on a project with edk2, and these guys are trying to port edk2 > to STM32MP25 platforms. I had no issue compiling and booting the image on > my device. Then I've come across an issue at very early stages of booting. > I was given a Synchronous Exception listed briefly below. > > Synchronous Exception at *0x00010A63501C* > PC 0x00010A63501C (0x00010A633000+0x201C) [ 0] ArmGicDxe.dll > PC 0x00010A6350CC (0x00010A633000+0x20CC) [ 0] ArmGicDxe.dll > PC 0x00010A63657C (0x00010A633000+0x357C) [ 0] ArmGicDxe.dll > PC 0x00010A85853C (0x00010A851000+0x753C) [ 1] DxeCore.dll > PC 0x00010A8666DC (0x00010A851000+0x000156DC) [ 1] DxeCore.dll > PC 0x00010A85BF14 (0x00010A851000+0xAF14) [ 1] DxeCore.dll > PC 0x84006EBC > PC 0x8400700C > > [ 0] > /local/home/phanbagi/Documents/RPi3/Build/STM32MP25/DEBUG_GCC5/AARCH64/ArmPkg/Drivers/ArmGic/ArmGicDxe/DEBUG/ArmGicDxe.dll > [ 1] > /local/home/phanbagi/Documents/RPi3/Build/STM32MP25/DEBUG_GCC5/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll > > X0 0x0004AC14 X1 0x X2 0x > X3 0x4F46A76E3FDDA605 > X4 0x083D1B53F41229AD X5 0x003CF238 X6 0x0080 > X7 0x > X8 0x00010A850688 X9 0x0004 X10 0x000109E9F000 > X11 0x00010A297FFF > X12 0x X13 0x000E X14 0x00FF > X15 0x0002 > X16 0x00010AFFFAD0 X17 0x00395790 X18 0x > X19 0x0004AC14 > X20 0x00010A638000 X21 0x00010A638000 X22 0x00010A638000 > X23 0x00010A6380A8 > X24 0x00010A876448 X25 0x00010A873026 X26 0x00010A877000 > X27 0x800E > X28 0x0001 FP 0x00010AFFFA50 LR 0x00010A6350CC > > V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF V1 0xFF80FFD0 > 00010AFFFA30 > V2 0x0007 0E001858 V3 0x1000 > 0100 > V4 0x V5 0x > > V6 0x V7 0x > > V8 0x V9 0x > > V10 0x V11 0x > > V12 0x V13 0x > > V14 0x V15 0x > > V16 0x6145998C70863CB0 9004002B4AC68640 V17 0x646586E820A5D596 > AA30249EC11F1469 > V18 0x5756252D20E5B421 336EEAEC2122B3AB V19 0x6285C083AAECD063 > 020A72C37700322A > V20 0x40846C68BA2085FB 20966526601C44BB V21 0x818208204CD0A89D > 6D84E94813E14B24 > V22 0x2362CBA9962AF073 E83ED49CD4A13F98 V23 0x0C1123CD73F5C5B0 > C3E9222891467015 > V24 0x02B1194895858177 1B028BE1A860D73B V25 0x542AB00936183D90 > 752F1D1CCC4D2345 > V26 0x03D02013962356F3 0CCA6F83842045EA V27 0x9DF9D2A28D44915E > 2892460B223FD84C > V28 0x61A2CC120A809CD8 AC33034803D74C10 V29 0x7A1089709810B030 > 88B3298EEB68B450 > V30 0xD20F81310E0896C0 20E2BA370E2AC6F9 V31 0x6C3BBEA030B2BB50 > 2C2A91AAF201EF50 > > SP 0x00010AFFFA50 ELR 0x00010A63501C SPSR 0x43C5 FPSR > 0x > ESR 0x9605 FAR 0x0004AC14 > > ESR : EC 0x25 IL 0x1 ISS 0x0005 > > Data abort: Translation fault, first level > > > By using objdump to determine where the wrong source code is, I found out > that the program stopped around the code below. > > UINT32 > EFIAPI > MmioRead32 ( > IN UINTN Address > ) > { > UINT32 Value; > BOOLEAN Flag; > > ASSERT ((Address & 3) == 0); > > Flag = FilterBeforeMmIoRead (FilterWidth32, Address, ); > if (Flag) { > MemoryFence (); > > if (IsTdxGuest ()) { > Value = TdMmioRead32 (Address); > } else { > Value = *(volatile UINT32 *)Address; > } > > MemoryFence (); > } > > FilterAfterMmIoRead (FilterWidth32, Address, ); > > return Value; > } > > Output of objdump: > UINT32 > EFIAPI > MmioRead32 ( > IN UINTN Address > ) > { > 1fec: a9be7bfd stp x29, x30, [sp, #-32]! > 1ff0: 910003fd mov x29, sp > 1ff4: f9000bf3 str x19, [sp, #16] > 1ff8: aa0003f3 mov x19, x0 > UINT32 Value; > BOOLEAN Flag; > > ASSERT ((Address & 3) == 0); > 1ffc: f240041f tst x0, #0x3 > 2000: 54e0 b.eq 201c // b.none > 2004: b002 adrp x2, 3000 > 2008: b000 adrp x0, 3000 > 200c: 913ac442 add x2, x2, #0xeb1 > 2010: 913b1000 add x0, x0, #0xec4 > 2014:
Re: [edk2-devel] [PATCH] OvmfPkg: Set PcdCpuMaxLogicalProcessorNumber in OvmfXen
On Thu, Apr 25, 2024 at 11:42:01AM +0100, Alejandro Vallejo wrote: > Hi, > > On 25/04/2024 08:31, Gerd Hoffmann wrote: > > On Wed, Apr 24, 2024 at 02:36:32PM +0100, Alejandro Vallejo wrote: > >> Bump the compile-time constant for maximum processor count from 64 to 128 > >> in order to allow that many vCPUs to be brought online on Xen guests with > >> the default OVMF configuration. > > > >> + # UefiCpuPkg PCDs related to initial AP bringup and general AP > >> management. > >> + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|128 > > > > Note that this is a dynamic PCD, so you can set it at runtime to the > > number of vcpus present in the VM. See MaxCpuCountInitialization() in > > OvmfPkg/PlatformPei/Platform.c for example. > > > > take care, > > Gerd > > > > Thanks for the heads up. Do you mean setting it at runtime through > fw_cfg? I saw PlatformMaxCpuCountInitialization() providing some > customizability, but Xen's toolstack doesn't provide fw_cfg at the > moment so it can't (as far as I've seen) use it. We don't need fw_cfg to set a PCD at runtime. It's a bit more complicated than setting it at build time, but we can always ask Xen how many vcpu we have and set the PCD accordingly. This is something that can happen in OvmfPkg/XenPlatformPei module. But to be honest, I don't know if it's worth it, because I don't know the downside of having a higher value for PcdCpuMaxLogicalProcessorNumber. Cheers, -- Anthony PERARD -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118304): https://edk2.groups.io/g/devel/message/118304 Mute This Topic: https://groups.io/mt/105721898/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg: Set PcdCpuMaxLogicalProcessorNumber in OvmfXen
Hi, > It's a bit more complicated than setting it at build time, but we can > always ask Xen how many vcpu we have and set the PCD accordingly. This > is something that can happen in OvmfPkg/XenPlatformPei module. Exactly. > But to be honest, I don't know if it's worth it, because I don't know the > downside of having a higher value for PcdCpuMaxLogicalProcessorNumber. Well, the downside is that you have to keep ovmf and xen in sync manually. Your call. If you prefer to do it that way I have no objections, just wanted point out the alternative approach. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118301): https://edk2.groups.io/g/devel/message/118301 Mute This Topic: https://groups.io/mt/105721898/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
On Thu, 25 Apr 2024 at 14:13, Chao Li 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 > Cc: Jiewen Yao > Cc: Gerd Hoffmann > Cc: Leif Lindholm > Cc: Sami Mujawar > Cc: Sunil V L > Cc: Andrei Warkentin > Signed-off-by: Chao Li > --- > .../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 > #include > > +#include > +#include > + > #include > #include > #include > +#include > #include > #include > #include > @@ -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 ( > +, > +(VOID *), 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 FwCfgResourceThe 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 (); 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))); > + MmioWrite64 (QemuGetFwCfgDmaAddress (), SwapBytes64 ((UINT64))); > #else > - MmioWrite32 ((UINT32)(mFwCfgDmaAddress + 4), SwapBytes32 > ((UINT32))); > + MmioWrite32
Re: [edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files
On Thu, 25 Apr 2024 at 14:13, Chao Li wrote: > > Separate QemuFwCfgLibMmio.c into two files named QemuFwCfgLibMmio.c and > QemuFwCfgLibMmioDxe.c, added a new header named > QemuFwCfgLibMmioInternal.h for MMIO version. > > Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc"). > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755 > > Cc: Ard Biesheuvel > Cc: Jiewen Yao > Cc: Gerd Hoffmann > Cc: Leif Lindholm > Cc: Sami Mujawar > Cc: Sunil V L > Cc: Andrei Warkentin > Signed-off-by: Chao Li > --- > .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c | 194 +- > .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf | 4 +- > .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h | 179 > .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c | 153 ++ > 4 files changed, 340 insertions(+), 190 deletions(-) > create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h > create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c > > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c > b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c > index 115a210759..dc949c8e26 100644 > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c > @@ -1,10 +1,9 @@ > /** @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. > Please only claim copyright for code that you wrote, not for code that you just moved between files. >SPDX-License-Identifier: BSD-2-Clause-Patent > **/ > @@ -20,63 +19,7 @@ > > #include > > -STATIC UINTN mFwCfgSelectorAddress; > -STATIC UINTN mFwCfgDataAddress; > -STATIC UINTN mFwCfgDmaAddress; > - > -/** > - Reads firmware configuration bytes into a buffer > - > - @param[in] SizeSize in bytes to read > - @param[in] Buffer Buffer to store data into (OPTIONAL if Size is 0) > - > -**/ > -typedef > -VOID(EFIAPI READ_BYTES_FUNCTION)( > - IN UINTN Size, > - IN VOID *Buffer OPTIONAL > - ); > - > -/** > - Writes bytes from a buffer to firmware configuration > - > - @param[in] SizeSize in bytes to write > - @param[in] Buffer Buffer to transfer data from (OPTIONAL if Size is 0) > - > -**/ > -typedef > -VOID(EFIAPI WRITE_BYTES_FUNCTION)( > - IN UINTN Size, > - IN VOID *Buffer OPTIONAL > - ); > - > -/** > - Skips bytes in firmware configuration > - > - @param[in] Size Size in bytes to skip > - > -**/ > -typedef > -VOID(EFIAPI SKIP_BYTES_FUNCTION)( > - IN UINTN Size > - ); > - > -// > -// Forward declaration of the two implementations we have. > -// > -STATIC READ_BYTES_FUNCTION MmioReadBytes; > -STATIC WRITE_BYTES_FUNCTION MmioWriteBytes; > -STATIC SKIP_BYTES_FUNCTION MmioSkipBytes; > -STATIC READ_BYTES_FUNCTION DmaReadBytes; > -STATIC WRITE_BYTES_FUNCTION DmaWriteBytes; > -STATIC SKIP_BYTES_FUNCTION DmaSkipBytes; > - > -// > -// These correspond to the implementation we detect at runtime. > -// > -STATIC READ_BYTES_FUNCTION *InternalQemuFwCfgReadBytes = MmioReadBytes; > -STATIC WRITE_BYTES_FUNCTION *InternalQemuFwCfgWriteBytes = MmioWriteBytes; > -STATIC SKIP_BYTES_FUNCTION *InternalQemuFwCfgSkipBytes = MmioSkipBytes; > +#include "QemuFwCfgLibMmioInternal.h" > > /** >Returns a boolean indicating if the firmware configuration interface > @@ -97,126 +40,6 @@ QemuFwCfgIsAvailable ( >return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0); > } > > -RETURN_STATUS > -EFIAPI > -QemuFwCfgInitialize ( > - VOID > - ) > -{ > - EFI_STATUS Status; > - FDT_CLIENT_PROTOCOL *FdtClient; > - CONST UINT64 *Reg; > - UINT32 RegSize; > - UINTNAddressCells, SizeCells; > - UINT64 FwCfgSelectorAddress; > - UINT64 FwCfgSelectorSize; > - UINT64 FwCfgDataAddress; > - UINT64 FwCfgDataSize; > - UINT64 FwCfgDmaAddress; > - UINT64 FwCfgDmaSize; > - > - Status = gBS->LocateProtocol ( > - , > - NULL, > - (VOID **) > - ); > - ASSERT_EFI_ERROR (Status); > - > - Status = FdtClient->FindCompatibleNodeReg ( > -FdtClient, > -"qemu,fw-cfg-mmio", > -(CONST VOID **), > -, > -, > - > -); > - if (EFI_ERROR (Status)) { > -DEBUG (( > - DEBUG_WARN, > - "%a: No 'qemu,fw-cfg-mmio' compatible DT node found (Status == %r)\n", > - __func__, > - Status > - )); > -return EFI_SUCCESS; > - } > - > - ASSERT (AddressCells == 2); > - ASSERT (SizeCells
[edk2-devel] [PATCH v3 6/7] OvmfPkg/RiscVVirt: Enable QemuFwCfgMmioDxeLib.inf
Enable QemuFwCfgMmioDxeLib.inf in RiscVVirtQemu.dsc Build-tested only (with "RiscVVirtQemu.dsc"). BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755 Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Sunil V L Cc: Andrei Warkentin Signed-off-by: Chao Li --- OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc index 27f24648e8..e0ed6fb9bc 100644 --- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc +++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc @@ -78,7 +78,7 @@ [LibraryClasses.common] # Virtio Support VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf - QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf + QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf -- 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118297): https://edk2.groups.io/g/devel/message/118297 Mute This Topic: https://groups.io/mt/105728777/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 7/7] OvmfPkg: Remove QemuFwCfgLibMmio.inf
All of platforms are switching to QemuFwCfgMmioDxeLib.inf, remove QemuFwCfgLibMmio.inf now. BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755 Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Gerd Hoffmann Signed-off-by: Chao Li --- .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf | 51 --- 1 file changed, 51 deletions(-) delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf deleted file mode 100644 index 8e191f2d22..00 --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf +++ /dev/null @@ -1,51 +0,0 @@ -## @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= 0x00010005 - BASE_NAME = QemuFwCfgLib - FILE_GUID = B271F41F-B841-48A9-BA8D-545B4BC2E2BF - MODULE_TYPE= BASE - VERSION_STRING = 1.0 - LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER UEFI_DRIVER - - CONSTRUCTOR= QemuFwCfgInitialize - -# -# The following information is for reference only and not required by the build -# tools. -# -# VALID_ARCHITECTURES = ARM AARCH64 RISCV64 LOONGARCH64 -# - -[Sources] - QemuFwCfgLibMmio.c - QemuFwCfgMmioDxe.c - -[Packages] - MdePkg/MdePkg.dec - OvmfPkg/OvmfPkg.dec - EmbeddedPkg/EmbeddedPkg.dec - -[LibraryClasses] - BaseLib - BaseMemoryLib - DebugLib - HobLib - IoLib - UefiBootServicesTableLib - -[Protocols] - gFdtClientProtocolGuid## CONSUMES - -[Depex] - gFdtClientProtocolGuid -- 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118298): https://edk2.groups.io/g/devel/message/118298 Mute This Topic: https://groups.io/mt/105728778/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 5/7] ArmVirtPkg: Enable QemuFwCfgMmioDxeLib.inf
Enable QemuFwCfgMmioDxeLib.inf in ArmVirtQemu.dsc and ArmVirtQemuKernel.dsc. Build-tested only (with "ArmVirtQemu.dsc"). BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755 Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Leif Lindholm Cc: Sami Mujawar Signed-off-by: Chao Li --- ArmVirtPkg/ArmVirtQemu.dsc | 2 +- ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc index e48c75b5e9..987b8bb238 100644 --- a/ArmVirtPkg/ArmVirtQemu.dsc +++ b/ArmVirtPkg/ArmVirtQemu.dsc @@ -60,7 +60,7 @@ [LibraryClasses.common] # Virtio Support VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf - QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf + QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index 668a65ba64..efe2df97bd 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -57,7 +57,7 @@ [LibraryClasses.common] # Virtio Support VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf - QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf + QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf -- 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118296): https://edk2.groups.io/g/devel/message/118296 Mute This Topic: https://groups.io/mt/105728772/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 4/7] OvmfPkg: Copy the same new INF as QemuFwCfgLibMmio.inf
Copy QemuFwCfgLibMmio.inf to QemuFwCfgMmioDxeLib.inf, QemuFwCfgLibMmio.inf will be deleted when all platforms switching is completed. BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755 Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Gerd Hoffmann Signed-off-by: Chao Li --- .../QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf | 51 +++ 1 file changed, 51 insertions(+) create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf new file mode 100644 index 00..8e191f2d22 --- /dev/null +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf @@ -0,0 +1,51 @@ +## @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= 0x00010005 + BASE_NAME = QemuFwCfgLib + FILE_GUID = B271F41F-B841-48A9-BA8D-545B4BC2E2BF + MODULE_TYPE= BASE + VERSION_STRING = 1.0 + LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER UEFI_DRIVER + + CONSTRUCTOR= QemuFwCfgInitialize + +# +# The following information is for reference only and not required by the build +# tools. +# +# VALID_ARCHITECTURES = ARM AARCH64 RISCV64 LOONGARCH64 +# + +[Sources] + QemuFwCfgLibMmio.c + QemuFwCfgMmioDxe.c + +[Packages] + MdePkg/MdePkg.dec + OvmfPkg/OvmfPkg.dec + EmbeddedPkg/EmbeddedPkg.dec + +[LibraryClasses] + BaseLib + BaseMemoryLib + DebugLib + HobLib + IoLib + UefiBootServicesTableLib + +[Protocols] + gFdtClientProtocolGuid## CONSUMES + +[Depex] + gFdtClientProtocolGuid -- 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118295): https://edk2.groups.io/g/devel/message/118295 Mute This Topic: https://groups.io/mt/105728771/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version
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 | 228 ++ .../QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf | 49 2 files changed, 277 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 00..fa6f531e3a --- /dev/null +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c @@ -0,0 +1,228 @@ +/** @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 "QemuFwCfgLibMmioInternal.h" + +// +// These correspond to the implementation we detect at runtime. +// +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 + ) +{ + 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; + UINT64FwCfgSelectorAddress; + UINT64FwCfgSelectorSize; + UINT64FwCfgDataAddress; + UINT64FwCfgDataSize; + UINT64FwCfgDmaAddress; + UINT64FwCfgDmaSize; + 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; + + 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", ); +if ((Type) && +(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", ); + 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 +
[edk2-devel] [PATCH v3 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
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 Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Leif Lindholm Cc: Sami Mujawar Cc: Sunil V L Cc: Andrei Warkentin Signed-off-by: Chao Li --- .../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 #include +#include +#include + #include #include #include +#include #include #include #include @@ -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 ( +, +(VOID *), +sizeof (QEMU_FW_CFG_RESOURCE) +); +} + +/** + Get firmware configure resource in HOB. + + @param VOID + + @retval FwCfgResourceThe firmware configure resouce in HOB. + 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 (); + 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))); + MmioWrite64 (QemuGetFwCfgDmaAddress (), SwapBytes64 ((UINT64))); #else - MmioWrite32 ((UINT32)(mFwCfgDmaAddress + 4), SwapBytes32 ((UINT32))); + MmioWrite32 ((UINT32)(QemuGetFwCfgDmaAddress () + 4), SwapBytes32 ((UINT32))); #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
[edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files
Separate QemuFwCfgLibMmio.c into two files named QemuFwCfgLibMmio.c and QemuFwCfgLibMmioDxe.c, added a new header named QemuFwCfgLibMmioInternal.h for MMIO version. Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc"). BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755 Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Leif Lindholm Cc: Sami Mujawar Cc: Sunil V L Cc: Andrei Warkentin Signed-off-by: Chao Li --- .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c | 194 +- .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf | 4 +- .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h | 179 .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c | 153 ++ 4 files changed, 340 insertions(+), 190 deletions(-) create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c index 115a210759..dc949c8e26 100644 --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c @@ -1,10 +1,9 @@ /** @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 **/ @@ -20,63 +19,7 @@ #include -STATIC UINTN mFwCfgSelectorAddress; -STATIC UINTN mFwCfgDataAddress; -STATIC UINTN mFwCfgDmaAddress; - -/** - Reads firmware configuration bytes into a buffer - - @param[in] SizeSize in bytes to read - @param[in] Buffer Buffer to store data into (OPTIONAL if Size is 0) - -**/ -typedef -VOID(EFIAPI READ_BYTES_FUNCTION)( - IN UINTN Size, - IN VOID *Buffer OPTIONAL - ); - -/** - Writes bytes from a buffer to firmware configuration - - @param[in] SizeSize in bytes to write - @param[in] Buffer Buffer to transfer data from (OPTIONAL if Size is 0) - -**/ -typedef -VOID(EFIAPI WRITE_BYTES_FUNCTION)( - IN UINTN Size, - IN VOID *Buffer OPTIONAL - ); - -/** - Skips bytes in firmware configuration - - @param[in] Size Size in bytes to skip - -**/ -typedef -VOID(EFIAPI SKIP_BYTES_FUNCTION)( - IN UINTN Size - ); - -// -// Forward declaration of the two implementations we have. -// -STATIC READ_BYTES_FUNCTION MmioReadBytes; -STATIC WRITE_BYTES_FUNCTION MmioWriteBytes; -STATIC SKIP_BYTES_FUNCTION MmioSkipBytes; -STATIC READ_BYTES_FUNCTION DmaReadBytes; -STATIC WRITE_BYTES_FUNCTION DmaWriteBytes; -STATIC SKIP_BYTES_FUNCTION DmaSkipBytes; - -// -// These correspond to the implementation we detect at runtime. -// -STATIC READ_BYTES_FUNCTION *InternalQemuFwCfgReadBytes = MmioReadBytes; -STATIC WRITE_BYTES_FUNCTION *InternalQemuFwCfgWriteBytes = MmioWriteBytes; -STATIC SKIP_BYTES_FUNCTION *InternalQemuFwCfgSkipBytes = MmioSkipBytes; +#include "QemuFwCfgLibMmioInternal.h" /** Returns a boolean indicating if the firmware configuration interface @@ -97,126 +40,6 @@ QemuFwCfgIsAvailable ( return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0); } -RETURN_STATUS -EFIAPI -QemuFwCfgInitialize ( - VOID - ) -{ - EFI_STATUS Status; - FDT_CLIENT_PROTOCOL *FdtClient; - CONST UINT64 *Reg; - UINT32 RegSize; - UINTNAddressCells, SizeCells; - UINT64 FwCfgSelectorAddress; - UINT64 FwCfgSelectorSize; - UINT64 FwCfgDataAddress; - UINT64 FwCfgDataSize; - UINT64 FwCfgDmaAddress; - UINT64 FwCfgDmaSize; - - Status = gBS->LocateProtocol ( - , - NULL, - (VOID **) - ); - ASSERT_EFI_ERROR (Status); - - Status = FdtClient->FindCompatibleNodeReg ( -FdtClient, -"qemu,fw-cfg-mmio", -(CONST VOID **), -, -, - -); - if (EFI_ERROR (Status)) { -DEBUG (( - DEBUG_WARN, - "%a: No 'qemu,fw-cfg-mmio' compatible DT node found (Status == %r)\n", - __func__, - Status - )); -return EFI_SUCCESS; - } - - ASSERT (AddressCells == 2); - ASSERT (SizeCells == 2); - ASSERT (RegSize == 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
[edk2-devel] [PATCH v3 0/7] Adjust the QemuFwCfgLibMmio and add PEI stage
Patch1: Added three PCDs for QemuFwCfgLibMmio Patch2: Sparate QemuFwCfgLibMmio.c into two files and default as DXE stage library. Patch3: Added QemuFwCfgMmiLib PEI version Patch4: Rename QemuFwCfgLibMmio.inf to QemuFwCfgMmioDxeLib.inf and enable it in AARCH64 and RISCV64. V1 -> V2: 1. Use HOBs instead of PCD. 2. The old patch2 is divided into two parts, one is code splitting, and the other is functional changes. 3. add two patches to keep the safe when change the platform DSC file. V2 -> V3: 1. Merge three HOBs into a single HOB. 2. Remove the dynamic global variables in PEI. BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755 PR: https://github.com/tianocore/edk2/pull/5568 Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Leif Lindholm Cc: Sami Mujawar Cc: Sunil V L Cc: Andrei Warkentin Chao Li (7): OvmfPkg: Separate QemuFwCfgLibMmio.c into two files OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version OvmfPkg: Copy the same new INF as QemuFwCfgLibMmio.inf ArmVirtPkg: Enable QemuFwCfgMmioDxeLib.inf OvmfPkg/RiscVVirt: Enable QemuFwCfgMmioDxeLib.inf OvmfPkg: Remove QemuFwCfgLibMmio.inf ArmVirtPkg/ArmVirtQemu.dsc| 2 +- ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +- .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c | 249 + .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h | 253 ++ .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c | 222 +++ ...CfgLibMmio.inf => QemuFwCfgMmioDxeLib.inf} | 5 +- .../Library/QemuFwCfgLib/QemuFwCfgMmioPei.c | 228 .../QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf | 49 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc | 2 +- 9 files changed, 822 insertions(+), 190 deletions(-) create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c rename OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLibMmio.inf => QemuFwCfgMmioDxeLib.inf} (81%) create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf -- 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118291): https://edk2.groups.io/g/devel/message/118291 Mute This Topic: https://groups.io/mt/105728764/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH edk2-platforms WIP 2/3] SbsaQemu: describe PCIe buses in SSDT tables
We can have more than one PCI Express bus. So instead of having static description in DSDT we create SSDT table for each existing PCIe bus. Signed-off-by: Marcin Juszkiewicz --- Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 2 + .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 37 +- .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h | 23 + .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 87 ++- .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.c | 576 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl | 302 -- .../Drivers/SbsaQemuAcpiDxe/SsdtTemplate.asl| 82 +++ 7 files changed, 790 insertions(+), 319 deletions(-) diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc index e246db8b0a23..b012eaa34147 100644 --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc @@ -173,6 +173,8 @@ [LibraryClasses.common] ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf AcpiLib|EmbeddedPkg/Library/AcpiLib/AcpiLib.inf + AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf + AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf index 727c8e82d16e..6de1073e6ac2 100644 --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf @@ -18,18 +18,25 @@ [Defines] [Sources] SbsaQemuAcpiDxe.c + SbsaQemuAcpiDxe.h + SbsaQemuAcpiPcie.c + SbsaQemuAcpiPcie.h + SsdtTemplate.asl [Packages] ArmPkg/ArmPkg.dec ArmPlatformPkg/ArmPlatformPkg.dec ArmVirtPkg/ArmVirtPkg.dec + DynamicTablesPkg/DynamicTablesPkg.dec EmbeddedPkg/EmbeddedPkg.dec MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec Silicon/Qemu/SbsaQemu/SbsaQemu.dec [LibraryClasses] + AcpiHelperLib AcpiLib + AmlLib ArmLib BaseMemoryLib BaseLib @@ -37,6 +44,7 @@ [LibraryClasses] DxeServicesLib HardwareInfoLib PcdLib + PciLib PrintLib UefiDriverEntryPoint UefiLib @@ -54,14 +62,39 @@ [Pcd] gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase [Depex] - gEfiAcpiTableProtocolGuid ## CONSUMES + # We want all PCIe buses to be scanned first + gEfiPciIoProtocolGuid ## CONSUMES [Guids] gEdkiiPlatformHasAcpiGuid [Protocols] gEfiAcpiSdtProtocolGuid - gEfiAcpiTableProtocolGuid ## CONSUMES + gEfiAcpiTableProtocolGuid + gEfiPciRootBridgeIoProtocolGuid + + +[Pcd] + gArmTokenSpaceGuid.PcdPciBusMin + gArmTokenSpaceGuid.PcdPciBusMax + gArmTokenSpaceGuid.PcdPciIoBase + gArmTokenSpaceGuid.PcdPciIoSize + gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslation + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciIoLimit + + gArmTokenSpaceGuid.PcdPciMmio32Base + gArmTokenSpaceGuid.PcdPciMmio32Size + gEfiMdePkgTokenSpaceGuid.PcdPciMmio32Translation + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciMmio32Limit + + gArmTokenSpaceGuid.PcdPciMmio64Base + gArmTokenSpaceGuid.PcdPciMmio64Size + gEfiMdePkgTokenSpaceGuid.PcdPciMmio64Translation + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciMmio64Limit + + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciExpressBarSize + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciExpressBarLimit [FixedPcd] gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h new file mode 100644 index ..56cc6f1381da --- /dev/null +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h @@ -0,0 +1,23 @@ +/** @file +* This file is an ACPI driver for the Qemu SBSA platform. +* +* Copyright (c) 2024, Linaro Ltd. All rights reserved. +* +* SPDX-License-Identifier: BSD-2-Clause-Patent +* +**/ + +#ifndef SBSAQEMU_ACPI_PCIE_H +#define SBSAQEMU_ACPI_PCIE_H + +#pragma pack(1) + +/* AML bytecode generated from SsdtTemplate.asl */ +extern CHAR8 ssdttemplate_aml_code[]; + +EFI_STATUS +AddPcieHostBridges ( + AML_OBJECT_NODE_HANDLE ScopeNode + ); + +#endif diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c index 30239e7dca0d..f3d5dc9e9ba7 100644 --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c @@ -6,28 +6,27 @@ * SPDX-License-Identifier: BSD-2-Clause-Patent * **/ -#include -#include #include #include #include + #include +#include #include #include #include -#include -#include -#include #include
[edk2-devel] [PATCH edk2-platforms WIP 3/3] SbsaQemu: generate MCFG table
We want to have dynaminc PCI Express variables. Which forces us to generate MCFG from C code. Signed-off-by: Marcin Juszkiewicz --- Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 1 - .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c| 83 Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc | 43 -- 3 files changed, 83 insertions(+), 44 deletions(-) diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf index 8d4905362edc..37abf2f4c512 100644 --- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf +++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf @@ -19,7 +19,6 @@ [Sources] Dbg2.aslc Dsdt.asl Fadt.aslc - Mcfg.aslc Spcr.aslc [Packages] diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c index f3d5dc9e9ba7..6c7913eead81 100644 --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c @@ -7,6 +7,7 @@ * **/ #include +#include #include #include @@ -883,6 +884,83 @@ AddSsdtPcieTable ( return EFI_SUCCESS; } +/** Adds the MCFG ACPI table. + + @param AcpiTableThe ACPI Table. + @param PcieCfgData PCIe configuration data. + @param NumPcieSegments Number of PCIe segments. + + @return EFI_SUCCESS on success, or an error code. + +**/ +STATIC +EFI_STATUS +AddMcfgTable ( + IN EFI_ACPI_TABLE_PROTOCOL *AcpiTable + ) +{ + EFI_STATUSStatus; + UINTN TableHandle; + UINT32TableSize; + EFI_PHYSICAL_ADDRESS PageAddress; + UINT8 *New; + + EFI_ACPI_DESCRIPTION_HEADER Header = +SBSAQEMU_ACPI_HEADER ( + EFI_ACPI_6_3_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE, + EFI_ACPI_DESCRIPTION_HEADER, + EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION + ); + + TableSize = sizeof (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER) + + sizeof (EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE); + + Status = gBS->AllocatePages ( + AllocateAnyPages, + EfiACPIReclaimMemory, + EFI_SIZE_TO_PAGES (TableSize), + + ); + if (EFI_ERROR (Status)) { +DEBUG ((DEBUG_ERROR, "Failed to allocate pages for MCFG table\n")); +return EFI_OUT_OF_RESOURCES; + } + + New = (UINT8 *)(UINTN)PageAddress; + ZeroMem (New, TableSize); + + // Add the ACPI Description table header + CopyMem (New, , sizeof (EFI_ACPI_DESCRIPTION_HEADER)); + ((EFI_ACPI_DESCRIPTION_HEADER *)New)->Length = TableSize; + New += sizeof (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER); + + EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *CfgPtr; + + CfgPtr = (VOID *)New; + + CfgPtr->BaseAddress = PcdGet64 (PcdPciExpressBaseAddress); + CfgPtr->PciSegmentGroupNumber = 0; + CfgPtr->StartBusNumber= PcdGet32 (PcdPciBusMin); + CfgPtr->EndBusNumber = PcdGet32 (PcdPciBusMax); + + New += sizeof (EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE); + + // Perform Checksum + AcpiTableChecksum ((UINT8 *)PageAddress, TableSize); + + Status = AcpiTable->InstallAcpiTable ( +AcpiTable, +(EFI_ACPI_COMMON_HEADER *)PageAddress, +TableSize, + +); + if (EFI_ERROR (Status)) { +DEBUG ((DEBUG_ERROR, "Failed to install MCFG table\n")); + } + + return Status; +} + EFI_STATUS EFIAPI @@ -951,6 +1029,11 @@ InitializeSbsaQemuAcpiDxe ( DEBUG ((DEBUG_ERROR, "Failed to add SSDT table\n")); } + Status = AddMcfgTable (AcpiTable); + if (EFI_ERROR (Status)) { +DEBUG ((DEBUG_ERROR, "Failed to add MCFG table\n")); + } + return EFI_SUCCESS; } diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc b/Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc deleted file mode 100644 index 289f4ad4ea3a.. --- a/Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc +++ /dev/null @@ -1,43 +0,0 @@ -/** @file -* ACPI Memory mapped configuration space base address Description Table (MCFG). -* -* Copyright (c) 2020, Linaro Limited. All rights reserved. -* -* SPDX-License-Identifier: BSD-2-Clause-Patent -**/ - -#include -#include -#include - -#pragma pack(push, 1) - -typedef struct { - EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER Header; - EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE Structure[1]; -} EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_DESCRIPTION_TABLE; - -EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_DESCRIPTION_TABLE
[edk2-devel] [PATCH WIP edk2-platforms 0/3] SbsaQemu: add support for multiple PCI Express buses
QEMU allows to have NUMA setup where each node has own cpus, memory and i/o. We already handle cpus and memory. This patchset adds support for having multiple PCI Express buses. SbsaQemu assumed that there is only bus 0. First patch does PCIe bus scan to find all host bridges (bus 0 one and additional 'pxb-pcie' ones). Second patch moves description of PCIe from DSDT to SSDT (one per each PCIe bus). So Operating System will know about all of them. Third patch moves generation of MCFG table to C. It is preparation to move PCIe Pcds from being fixed to dynamic ones. There are some booting issues with assigning resources for cards: pci :00:03.0: BAR 15: no space for [mem size 0x0020 64bit pref] pci :00:03.0: BAR 15: failed to assign [mem size 0x0020 64bit pref] pci :00:01.0: BAR 6: no space for [mem size 0x0004 pref] pci :00:01.0: BAR 6: failed to assign [mem size 0x0004 pref] pci :00:03.0: BAR 13: no space for [io size 0x1000] pci :00:03.0: BAR 13: failed to assign [io size 0x1000] Boot log (Linux + lspci + ACPI tables dump): https://people.linaro.org/~marcin.juszkiewicz/sbsa-ref/boot-linux-with-numa-multiple-pcie-buses.txt I am wondering where I made mistakes in handling PCIe buses. Thanks go to Leif for pointing me to use of Aml to generate SSDT tables. Cc: Leif Lindholm Cc: Ard Biesheuvel Cc: Graeme Gregory Cc: Ray Ni To: devel@edk2.groups.io Signed-off-by: Marcin Juszkiewicz --- Marcin Juszkiewicz (3): SbsaQemu: scan for PCIe buses SbsaQemu: describe PCIe buses in SSDT tables SbsaQemu: generate MCFG table Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 2 + Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 1 - .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 37 +- .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h | 23 + .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 170 +- .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.c | 576 .../SbsaQemuPciHostBridgeLib.c | 185 --- Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl | 302 -- Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc | 43 -- .../Drivers/SbsaQemuAcpiDxe/SsdtTemplate.asl| 82 +++ 10 files changed, 982 insertions(+), 439 deletions(-) --- base-commit: 73cfdc4afff3e641be217b31b985761ef8338412 change-id: 20240425-review-multiple-pcie-0425-54ce3817fd3f Best regards, -- Marcin Juszkiewicz -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118288): https://edk2.groups.io/g/devel/message/118288 Mute This Topic: https://groups.io/mt/105728624/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH edk2-platforms WIP 1/3] SbsaQemu: scan for PCIe buses
SbsaQemu assumes that there is only one PCI Express bus. But there can be multiple PCIe buses as NUMA systems can get 'pxb-pcie' HostBridge devices added. Let scan for all PCIe buses and report them back so EDK2 will be able to find all expansions. Signed-off-by: Marcin Juszkiewicz --- .../SbsaQemuPciHostBridgeLib.c | 185 1 file changed, 109 insertions(+), 76 deletions(-) diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c index 9739c7500def..1c4ed1c74e52 100644 --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c @@ -6,10 +6,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ +#include +#include +#include #include #include #include #include +#include #include #include @@ -52,76 +56,49 @@ CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = { L"Mem", L"I/O", L"Bus" }; -STATIC PCI_ROOT_BRIDGE mRootBridge = { - /* UINT32 Segment; Segment number */ - 0, - - /* UINT64 Supports; Supported attributes */ - EFI_PCI_ATTRIBUTE_ISA_IO_16 | EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | - EFI_PCI_ATTRIBUTE_VGA_IO_16 | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16, - - /* UINT64 Attributes; Initial attributes */ - EFI_PCI_ATTRIBUTE_ISA_IO_16 | EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | - EFI_PCI_ATTRIBUTE_VGA_IO_16 | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16, - - /* BOOLEAN DmaAbove4G; DMA above 4GB memory */ - TRUE, - - /* BOOLEAN NoExtendedConfigSpace; When FALSE, the root bridge supports - Extended (4096-byte) Configuration Space. When TRUE, the root bridge - supports 256-byte Configuration Space only. */ - FALSE, - - /* BOOLEAN ResourceAssigned; Resource assignment status of the root bridge. - Set to TRUE if Bus/IO/MMIO resources for root bridge have been assigned */ - FALSE, - - /* UINT64 AllocationAttributes; Allocation attributes. */ - EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM | - EFI_PCI_HOST_BRIDGE_MEM64_DECODE, /* as Mmio64Size > 0 */ - - { - /* PCI_ROOT_BRIDGE_APERTURE Bus; Bus aperture which can be used by the - * root bridge. */ - FixedPcdGet32 (PcdPciBusMin), - FixedPcdGet32 (PcdPciBusMax) - }, - - /* PCI_ROOT_BRIDGE_APERTURE Io; IO aperture which can be used by the root - bridge */ - { - FixedPcdGet64 (PcdPciIoBase), - FixedPcdGet64 (PcdPciIoBase) + FixedPcdGet64 (PcdPciIoSize) - 1 - }, - - /* PCI_ROOT_BRIDGE_APERTURE Mem; MMIO aperture below 4GB which can be used by - the root bridge - (gEfiMdePkgTokenSpaceGuid.PcdPciMmio32Translation as 0x0) */ - { -FixedPcdGet32 (PcdPciMmio32Base), -FixedPcdGet32 (PcdPciMmio32Base) + FixedPcdGet32 (PcdPciMmio32Size) - 1, - }, - - /* PCI_ROOT_BRIDGE_APERTURE MemAbove4G; MMIO aperture above 4GB which can be - used by the root bridge. - (gEfiMdePkgTokenSpaceGuid.PcdPciMmio64Translation as 0x0) */ - { -FixedPcdGet64 (PcdPciMmio64Base), -FixedPcdGet64 (PcdPciMmio64Base) + FixedPcdGet64 (PcdPciMmio64Size) - 1 - }, - - /* PCI_ROOT_BRIDGE_APERTURE PMem; Prefetchable MMIO aperture below 4GB which - can be used by the root bridge. - In our case, there are no separate ranges for prefetchable and - non-prefetchable BARs */ - { MAX_UINT64, 0 }, - - /* PCI_ROOT_BRIDGE_APERTURE PMemAbove4G; Prefetchable MMIO aperture above 4GB - which can be used by the root bridge. */ - { MAX_UINT64, 0 }, - /* EFI_DEVICE_PATH_PROTOCOL *DevicePath; Device path. */ - (EFI_DEVICE_PATH_PROTOCOL *), -}; +EFI_STATUS +EFIAPI +PciHostBridgeUtilityInitRootBridge ( + IN UINTN RootBusNumber, + OUT PCI_ROOT_BRIDGE *RootBus + ) +{ + EFI_PCI_ROOT_BRIDGE_DEVICE_PATH *DevicePath; + UINTNMaxSubBusNumber = 255; + + DevicePath = AllocateCopyPool ( + sizeof mEfiPciRootBridgeDevicePath, + + ); + if (DevicePath == NULL) { +DEBUG ((DEBUG_ERROR, "%a: %r\n", __func__, EFI_OUT_OF_RESOURCES)); +return EFI_OUT_OF_RESOURCES; + } + + DevicePath->AcpiDevicePath.UID = RootBusNumber; + + RootBus->Segment = 0; + RootBus->Supports = 0; + RootBus->Attributes= 0; + RootBus->DmaAbove4G= TRUE; + RootBus->AllocationAttributes = EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM | EFI_PCI_HOST_BRIDGE_MEM64_DECODE, /* as Mmio64Size > 0 */ + RootBus->Bus.Base = RootBusNumber; + RootBus->Bus.Limit = MaxSubBusNumber; + RootBus->Io.Base = PcdGet64 (PcdPciIoBase); + RootBus->Io.Limit = PcdGet64 (PcdPciIoBase) + PcdGet64 (PcdPciIoSize) - 1; + RootBus->Mem.Base = PcdGet32 (PcdPciMmio32Base); + RootBus->Mem.Limit = PcdGet32 (PcdPciMmio32Base) + PcdGet32 (PcdPciMmio32Size) - 1; +
Re: [edk2-devel] [PATCH] OvmfPkg: Set PcdCpuMaxLogicalProcessorNumber in OvmfXen
Hi, On 25/04/2024 08:31, Gerd Hoffmann wrote: > On Wed, Apr 24, 2024 at 02:36:32PM +0100, Alejandro Vallejo wrote: >> Bump the compile-time constant for maximum processor count from 64 to 128 >> in order to allow that many vCPUs to be brought online on Xen guests with >> the default OVMF configuration. > >> + # UefiCpuPkg PCDs related to initial AP bringup and general AP management. >> + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|128 > > Note that this is a dynamic PCD, so you can set it at runtime to the > number of vcpus present in the VM. See MaxCpuCountInitialization() in > OvmfPkg/PlatformPei/Platform.c for example. > > take care, > Gerd > Thanks for the heads up. Do you mean setting it at runtime through fw_cfg? I saw PlatformMaxCpuCountInitialization() providing some customizability, but Xen's toolstack doesn't provide fw_cfg at the moment so it can't (as far as I've seen) use it. I'm currently forced to do the override at compile time passing this... --pcd gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber=128 ... to build.sh, but I'd rather have the default max match Xen's idea of max unless there's a strong reason not to. Cheers, Alejandro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118286): https://edk2.groups.io/g/devel/message/118286 Mute This Topic: https://groups.io/mt/105721898/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of HCRST
Hi @gaoliming, Could you provide the eta when the patch can be merged? Thanks, Xianglei -Original Message- From: Cai, Xianglei Sent: Thursday, April 18, 2024 5:00 PM To: gaoliming ; devel@edk2.groups.io Cc: Ni, Ray ; Lewandowski, Krzysztof ; Huang, Jenny ; Shih, More ; Chiu, Ian Subject: RE: [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of HCRST Hi Liming, Could you help pick the change to master branch? Thanks, Xianglei -Original Message- From: gaoliming Sent: Monday, April 15, 2024 2:36 PM To: Cai, Xianglei ; devel@edk2.groups.io Cc: Ni, Ray ; Lewandowski, Krzysztof ; Huang, Jenny ; Shih, More ; Chiu, Ian Subject: 回复: [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of HCRST Reviewed-by: Liming Gao > -邮件原件- > 发件人: Xianglei Cai > 发送时间: 2024年4月15日 14:34 > 收件人: devel@edk2.groups.io > 抄送: Xianglei Cai ; Ray Ni ; > Liming Gao ; Krzysztof Lewandowski > ; Jenny Huang > ; More Shih ; Ian Chiu > > 主题: [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of > HCRST > > https://bugzilla.tianocore.org/show_bug.cgi?id=4727 > > Recently some of XHCI host controllers require to have extra 1ms delay > before accessing any MMIO register during reset. PHY transition from > P3 to P0 can take around 1.3ms and the xHCI reset can take around > 1.5ms. > Add PCD to control the delay, the default is 2 ms. > > Cc: Ray Ni > Cc: Liming Gao > Cc: Krzysztof Lewandowski > Cc: Jenny Huang > Cc: More Shih > Cc: Ian Chiu > Signed-off-by: Xianglei Cai > Reviewed-by: Krzysztof Lewandowski > --- > MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h | 1 + > MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf | 4 > MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 2 +- > MdeModulePkg/MdeModulePkg.dec| 5 + > 4 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h > index 44016758724c..c9a12095c29e 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h > @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include #include > #include > +#include > > #include > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf > index 18ef87916ae4..e6c1ac8a6346 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf > @@ -56,6 +56,7 @@ >DebugLib >ReportStatusCodeLib >TimerLib > + PcdLib > > [Guids] >gEfiEventExitBootServicesGuid ## > SOMETIMES_CONSUMES ## Event > @@ -64,6 +65,9 @@ >gEfiPciIoProtocolGuid ## TO_START >gEfiUsb2HcProtocolGuid## BY_START > > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdDelayXhciHCReset ## > CONSUMES > + > # [Event] > # EVENT_TYPE_PERIODIC_TIMER ## CONSUMES > # > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > index 40f2f1f22766..525942a167b0 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > @@ -864,7 +864,7 @@ XhcResetHC ( > // Otherwise there may have the timeout case happened. > // The below is a workaround to solve such problem. > // > -gBS->Stall (XHC_1_MILLISECOND); > +gBS->Stall (PcdGet16 (PcdDelayXhciHCReset)); > Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, > XHC_USBCMD_RESET, FALSE, Timeout); > > if (!EFI_ERROR (Status)) { > diff --git a/MdeModulePkg/MdeModulePkg.dec > b/MdeModulePkg/MdeModulePkg.dec index a91058e5b5df..d9e2e724df9e > 100644 > --- a/MdeModulePkg/MdeModulePkg.dec > +++ b/MdeModulePkg/MdeModulePkg.dec > @@ -1148,6 +1148,11 @@ ># @Prompt Enable large address image loading. > > gEfiMdeModulePkgTokenSpaceGuid.PcdImageLargeAddressLoad|TRUE|BOO > LEAN|0x30001059 > > + ## Indicates time delay for XHCI registers access after it issues HCRST. > + # Default is 2000, it represent delay is 2 ms. > + # @Prompt Delay access XHCI register after it issues HCRST (us) > + > gEfiMdeModulePkgTokenSpaceGuid.PcdDelayXhciHCReset|2000|UINT16|0x3 > 0001060 > + > [PcdsFixedAtBuild, PcdsPatchableInModule] >## Dynamic type PCD can be registered callback function for Pcd > setting action. ># PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum > number of callback function > -- > 2.42.0.windows.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118285): https://edk2.groups.io/g/devel/message/118285 Mute This Topic: https://groups.io/mt/105594498/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint while USB Transaction error
Hi @gaoliming Could you provide the eta when the patch can be merged? Thanks, Xianglei -Original Message- From: Cai, Xianglei Sent: Thursday, April 18, 2024 4:59 PM To: gaoliming ; devel@edk2.groups.io Cc: Ni, Ray ; Lewandowski, Krzysztof ; Huang, Jenny ; Shih, More Subject: RE: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint while USB Transaction error Hi Liming, Could you help pick the change to the master branch? Thanks, Xianglei -Original Message- From: gaoliming Sent: Monday, April 15, 2024 5:52 PM To: devel@edk2.groups.io; Cai, Xianglei Cc: Ni, Ray ; Lewandowski, Krzysztof ; Huang, Jenny ; Shih, More Subject: 回复: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint while USB Transaction error Reviewed-by: Liming Gao > -邮件原件- > 发件人: devel@edk2.groups.io 代表 Xianglei Cai > 发送时间: 2024年4月15日 14:55 > 收件人: devel@edk2.groups.io > 抄送: Xianglei Cai ; Ray Ni ; > Liming Gao ; Krzysztof Lewandowski > ; Jenny Huang > ; More Shih > 主题: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint > while USB Transaction error > > https://bugzilla.tianocore.org/show_bug.cgi?id=4556 > > Based on XHCI spec 4.8.3, software should do the reset endpoint while > USB Transaction occur. > Add the error code for USB Transaction error since UEFI spec don't > have the related definition. > > Cc: Ray Ni > Cc: Liming Gao > Cc: Krzysztof Lewandowski > Cc: Jenny Huang > Cc: More Shih > Signed-off-by: Xianglei Cai > Reviewed-by: Krzysztof Lewandowski > --- > MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 5 - > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 5 - > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 7 +++ > 3 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c > index f4e61d223c1b..cf6b32959e68 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c > @@ -825,7 +825,10 @@ XhcTransfer ( >*TransferResult = Urb->Result; >*DataLength = Urb->Completed; > > - if ((*TransferResult == EFI_USB_ERR_STALL) || (*TransferResult == > EFI_USB_ERR_BABBLE)) { > + // > + // Based on XHCI spec 4.8.3, software should do the reset endpoint while > USB Transaction occur. > + // > + if ((*TransferResult == EFI_USB_ERR_STALL) || (*TransferResult == > EFI_USB_ERR_BABBLE) || (*TransferResult == > EDKII_USB_ERR_TRANSACTION)) { > ASSERT (Status == EFI_DEVICE_ERROR); > RecoveryStatus = XhcRecoverHaltedEndpoint (Xhc, Urb); > if (EFI_ERROR (RecoveryStatus)) { diff --git > a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > index 5d735008ba31..a97ed44dbfc3 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > @@ -1192,8 +1192,11 @@ XhcCheckUrbResult ( > DEBUG ((DEBUG_ERROR, "XhcCheckUrbResult: ERR_BUFFER! > Completecode = %x\n", EvtTrb->Completecode)); > goto EXIT; > > + // > + // Based on XHCI spec 4.8.3, software should do the reset > + endpoint > while USB Transaction occur. > + // >case TRB_COMPLETION_USB_TRANSACTION_ERROR: > -CheckedUrb->Result |= EFI_USB_ERR_TIMEOUT; > +CheckedUrb->Result |= EDKII_USB_ERR_TRANSACTION; > CheckedUrb->Finished = TRUE; > DEBUG ((DEBUG_ERROR, "XhcCheckUrbResult: > TRANSACTION_ERROR! Completecode = %x\n", EvtTrb->Completecode)); > goto EXIT; > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h > index 7c85f7993b5c..e606e212a1d3 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h > @@ -78,6 +78,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #define TRB_COMPLETION_STOPPED 26 > #define TRB_COMPLETION_STOPPED_LENGTH_INVALID 27 > > +// > +// USB Transfer Results Internal Definition // Based on XHCI spec > +4.8.3, software should do the reset endpoint while > USB Transaction occur. > +// Add the error code for USB Transaction error since UEFI spec don't have > the related definition. > +// > +#define EDKII_USB_ERR_TRANSACTION 0x200 > + > // > // The topology string used to present usb device location // > -- > 2.42.0.windows.2 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118284): https://edk2.groups.io/g/devel/message/118284 Mute This Topic: https://groups.io/mt/105594486/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version
Hi Gerd, I get it, I will refactor the code as soon as I can, it looks like there's still some work and will take some time. I will try to send the V3 tonight if possible. Thanks, Chao On 2024/4/25 17:02, Gerd Hoffmann wrote: On Thu, Apr 25, 2024 at 04:06:13PM +0800, Chao Li wrote: Hi Gerd, Thanks, Chao On 2024/4/25 15:53, Gerd Hoffmann wrote: Hi, +UINTN mFwCfgSelectorAddress; +UINTN mFwCfgDataAddress; +UINTN mFwCfgDmaAddress; Hmm, global variables for PEI? I think the point of storing these in the HOB is to avoid the need for global variables? Also does that work when running PEI in-place from flash? I think it would be useful if some platforms(not LoongArch) could use the global variables in PEI, because the global variables are faster. Performance isn't my main concern here, I very much prefer code which is easy to maintain. Taking the same code path on all platforms is good for that. It's less code and it also makes testing easier. The risk of breaking loongarch when changing something for riscv or arm is much lower if all platforms work the same way. I'd suggest to first refactor the existing DXE code to use a HOB instead of global variables. Have a helper function which looks up the HOB and returns a pointer to the configuration struct. That helper function can be slightly different for DXE/PEI, the DXE variant can cache the pointer to the struct in a global variable so it needs to do the lookup only once. + UINT64FwCfgDataSize; + UINT64FwCfgDmaAddress; + UINT64FwCfgDmaSize; First thing this function should do is check whenever the HOB already exists. Should that be the case there is no need to parse the device tree. This is a constructor in PEI, that has to parse the device tree and then build the HOBs. This is a library, so it can be linked into multiple PEI and DXE modules. So it must be prepared to run multiple times. On the second and all following runs the HOB will already exist. The DXE variant will need the check for sure. I'd strongly suggest to add it to the PEI variant too, even though it might not be needed right now because PlatformPei is the only PEI module using the library. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118283): https://edk2.groups.io/g/devel/message/118283 Mute This Topic: https://groups.io/mt/105724970/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version
On Thu, Apr 25, 2024 at 04:06:13PM +0800, Chao Li wrote: > Hi Gerd, > > > Thanks, > Chao > On 2024/4/25 15:53, Gerd Hoffmann wrote: > >Hi, > > > > > +UINTN mFwCfgSelectorAddress; > > > +UINTN mFwCfgDataAddress; > > > +UINTN mFwCfgDmaAddress; > > Hmm, global variables for PEI? I think the point of storing these in > > the HOB is to avoid the need for global variables? Also does that work > > when running PEI in-place from flash? > I think it would be useful if some platforms(not LoongArch) could use the > global variables in PEI, because the global variables are faster. Performance isn't my main concern here, I very much prefer code which is easy to maintain. Taking the same code path on all platforms is good for that. It's less code and it also makes testing easier. The risk of breaking loongarch when changing something for riscv or arm is much lower if all platforms work the same way. I'd suggest to first refactor the existing DXE code to use a HOB instead of global variables. Have a helper function which looks up the HOB and returns a pointer to the configuration struct. That helper function can be slightly different for DXE/PEI, the DXE variant can cache the pointer to the struct in a global variable so it needs to do the lookup only once. > > > + UINT64FwCfgDataSize; > > > + UINT64FwCfgDmaAddress; > > > + UINT64FwCfgDmaSize; > > First thing this function should do is check whenever the HOB already > > exists. Should that be the case there is no need to parse the device > > tree. > This is a constructor in PEI, that has to parse the device tree and then > build the HOBs. This is a library, so it can be linked into multiple PEI and DXE modules. So it must be prepared to run multiple times. On the second and all following runs the HOB will already exist. The DXE variant will need the check for sure. I'd strongly suggest to add it to the PEI variant too, even though it might not be needed right now because PlatformPei is the only PEI module using the library. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118282): https://edk2.groups.io/g/devel/message/118282 Mute This Topic: https://groups.io/mt/105724970/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
Hi Ard and Gerd, Thanks, Chao On 2024/4/25 16:11, Ard Biesheuvel wrote: On Thu, 25 Apr 2024 at 10:10, Chao Li wrote: Hi Gerd, Thanks, Chao On 2024/4/25 15:40, Gerd Hoffmann wrote: Hi, +EFI_GUID mFwCfgSelectorAddressGuid = FW_CONFIG_SELECTOR_ADDRESS_HOB_GUID; +EFI_GUID mFwCfgDataAddressGuid = FW_CONFIG_DATA_ADDRESS_HOB_GUID; +EFI_GUID mFwCfgDmaAddressGuid = FW_CONFIG_DMA_ADDRESS_HOB_GUID; Oh. I assumed that would be obvious (because it's common practice for HOBs), but I was thinking about a single HOB containing a struct with all three values instead of a separate HOB for each value. If uses a single HOB, it must define a structure in this library, and it may be more complicated to build and parse the HOB. Please use a single HOB, and avoid global variables in PEI (PEI modules may execute in place from read-only NOR flash, so global variables are not allowed in PEI) OK, I will adjust it now, and send the V3. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118281): https://edk2.groups.io/g/devel/message/118281 Mute This Topic: https://groups.io/mt/105724969/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
On Thu, 25 Apr 2024 at 10:10, Chao Li wrote: > > Hi Gerd, > > > Thanks, > Chao > On 2024/4/25 15:40, Gerd Hoffmann wrote: > > Hi, > > +EFI_GUID mFwCfgSelectorAddressGuid = FW_CONFIG_SELECTOR_ADDRESS_HOB_GUID; > +EFI_GUID mFwCfgDataAddressGuid = FW_CONFIG_DATA_ADDRESS_HOB_GUID; > +EFI_GUID mFwCfgDmaAddressGuid = FW_CONFIG_DMA_ADDRESS_HOB_GUID; > > Oh. I assumed that would be obvious (because it's common practice for > HOBs), but I was thinking about a single HOB containing a struct with > all three values instead of a separate HOB for each value. > > If uses a single HOB, it must define a structure in this library, and it may > be more complicated to build and parse the HOB. > Please use a single HOB, and avoid global variables in PEI (PEI modules may execute in place from read-only NOR flash, so global variables are not allowed in PEI) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118280): https://edk2.groups.io/g/devel/message/118280 Mute This Topic: https://groups.io/mt/105724969/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
Hi Gerd, Thanks, Chao On 2024/4/25 15:40, Gerd Hoffmann wrote: Hi, +EFI_GUID mFwCfgSelectorAddressGuid = FW_CONFIG_SELECTOR_ADDRESS_HOB_GUID; +EFI_GUID mFwCfgDataAddressGuid = FW_CONFIG_DATA_ADDRESS_HOB_GUID; +EFI_GUID mFwCfgDmaAddressGuid = FW_CONFIG_DMA_ADDRESS_HOB_GUID; Oh. I assumed that would be obvious (because it's common practice for HOBs), but I was thinking about a single HOB containing a struct with all three values instead of a separate HOB for each value. If uses a single HOB, it must define a structure in this library, and it may be more complicated to build and parse the HOB. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118279): https://edk2.groups.io/g/devel/message/118279 Mute This Topic: https://groups.io/mt/105724969/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version
Hi Gerd, Thanks, Chao On 2024/4/25 15:53, Gerd Hoffmann wrote: Hi, +UINTN mFwCfgSelectorAddress; +UINTN mFwCfgDataAddress; +UINTN mFwCfgDmaAddress; Hmm, global variables for PEI? I think the point of storing these in the HOB is to avoid the need for global variables? Also does that work when running PEI in-place from flash? I think it would be useful if some platforms(not LoongArch) could use the global variables in PEI, because the global variables are faster. +RETURN_STATUS +EFIAPI +QemuFwCfgInitialize ( + VOID + ) +{ + VOID *DeviceTreeBase; + INT32 Node; + INT32 Prev; + CONST CHAR8 *Type; + INT32 Len; + CONST UINT64 *Reg; + UINT64FwCfgSelectorAddress; + UINT64FwCfgSelectorSize; + UINT64FwCfgDataAddress; + UINT64FwCfgDataSize; + UINT64FwCfgDmaAddress; + UINT64FwCfgDmaSize; First thing this function should do is check whenever the HOB already exists. Should that be the case there is no need to parse the device tree. This is a constructor in PEI, that has to parse the device tree and then build the HOBs. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118278): https://edk2.groups.io/g/devel/message/118278 Mute This Topic: https://groups.io/mt/105724970/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version
Hi, > +UINTN mFwCfgSelectorAddress; > +UINTN mFwCfgDataAddress; > +UINTN mFwCfgDmaAddress; Hmm, global variables for PEI? I think the point of storing these in the HOB is to avoid the need for global variables? Also does that work when running PEI in-place from flash? > +RETURN_STATUS > +EFIAPI > +QemuFwCfgInitialize ( > + VOID > + ) > +{ > + VOID *DeviceTreeBase; > + INT32 Node; > + INT32 Prev; > + CONST CHAR8 *Type; > + INT32 Len; > + CONST UINT64 *Reg; > + UINT64FwCfgSelectorAddress; > + UINT64FwCfgSelectorSize; > + UINT64FwCfgDataAddress; > + UINT64FwCfgDataSize; > + UINT64FwCfgDmaAddress; > + UINT64FwCfgDmaSize; First thing this function should do is check whenever the HOB already exists. Should that be the case there is no need to parse the device tree. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118277): https://edk2.groups.io/g/devel/message/118277 Mute This Topic: https://groups.io/mt/105724970/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 08/13] OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
> > Creating the EFI_SMM_SMRAM_MEMORY_GUID HOB should be moved to its > own > function. Also move over the comments from SmmAccess describing the > regions please. > > Adding a reference to the PI spec section describing this would be good > too. > Got it, I will refine the patch to V4 based on your comments. Thanks, Jiaxin -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118276): https://edk2.groups.io/g/devel/message/118276 Mute This Topic: https://groups.io/mt/105593577/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
Hi, > +EFI_GUID mFwCfgSelectorAddressGuid = FW_CONFIG_SELECTOR_ADDRESS_HOB_GUID; > +EFI_GUID mFwCfgDataAddressGuid = FW_CONFIG_DATA_ADDRESS_HOB_GUID; > +EFI_GUID mFwCfgDmaAddressGuid = FW_CONFIG_DMA_ADDRESS_HOB_GUID; Oh. I assumed that would be obvious (because it's common practice for HOBs), but I was thinking about a single HOB containing a struct with all three values instead of a separate HOB for each value. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118275): https://edk2.groups.io/g/devel/message/118275 Mute This Topic: https://groups.io/mt/105724969/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] OvmfPkg: Set PcdCpuMaxLogicalProcessorNumber in OvmfXen
On Wed, Apr 24, 2024 at 02:36:32PM +0100, Alejandro Vallejo wrote: > Bump the compile-time constant for maximum processor count from 64 to 128 > in order to allow that many vCPUs to be brought online on Xen guests with > the default OVMF configuration. > + # UefiCpuPkg PCDs related to initial AP bringup and general AP management. > + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|128 Note that this is a dynamic PCD, so you can set it at runtime to the number of vcpus present in the VM. See MaxCpuCountInitialization() in OvmfPkg/PlatformPei/Platform.c for example. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118274): https://edk2.groups.io/g/devel/message/118274 Mute This Topic: https://groups.io/mt/105721898/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 08/13] OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
Hi, > Let me explain more why need this change: > > 1. The EFI_SMM_SMRAM_MEMORY_GUID HOB, as defined in the PI specification, is > used to describe the SMRAM memory regions supported by the platform. This HOB > should be produced during the memory detection phase to align with the PI > spec. > > 2. In addition to the memory reserved for ACPI S3 resume, an increasing > number of features require reserving SMRAM for specific purposes, such as > SmmRelocation. Other advanced features in Intel platforms also necessitate > this. The implementation of these features varies and is entirely dependent > on the platform. This is why an increasing number of platforms are adopting > the EFI_SMM_SMRAM_MEMORY_GUID HOB for SMRAM description. > > 3. It is crucial that the SMRAM information remains consistent when retrieved > from the platform, whether through the SMM ACCESS PPI/Protocol or the > EFI_SMM_SMRAM_MEMORY_GUID HOB. Inconsistencies can lead to unexpected issues, > most commonly memory region conflicts. > > 4. The SMM ACCESS PPI/Protocol can be naturally implemented for general use. > The common approach is to utilize the EFI_SMM_SMRAM_MEMORY_GUID HOB. For > reference, see the existing implementation in the EDK2 repository at > edk2/UefiPayloadPkg/SmmAccessDxe/SmmAccessDxe.inf and > edk2-platforms/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.inf. > > > For the reasons mentioned, we are moving the SMRAM memory regions to HOBs and > allowing SMM access to consume these HOBs. > > I will add the above info into commit message. Thanks. Creating the EFI_SMM_SMRAM_MEMORY_GUID HOB should be moved to its own function. Also move over the comments from SmmAccess describing the regions please. Adding a reference to the PI spec section describing this would be good too. > > Storing anything SMM related outside SMRAM makes me nervous. > > I'd strongly suggest to avoid that. > > > > It might be that in this specific case it is not a problem. But it > > needs very careful review of the implications (which I have not done) > > and you have to hope you don't miss a possible attack vector, such as > > someone modifying the HOB and the firmware then storing SMM data + code > > outside SMRAM. > > Understand, but here is the case we can record the info in non-smram > since PI spec exposes that, there is no difference the info retrieved > from PPI/ non-smm Protocol or the non-smram. Good point. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118273): https://edk2.groups.io/g/devel/message/118273 Mute This Topic: https://groups.io/mt/105593577/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib
Hi, > That means the SMMRevId is 0_xx64h for AMD64 processor. But I am not > sure what the value is for AMD32 processor. Maybe 0 according to the > OVMF logic. The smm emulation in the linux kernel uses 0 and 0x64. > But, I am very suspicious about the logic in AMD's version as below: > --- AMD's version > SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_32BIT; > > LMAValue = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA; > if (LMAValue) { > SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_64BIT; > } > --- > The above logic detects the current CPU mode and 64bit save state area layout > is used if it's running in 64bit. > But if a AMD64 CPU runs in 32bit mode, the above logic causes the > 32bit save state area layout is used. It's not right! The save state > area layout does not depend on the CPU running mode, but whether it's > a legacy CPU or a 64-capable CPU. Well, that is not entirely clear to me. Could it be 64-bit processors support both 32-bit and 64-bit format, for backward compatibility reasons? So OvmfPkgIa32 builds could use the 32-bit format, OvmfPkgX64 builds use the 64-bit format, everything works fine? The tricky corner case is OvmfPkgIa32X64, where (after applying this series) 32-bit PEI should setup things for 64-bit SMM/DXE, and checking the current processor mode will not give use the result we need. > Jiaxin, I agree that the confusion should be cleaned up by AMD > experts. Let's not change any existing behavior. Agree. Tom? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118272): https://edk2.groups.io/g/devel/message/118272 Mute This Topic: https://groups.io/mt/105593568/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] Add SHA3/SM3 functions with openssl for Mbedtls
Looks good to me. Reviewed-by: Yi Li -Original Message- From: Hou, Wenxing Sent: Wednesday, April 24, 2024 4:25 PM To: devel@edk2.groups.io Cc: Yao, Jiewen ; Li, Yi1 Subject: [PATCH v2] Add SHA3/SM3 functions with openssl for Mbedtls REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177 Because the Mbedlts 3.3.0 doesn't have SHA3 and Sm3, the SHA3 and Sm3 implementaion based on Openssl. And the implementaion has passed build check. Cc: Jiewen Yao Cc: Yi Li Signed-off-by: Wenxing Hou --- CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf | 9 +++-- CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf| 9 +++-- .../Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf | 3 ++- CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf| 9 +++-- .../Library/BaseCryptLibMbedTls/TestBaseCryptLib.inf | 4 +++- CryptoPkg/Library/MbedTlsLib/MbedTlsLib.inf | 6 ++ CryptoPkg/Library/MbedTlsLib/MbedTlsLibFull.inf | 6 ++ 7 files changed, 38 insertions(+), 8 deletions(-) diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf index 16def792c5..999054500f 100644 --- a/CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf +++ b/CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf @@ -18,6 +18,7 @@ MODULE_TYPE= DXE_DRIVER VERSION_STRING = 1.0 LIBRARY_CLASS = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER + DEFINE BASE_CRYPT_PATH = ../BaseCryptLib # # The following information is for reference only and not required by the build tools. @@ -31,10 +32,14 @@ Cipher/CryptAes.c Hash/CryptSha256.c Hash/CryptSha512.c - Hash/CryptParallelHashNull.c - Hash/CryptSm3Null.c Hash/CryptMd5.c Hash/CryptSha1.c + $(BASE_CRYPT_PATH)/Hash/CryptCShake256.c + $(BASE_CRYPT_PATH)/Hash/CryptDispatchApDxe.c + $(BASE_CRYPT_PATH)/Hash/CryptParallelHash.c + $(BASE_CRYPT_PATH)/Hash/CryptSha3.c + $(BASE_CRYPT_PATH)/Hash/CryptSm3.c + $(BASE_CRYPT_PATH)/Hash/CryptXkcp.c Hmac/CryptHmac.c Kdf/CryptHkdf.c Pk/CryptRsaBasic.c diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf b/CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf index 72b22a24e8..a153c0c8e4 100644 --- a/CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf +++ b/CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf @@ -26,6 +26,7 @@ MODULE_TYPE= PEIM VERSION_STRING = 1.0 LIBRARY_CLASS = BaseCryptLib|PEIM PEI_CORE + DEFINE BASE_CRYPT_PATH = ../BaseCryptLib # # The following information is for reference only and not required by the build tools. @@ -38,9 +39,13 @@ Hash/CryptMd5.c Hash/CryptSha1.c Hash/CryptSha256.c - Hash/CryptSm3Null.c Hash/CryptSha512.c - Hash/CryptParallelHashNull.c + $(BASE_CRYPT_PATH)/Hash/CryptCShake256.c + $(BASE_CRYPT_PATH)/Hash/CryptDispatchApPei.c + $(BASE_CRYPT_PATH)/Hash/CryptParallelHash.c + $(BASE_CRYPT_PATH)/Hash/CryptSha3.c + $(BASE_CRYPT_PATH)/Hash/CryptSm3.c + $(BASE_CRYPT_PATH)/Hash/CryptXkcp.c Hmac/CryptHmac.c Kdf/CryptHkdf.c Cipher/CryptAes.c diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf b/CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf index 9f17ef00bf..1b33dbdaad 100644 --- a/CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf +++ b/CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf @@ -25,6 +25,7 @@ VERSION_STRING = 1.0 LIBRARY_CLASS = BaseCryptLib|DXE_RUNTIME_DRIVER CONSTRUCTOR= RuntimeCryptLibConstructor + DEFINE BASE_CRYPT_PATH = ../BaseCryptLib # # The following information is for reference only and not required by the build tools. @@ -37,9 +38,9 @@ Hash/CryptMd5.c Hash/CryptSha1.c Hash/CryptSha256.c - Hash/CryptSm3Null.c Hash/CryptSha512.c Hash/CryptParallelHashNull.c + $(BASE_CRYPT_PATH)/Hash/CryptSm3.c Hmac/CryptHmac.c Kdf/CryptHkdf.c Cipher/CryptAes.c diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf b/CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf index 40c56d1b7d..d9a9cb8d10 100644 --- a/CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf +++ b/CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf @@ -24,6 +24,7 @@ VERSION_STRING = 1.0 PI_SPECIFICATION_VERSION = 0x0001000A LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE + DEFINE BASE_CRYPT_PATH = ../BaseCryptLib # # The following information is for reference only and not required by the build tools. @@ -36,9 +37,13 @@ Hash/CryptMd5.c Hash/CryptSha1.c Hash/CryptSha256.c - Hash/CryptSm3Null.c Hash/CryptSha512.c -