[edk2-devel] [PATCH] NanhuDev:Add BOSC NanhuDev platform

[WangYang]

This commit adds the initial support for BOSC's
nanhu platform which provides up to 2 RISC-V RV64
processor cores.


Signed-off-by: Yang Wang 
Signed-off-by: Ran Wang 
Signed-off-by: YunFeng Yang 
Signed-off-by: YaXing Guo 
Cc: Bamvor Jian ZHANG 
Cc: Sunil V L 
---
 .../NanhuDev/DeviceTree.fdf.inc   |  36 ++
 .../NanhuDev/DeviceTree/NanhuDev.dts  | 120 
 .../NanhuDev/DeviceTree/NanhuDeviceTree.inf   |  22 +
 .../XiangshanSeriesPkg/NanhuDev/NanhuDev.dec  |  24 +
 .../XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc  | 547 ++
 .../XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf  | 326 +++
 .../NanhuDev/NanhuDev.fdf.inc | 108 
 .../XiangshanSeriesPkg/NanhuDev/NanhuDev.uni  |  13 +
 .../NanhuDev/NanhuDevPkgExtra.uni |  12 +
 .../NanhuDev/Universal/Sec/SecMain.inf|  82 +++
 .../NanhuDev/VarStore.fdf.inc |  70 +++
 Platform/Bosc/XiangshanSeriesPkg/Readme.md|  59 ++
 .../XiangshanSeriesPkg/XiangshanSeriesPkg.dec |  29 +
 .../XiangshanSeriesPkg/XiangshanSeriesPkg.uni |  12 +
 .../XiangshanSeriesPkgExtra.uni   |  12 +
 Silicon/Bosc/NanHuPkg/NanHuPkg.dec|  33 ++
 16 files changed, 1505 insertions(+)
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree.fdf.inc
 create mode 100644 
Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree/NanhuDev.dts
 create mode 100644 
Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree/NanhuDeviceTree.inf
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dec
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf.inc
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.uni
 create mode 100644 
Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDevPkgExtra.uni
 create mode 100644 
Platform/Bosc/XiangshanSeriesPkg/NanhuDev/Universal/Sec/SecMain.inf
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/NanhuDev/VarStore.fdf.inc
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/Readme.md
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/XiangshanSeriesPkg.dec
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/XiangshanSeriesPkg.uni
 create mode 100644 Platform/Bosc/XiangshanSeriesPkg/XiangshanSeriesPkgExtra.uni
 create mode 100644 Silicon/Bosc/NanHuPkg/NanHuPkg.dec

diff --git a/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree.fdf.inc 
b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree.fdf.inc
new file mode 100644
index 00..f489e5631f
--- /dev/null
+++ b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree.fdf.inc
@@ -0,0 +1,36 @@
+## @file
+#  FDF include file with Layout Regions that define an empty variable store.
+#
+#  Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights 
reserved.
+#  Copyright (C) 2014, Red Hat, Inc.
+#  Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
+#  Copyright (c) 2024, BOSC. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+$(DTB_OFFSET)|$(DTB_SIZE)
+gUefiRiscVPlatformPkgTokenSpaceGuid.PcdRiscVDtbFvBase|gUefiRiscVPlatformPkgTokenSpaceGuid.PcdRiscVDtbFvSize
+FV = DTBFV
+
+[FV.DTBFV]
+BlockSize  = 0x1000
+FvAlignment= 16
+ERASE_POLARITY = 1
+MEMORY_MAPPED  = TRUE
+STICKY_WRITE   = TRUE
+LOCK_CAP   = TRUE
+LOCK_STATUS= TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP  = TRUE
+WRITE_STATUS   = TRUE
+WRITE_LOCK_CAP = TRUE
+WRITE_LOCK_STATUS  = TRUE
+READ_DISABLED_CAP  = TRUE
+READ_ENABLED_CAP   = TRUE
+READ_STATUS= TRUE
+READ_LOCK_CAP  = TRUE
+READ_LOCK_STATUS   = TRUE
+
+INF RuleOverride = DTB 
Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree/NanhuDeviceTree.inf
diff --git a/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree/NanhuDev.dts 
b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree/NanhuDev.dts
new file mode 100644
index 00..12617cab69
--- /dev/null
+++ b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/DeviceTree/NanhuDev.dts
@@ -0,0 +1,120 @@
+/** @file
+ Nanhu platform Device Tree
+
+ Copyright (c) 2024, BOSC. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+/dts-v1/;
+/ {
+#address-cells = <2>;
+#size-cells = <2>;
+compatible = "bosc,nanhu-dev";
+model = "bosc,nanhu-dev";
+cpus {
+#address-cells = <1>;
+#size-cells = <0>;
+timebase-frequency = <50>;//500Khz
+cpu0: cpu@0 {
+clock-frequency = <100>;
+compatible = "bosc,nanhu", "riscv";
+d-cache-block-size = <64>;
+d-cache-sets = <64>;
+d-cache-size = <16384>;
+d-tlb-sets = <1>;
+d-tlb-size = <32>;
+device_type = "cpu";
+i-cache-block-size = <64>;
+i-cache-sets = <64>;
+

Re: [edk2-devel] [PATCH v2 00/23] Provide SEV-SNP support for running under an SVSM

[Yao, Jiewen]

Some feedback:

1) 0002-MdePkg-GHCB-APIC-ID-retrieval-support-definitions

MdePkg only contains the definition in the standard.

Question: Is EFI_APIC_IDS_GUID definition in some AMD/SVSM specification?

2) 0012-UefiCpuPkg-CcSvsmLib-Create-the-CcSvsmLib-library-to-support-an-SVSM

I am not sure the position of SVSM.
If the SVSM interface is AMD specific, the it should be AmdSvsmLib.
If the SVSM interface is generic, then we should define everything in a generic 
way.

It is very confusing to mix a generic CcSvsm lib with AMD specific 
.


Thank you
Yao, Jiewen

> -Original Message-
> From: Tom Lendacky 
> Sent: Friday, February 23, 2024 1:30 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Aktas, Erdem
> ; Gerd Hoffmann ; Yao, Jiewen
> ; Laszlo Ersek ; Liming Gao
> ; Kinney, Michael D ;
> Xu, Min M ; Liu, Zhiguang ;
> Kumar, Rahul R ; Ni, Ray ; Michael
> Roth 
> Subject: [PATCH v2 00/23] Provide SEV-SNP support for running under an SVSM
> 
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> This series adds SEV-SNP support for running OVMF under an Secure VM
> Service Module (SVSM) at a less privileged VM Privilege Level (VMPL).
> By running at a less priviledged VMPL, the SVSM can be used to provide
> services, e.g. a virtual TPM, for the guest OS within the SEV-SNP
> confidential VM (CVM) rather than trust such services from the hypervisor.
> 
> Currently, OVMF expects to run at the highest VMPL, VMPL0, and there are
> certain SNP related operations that require that VMPL level. Specifically,
> the PVALIDATE instruction and the RMPADJUST instruction when setting the
> the VMSA attribute of a page (used when starting APs).
> 
> If OVMF is to run at a less privileged VMPL, e.g. VMPL2, then it must
> use an SVSM (which is running at VMPL0) to perform the operations that
> it is no longer able to perform.
> 
> When running under an SVSM, OVMF must know the APIC IDs of the vCPUs that
> it will be starting. As a result, the GHCB APIC ID retrieval action must
> be performed. Since this service can also work with SEV-SNP running at
> VMPL0, the patches to make use of this feature are near the beginning of
> the series.
> 
> How OVMF interacts with and uses the SVSM is documented in the SVSM
> specification [1] and the GHCB specification [2].
> 
> This support creates a new CcSvsmLib library that is used by MpInitLib.
> This requires an update to the edk2-platform DSC files to add the new
> library. The edk2-platform change would be needed after patch 12, but
> before patch 15.
> 
> This series introduces support to run OVMF under an SVSM. It consists
> of:
>   - Retrieving the list of vCPU APIC IDs and starting up all APs without
> performing a broadcast SIPI
>   - Reorganizing the page state change support to not directly use the
> GHCB buffer since an SVSM will use the calling area buffer, instead
>   - Detecting the presence of an SVSM
>   - When not running at VMPL0, invoking the SVSM for page validation and
> VMSA page creation/deletion
>   - Detecting and allowing OVMF to run in a VMPL other than 0 when an
> SVSM is present
> 
> The series is based off of commit:
> 
>   2ca8d5597443 ("UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before
> lock cmpxchg")
> 
> [1] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-
> docs/specifications/58019.pdf
> [2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-
> docs/specifications/56421.pdf
> 
> ---
> 
> Changes in v2:
> - Move the APIC IDs retrieval support to the beginning of the patch series
> - Use a GUIDed HOB to hold the APIC ID list instead of a PCD
> - Split up Page State Change reorganization into multiple patches
> - Created CcSvsmLib library instead of extending CcExitLib
> - This will require a corresponding update to edk2-platform DSC files
> - Removed Ray Ni's Acked-by since it is not a minor change
> - Variable name changes and other misc changes
> 
> Tom Lendacky (23):
>   OvmfPkg/BaseMemEncryptLib: Fix error check from AsmRmpAdjust()
>   MdePkg: GHCB APIC ID retrieval support definitions
>   OvmfPkg/PlatformPei: Retrieve APIC IDs from the hypervisor
>   UefiCpuPkg/MpInitLib: Always use AP Create if PcdSevSnpApicIds is set
>   OvmfPkg/BaseMemEncryptSevLib: Fix uncrustify errors
>   OvmfPkg/BaseMemEncryptSevLib: Calculate memory size for Page State
> Change
>   MdePkg: Avoid hardcoded value for number of Page State Change entries
>   OvmfPkg/BaseMemEncryptSevLib: Re-organize page state change support
>   OvmfPkg/BaseMemEncryptSevLib: Maximize Page State Change efficiency
>   MdePkg/Register/Amd: Define the SVSM related information
>   MdePkg/BaseLib: Add a new VMGEXIT instruction invocation for SVSM
>   UefiCpuPkg/CcSvsmLib: Create the CcSvsmLib library to support an SVSM
>   UefiPayloadPkg: Prepare UefiPayloadPkg to use the CcSvsmLib library
>   Ovmfpkg/CcSvsmLib: Create CcSvsmLib to handle SVSM related services
>   UefiCpuPkg/MpInitLib: Use 

Re: [edk2-devel] [PATCH v4 3/3] SecurityPkg: Update ReceiveData and SendData function description

[Yao, Jiewen]

Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Shang, Qingyu 
> Sent: Monday, February 26, 2024 11:06 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen 
> Subject: [PATCH v4 3/3] SecurityPkg: Update ReceiveData and SendData function
> description
> 
> Refer to Uefi spec 2.10 section 13.14, update the parameter 'MediaId'
> description for EFI_STORAGE_SECURITY_COMMAND_PROTOCOL function
> ReceiveData
> and SendData.
> 
> Cc: Jiewen Yao 
> Signed-off-by: Qingyu 
> ---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> index 0fb6b1bf41d5..1ee55105e435 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> @@ -49,7 +49,9 @@ EFI_GUID  mOpalDeviceLockBoxGuid =
> OPAL_DEVICE_LOCKBOX_GUID;
>function shall return EFI_DEVICE_ERROR.
> 
>@param  This Indicates a pointer to the calling 
> context.
> -  @param  MediaId  ID of the medium to receive data from.
> +  @param  MediaId  ID of the medium to receive data 
> from. If there is
> no
> +   block IO protocol supported by the 
> physical device, the
> +   value of MediaId is undefined.
>@param  Timeout  The timeout, in 100ns units, to use 
> for the
> execution
> of the security protocol command. A 
> Timeout value of 0
> means that this function will wait 
> indefinitely for the
> @@ -148,7 +150,9 @@ SecurityReceiveData (
>shall return EFI_DEVICE_ERROR.
> 
>@param  This Indicates a pointer to the calling 
> context.
> -  @param  MediaId  ID of the medium to receive data from.
> +  @param  MediaId  ID of the medium to receive data 
> from. If there is
> no
> +   block IO protocol supported by the 
> physical device, the
> +   value of MediaId is undefined.
>@param  Timeout  The timeout, in 100ns units, to use 
> for the
> execution
> of the security protocol command. A 
> Timeout value of 0
> means that this function will wait 
> indefinitely for the
> --
> 2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116091): https://edk2.groups.io/g/devel/message/116091
Mute This Topic: https://groups.io/mt/104581211/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] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec

[Yao, Jiewen]

Thanks for the update.

First, would you please clarify which test you have done for this patch set.
Have you tested all previous function to ensure it still works?

Second, would you please clarify if there is any compatibility issue to follow 
the new UEFI 2.10?
For example, what if the core HII is still UEFI 2.9? would that still work?

Third, because I am not HII expert, I would like to have HII expert to comment 
the HII/Browser related change.

Thank you
Yao, Jiewen

> -Original Message-
> From: Tan, Ming 
> Sent: Tuesday, February 27, 2024 10:59 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M ; Yao, Jiewen 
> Subject: [PATCH v2] SecurityPkg/SecureBootConfigDxe: Update UI according to
> UEFI spec
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4713
> 
> In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
> EFI_BROWSER_ACTION_FORM_OPEN:
> NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used
> with
> this browser action because question values have not been retrieved yet.
> 
> So should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN
> call back function.
> 
> Now call SecureBootExtractConfigFromVariable() to save the change to EFI
> variable, then HII use EFI variable to control the UI.
> 
> Cc: Min Xu 
> Cc: Jiewen Yao 
> Signed-off-by: Ming Tan 
> ---
>   V2: Change code style to pass uncrustify check.
> 
>  .../SecureBootConfigImpl.c| 37 ++-
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> index 2c11129526..e2e61d1e07 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> @@ -3366,6 +3366,8 @@ SecureBootExtractConfigFromVariable (
>  ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;
> 
>}
> 
> 
> 
> +  ConfigData->ListCount = Private->ListCount;
> 
> +
> 
>//
> 
>// If it is Physical Presence User, set the PhysicalPresent to true.
> 
>//
> 
> @@ -4541,12 +4543,13 @@ SecureBootCallback (
>EFI_HII_POPUP_PROTOCOL  *HiiPopup;
> 
>EFI_HII_POPUP_SELECTION UserSelection;
> 
> 
> 
> -  Status = EFI_SUCCESS;
> 
> -  SecureBootEnable   = NULL;
> 
> -  SecureBootMode = NULL;
> 
> -  SetupMode  = NULL;
> 
> -  File   = NULL;
> 
> -  EnrollKeyErrorCode = None_Error;
> 
> +  Status   = EFI_SUCCESS;
> 
> +  SecureBootEnable = NULL;
> 
> +  SecureBootMode   = NULL;
> 
> +  SetupMode= NULL;
> 
> +  File = NULL;
> 
> +  EnrollKeyErrorCode   = None_Error;
> 
> +  GetBrowserDataResult = FALSE;
> 
> 
> 
>if ((This == NULL) || (Value == NULL) || (ActionRequest == NULL)) {
> 
>  return EFI_INVALID_PARAMETER;
> 
> @@ -4565,15 +4568,12 @@ SecureBootCallback (
>  return EFI_OUT_OF_RESOURCES;
> 
>}
> 
> 
> 
> -  GetBrowserDataResult = HiiGetBrowserData
> (, mSecureBootStorageName, BufferSize,
> (UINT8 *)IfrNvData);
> 
> -
> 
>if (Action == EFI_BROWSER_ACTION_FORM_OPEN) {
> 
>  if (QuestionId == KEY_SECURE_BOOT_MODE) {
> 
>//
> 
>// Update secure boot strings when opening this form
> 
>//
> 
> -  Status = UpdateSecureBootString (Private);
> 
> -  SecureBootExtractConfigFromVariable (Private, IfrNvData);
> 
> +  Status = UpdateSecureBootString (Private);
> 
>mIsEnterSecureBootForm = TRUE;
> 
>  } else {
> 
>//
> 
> @@ -4587,23 +4587,22 @@ SecureBootCallback (
>(QuestionId == KEY_SECURE_BOOT_DBT_OPTION))
> 
>{
> 
>  CloseEnrolledFile (Private->FileContext);
> 
> -  } else if (QuestionId == KEY_SECURE_BOOT_DELETE_ALL_LIST) {
> 
> -//
> 
> -// Update ListCount field in varstore
> 
> -// Button "Delete All Signature List" is
> 
> -// enable when ListCount is greater than 0.
> 
> -//
> 
> -IfrNvData->ListCount = Private->ListCount;
> 
>}
> 
>  }
> 
> 
> 
>  goto EXIT;
> 
>}
> 
> 
> 
> +  GetBrowserDataResult = HiiGetBrowserData
> (, mSecureBootStorageName, BufferSize,
> (UINT8 *)IfrNvData);
> 
> +
> 
>if (Action == EFI_BROWSER_ACTION_RETRIEVE) {
> 
>  Status = EFI_UNSUPPORTED;
> 
>  if (QuestionId == KEY_SECURE_BOOT_MODE) {
> 
>if (mIsEnterSecureBootForm) {
> 
> +if (GetBrowserDataResult) {
> 
> +  SecureBootExtractConfigFromVariable (Private, IfrNvData);
> 
> +}
> 
> +
> 
>  Value->u8 = SECURE_BOOT_MODE_STANDARD;
> 
>  Status= EFI_SUCCESS;
> 
>}
> 
> @@ -5179,6 +5178,10 @@ SecureBootCallback (
>  }
> 
>}
> 
> 
> 
> +  if (GetBrowserDataResult) {
> 
> +SecureBootExtractConfigFromVariable (Private, IfrNvData);
> 
> +  }
> 
> +
> 

Re: [edk2-devel] [PATCH 09/10] OvmfPkg/ResetVector: leave SEV VC handler installed longer

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> When running in SEV mode keep the VC handler installed.
> Add a function to uninstall it later.
> 
> This allows using the cpuid instruction in SetCr3ForPageTables64,
> which is needed to check for la57 & 1G page support.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm   | 12 ++--
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm |  1 +
>  OvmfPkg/ResetVector/Main.asm  |  4 
>  3 files changed, 15 insertions(+), 2 deletions(-)

I'll leave this to Tom.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116089): https://edk2.groups.io/g/devel/message/116089
Mute This Topic: https://groups.io/mt/104506799/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 10/10] OvmfPkg/ResetVector: wire up 5-level paging for SEV

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Removes the GetSevCBitMaskAbove31 OneTimeCall because we need that twice
> (for 4-level and 5-level paging).  Open code the single instruction left
> in that function instead.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm   |  8 
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 14 +-
>  2 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm 
> b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 9063ce1080d3..d1e5e8dfae71 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -191,14 +191,6 @@ pageTableEntries4kLoop:
>  SevClearPageEncMaskForGhcbPageExit:
>  OneTimeCallRet SevClearPageEncMaskForGhcbPage
>  
> -; Get the C-bit mask above 31.
> -; Modified: EDX
> -;
> -; The value is returned in the EDX
> -GetSevCBitMaskAbove31:
> -mov   edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> -OneTimeCallRet GetSevCBitMaskAbove31
> -
>  %endif
>  
>  ; Check if Secure Encrypted Virtualization (SEV) features are enabled.
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 6e2063430802..55664fa64f62 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -243,11 +243,23 @@ SevInit:
>  ; SEV workflow
>  ;
>  ClearOvmfPageTables
> +%if PG_5_LEVEL
> +Check5LevelPaging Sev4Level
>  ; If SEV is enabled, the C-bit position is always above 31.
>  ; The mask will be saved in the EDX and applied during the
>  ; the page table build below.
> -OneTimeCall   GetSevCBitMaskAbove31
> +mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> +CreatePageTables5Level edx
> +Enable5LevelPaging
> +jmp SevCommon
> +Sev4Level:
> +%endif
> +; If SEV is enabled, the C-bit position is always above 31.
> +; The mask will be saved in the EDX and applied during the
> +; the page table build below.
> +mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
>  CreatePageTables4Level edx
> +SevCommon:
>  ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>  OneTimeCall   SevClearPageEncMaskForGhcbPage
>  OneTimeCall   SevClearVcHandlerAndStack

This patch looks fine to me (especially the SevCommon label), but can
you please reimplement GetSevCBitMaskAbove31 as a macro, rather than
open-coding the

  mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]

instruction?

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116088): https://edk2.groups.io/g/devel/message/116088
Mute This Topic: https://groups.io/mt/104506802/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 08/10] OvmfPkg/ResetVector: wire up 5-level paging for TDX

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> BSP workflow is quite simliar to the non-coco case.
> 
> TDX_WORK_AREA_PGTBL_READY is used to record the paging mode:
>   1 == 4-level paging
>   2 == 5-level paging
> 
> APs will look at TDX_WORK_AREA_PGTBL_READY to figure whenever
> they should enable 5-level paging or not.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/Ia32/IntelTdx.asm | 13 -
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 12 
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm 
> b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> index c6b86019dfb9..7d775591a05b 100644
> --- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> +++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> @@ -179,7 +179,7 @@ InitTdx:
>  ;
>  ; Modified:  EAX, EDX
>  ;
> -; 0-NonTdx, 1-TdxBsp, 2-TdxAps
> +; 0-NonTdx, 1-TdxBsp, 2-TdxAps, 3-TdxAps5Level
>  ;
>  CheckTdxFeaturesBeforeBuildPagetables:
>  xor eax, eax
> @@ -200,6 +200,17 @@ TdxPostBuildPageTables:
>  mov byte[TDX_WORK_AREA_PGTBL_READY], 1
>  OneTimeCallRet TdxPostBuildPageTables
>  
> +%if PG_5_LEVEL
> +
> +;
> +; Set byte[TDX_WORK_AREA_PGTBL_READY] to 2
> +;
> +TdxPostBuildPageTables5Level:
> +mov byte[TDX_WORK_AREA_PGTBL_READY], 2
> +OneTimeCallRet TdxPostBuildPageTables5Level
> +
> +%endif
> +
>  ;
>  ; Check if TDX is enabled
>  ;
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index d736db028277..ada3dc0ffbe0 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -44,6 +44,7 @@ BITS32
>  
>  %define TDX_BSP 1
>  %define TDX_AP  2
> +%define TDX_AP_5_LEVEL  3
>  
>  ;
>  ; For OVMF, build some initial page tables at
> @@ -214,6 +215,10 @@ SetCr3ForPageTables64:
>  jeTdxBspInit
>  cmp   eax, TDX_AP
>  jeSetCr3
> +%if PG_5_LEVEL
> +cmp   eax, TDX_AP_5_LEVEL
> +jeSetCr3La57
> +%endif
>  
>  ; Check whether the SEV is active and populate the SevEsWorkArea
>  OneTimeCall   CheckSevFeatures
> @@ -252,6 +257,13 @@ TdxBspInit:
>  ; TDX BSP workflow
>  ;
>  ClearOvmfPageTables
> +%if PG_5_LEVEL
> +Check5LevelPaging Tdx4Level
> +CreatePageTables5Level 0
> +OneTimeCall TdxPostBuildPageTables5Level
> +jmp SetCr3La57
> +Tdx4Level:
> +%endif
>  CreatePageTables4Level 0
>  OneTimeCall TdxPostBuildPageTables
>  jmp SetCr3

With SetCr3La57 eliminated as I request under patch #6, this patch looks
fine to me.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116087): https://edk2.groups.io/g/devel/message/116087
Mute This Topic: https://groups.io/mt/104506795/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 07/10] OvmfPkg/ResetVector: print post codes for 4/5 level paging

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 825589f31193..d736db028277 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -69,6 +69,10 @@ BITS32
>  ; Argument: upper 32 bits of the page table entries
>  ;
>  %macro CreatePageTables4Level 1
> +
> +; indicate 4-level paging
> +debugShowPostCode 0x41
> +
>  ;
>  ; Top level Page Directory Pointers (1 * 512GB entry)
>  ;
> @@ -153,6 +157,10 @@ BITS32
>  ; level 3 directory.
>  ;
>  %macro CreatePageTables5Level 1
> +
> +; indicate 5-level paging
> +debugShowPostCode 0x51
> +
>  ; level 5
>  mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
>  mov dword[PT_ADDR (4)], %1

Reviewed-by: Laszlo Ersek 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116086): https://edk2.groups.io/g/devel/message/116086
Mute This Topic: https://groups.io/mt/104506801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 06/10] OvmfPkg/ResetVector: add 5-level paging support

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Add macros to check for 5-level paging and gigabyte page support.
> Enable 5-level paging for the non-confidential-computing case.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/ResetVector.inf   |   1 +
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 105 ++
>  OvmfPkg/ResetVector/ResetVector.nasmb |   1 +
>  3 files changed, 107 insertions(+)
> 
> diff --git a/OvmfPkg/ResetVector/ResetVector.inf 
> b/OvmfPkg/ResetVector/ResetVector.inf
> index a4154ca90c28..65f71b05a02e 100644
> --- a/OvmfPkg/ResetVector/ResetVector.inf
> +++ b/OvmfPkg/ResetVector/ResetVector.inf
> @@ -64,3 +64,4 @@ [FixedPcd]
>gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 84a7b4efc019..825589f31193 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -101,6 +101,97 @@ BITS32
>  loop.pageTableEntriesLoop4Level
>  %endmacro
>  
> +;
> +; Check whenever 5-level paging can ca used

(1) typo in comment

> +;
> +; Argument: jump label for 4-level paging
> +;
> +%macro Check5LevelPaging 1
> +; check for cpuid leaf 0x07
> +mov eax, 0x00
> +cpuid
> +cmp eax, 0x07
> +jb  %1
> +
> +; check for la57 (aka 5-level paging)
> +mov eax, 0x07
> +mov ecx, 0x00
> +cpuid
> +bt  ecx, 16
> +jnc %1
> +
> +; check for cpuid leaf 0x8001
> +mov eax, 0x8000
> +cpuid
> +cmp eax, 0x8001
> +jb  %1
> +
> +; check for 1g pages
> +mov eax, 0x8001
> +cpuid
> +bt  edx, 26
> +jnc %1
> +%endmacro
> +
> +;
> +; Create page tables for 5-level paging with gigabyte pages
> +;
> +; Argument: upper 32 bits of the page table entries
> +;
> +; We have 6 pages available for the early page tables,
> +; we use four of them:
> +;PT_ADDR(0)  - level 5 directory
> +;PT_ADDR(0x1000) - level 4 directory
> +;PT_ADDR(0x2000) - level 2 directory (0 -> 1GB)
> +;PT_ADDR(0x3000) - level 3 directory
> +;
> +; The level 2 directory for the first gigabyte has the same
> +; physical address in both 4-level and 5-level paging mode,
> +; SevClearPageEncMaskForGhcbPage depends on this.
> +;
> +; The 1 GB -> 4 GB range is mapped using 1G pages in the
> +; level 3 directory.
> +;
> +%macro CreatePageTables5Level 1
> +; level 5
> +mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
> +mov dword[PT_ADDR (4)], %1
> +
> +; level 4
> +mov dword[PT_ADDR (0x1000)], PT_ADDR (0x3000) + 
> PAGE_PDE_DIRECTORY_ATTR
> +mov dword[PT_ADDR (0x1004)], %1
> +
> +; level 3 (1x -> level 2, 3x 1GB)
> +mov dword[PT_ADDR (0x3000)], PT_ADDR (0x2000) + 
> PAGE_PDE_DIRECTORY_ATTR
> +mov dword[PT_ADDR (0x3004)], %1
> +mov dword[PT_ADDR (0x3008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +mov dword[PT_ADDR (0x300c)], %1
> +mov dword[PT_ADDR (0x3010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +mov dword[PT_ADDR (0x3014)], %1
> +mov dword[PT_ADDR (0x3018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +mov dword[PT_ADDR (0x301c)], %1
> +
> +;
> +; level 2 (512 * 2MB entries => 1GB)
> +;
> +mov ecx, 0x200
> +.pageTableEntriesLoop5Level:

(2) suggest "..@pageTableEntriesLoop5Level" as label name

> +mov eax, ecx
> +dec eax
> +shl eax, 21
> +add eax, PAGE_PDE_LARGEPAGE_ATTR
> +mov dword[ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> +mov dword[(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], %1
> +loop.pageTableEntriesLoop5Level
> +%endmacro
> +
> +%macro Enable5LevelPaging 0
> +; set la57 bit in cr4
> +mov eax, cr4
> +bts eax, 12
> +mov cr4, eax
> +%endmacro
> +
>  ;
>  ; Modified:  EAX, EBX, ECX, EDX
>  ;
> @@ -125,6 +216,12 @@ SetCr3ForPageTables64:
>  ; normal (non-CoCo) workflow
>  ;
>  ClearOvmfPageTables
> +%if PG_5_LEVEL
> +Check5LevelPaging Paging4Level
> +CreatePageTables5Level 0
> +jmp SetCr3La57
> +Paging4Level:
> +%endif
>  CreatePageTables4Level 0
>  jmp SetCr3
>  
> @@ -151,6 +248,14 @@ TdxBspInit:
>  OneTimeCall TdxPostBuildPageTables
>  jmp SetCr3
>  
> +;
> +; common workflow
> +;
> +%if PG_5_LEVEL
> +SetCr3La57:
> +Enable5LevelPaging
> +%endif
> +
>  SetCr3:
>  ;
>  ; Set CR3 now that the paging structures are available

(3) I don't like SetCr3La57.

It saves minimal code duplication, but makes the control flow much
harder to follow.

(3.1) From this patch, we have one (unconditional) jump to SetCr3La57. I
suggest simply inlining that, like this:

%if PG_5_LEVEL
 

Re: [edk2-devel] [PATCH 05/10] OvmfPkg/ResetVector: split SEV and non-CoCo workflows

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Use separate control flows for SEV and non-CoCo cases.
> 
> SevClearPageEncMaskForGhcbPage and GetSevCBitMaskAbove31 will now only
> be called when running in SEV mode, so the SEV check in these functions
> is not needed any more.
> 
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm   | 16 ++--
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 17 ++---
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm 
> b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 043c88a7abbe..ed94f1dc668f 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -152,12 +152,8 @@ SevEsUnexpectedRespTerminate:
>  
>  %ifdef ARCH_X64
>  
> -; If SEV-ES is enabled then initialize and make the GHCB page shared
> +; initialize and make the GHCB page shared

(1) This comment update is unjustified, I suggest reverting it.

(The SEV check is indeed superfluous below, but you -- correctly -- keep
the SEV-ES check, and the comment here is about SEV-ES, not SEV. Because
the check stays, the comment should stay too.)

>  SevClearPageEncMaskForGhcbPage:
> -; Check if SEV is enabled
> -cmp   byte[WORK_AREA_GUEST_TYPE], 1
> -jnz   SevClearPageEncMaskForGhcbPageExit
> -
>  ; Check if SEV-ES is enabled
>  mov   ecx, 1
>  bt[SEV_ES_WORK_AREA_STATUS_MSR], ecx
> @@ -195,20 +191,12 @@ pageTableEntries4kLoop:
>  SevClearPageEncMaskForGhcbPageExit:
>  OneTimeCallRet SevClearPageEncMaskForGhcbPage
>  
> -; Check if SEV is enabled, and get the C-bit mask above 31.
> +; Get the C-bit mask above 31.
>  ; Modified: EDX
>  ;
>  ; The value is returned in the EDX
>  GetSevCBitMaskAbove31:
> -xor   edx, edx
> -
> -; Check if SEV is enabled
> -cmp   byte[WORK_AREA_GUEST_TYPE], 1
> -jnz   GetSevCBitMaskAbove31Exit
> -
>  mov   edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> -
> -GetSevCBitMaskAbove31Exit:
>  OneTimeCallRet GetSevCBitMaskAbove31
>  
>  %endif
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 166e80293c89..84a7b4efc019 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -118,15 +118,26 @@ SetCr3ForPageTables64:
>  
>  ; Check whether the SEV is active and populate the SevEsWorkArea
>  OneTimeCall   CheckSevFeatures
> +cmp   byte[WORK_AREA_GUEST_TYPE], 1
> +jzSevInit
>  
> +;
> +; normal (non-CoCo) workflow
> +;
> +ClearOvmfPageTables
> +CreatePageTables4Level 0
> +jmp SetCr3
> +
> +SevInit:
> +;
> +; SEV workflow
> +;
> +ClearOvmfPageTables
>  ; If SEV is enabled, the C-bit position is always above 31.
>  ; The mask will be saved in the EDX and applied during the
>  ; the page table build below.
>  OneTimeCall   GetSevCBitMaskAbove31
> -
> -ClearOvmfPageTables
>  CreatePageTables4Level edx
> -
>  ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>  OneTimeCall   SevClearPageEncMaskForGhcbPage
>  jmp SetCr3

Nice.

The patch also sneakily reorders ClearOvmfPageTables against
GetSevCBitMaskAbove31 -- but that's an improvement: this way we no
longer depend on ClearOvmfPageTables not modifying EDX; instead, EDX
directly passes from GetSevCBitMaskAbove31 to CreatePageTables4Level.

With (1) undone:

Reviewed-by: Laszlo Ersek 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116084): https://edk2.groups.io/g/devel/message/116084
Mute This Topic: https://groups.io/mt/104506794/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 0/3] RISC-V: Support Svpbmt extension

[Sunil V L]

Hi Tuan,

On Mon, Feb 26, 2024 at 08:34:22PM -0800, Tuan Phan wrote:
> Hi Sunil/ Andrei,
> Any comments on this series?
> 
Did I miss your response to Laszlo's feedback on PATCH 2 - [1]? Apart
from that, don't we need to handle EFI_MEMORY_WT similar to
EFI_MEMORY_WC?

[1] - https://edk2.groups.io/g/devel/message/115243

Thanks,
Sunil
> Regards,
> 
> On Wed, Feb 14, 2024 at 10:16 PM Tuan Phan via groups.io  ventanamicro@groups.io> wrote:
> 
> >
> >
> > On Wed, Feb 14, 2024 at 9:43 PM Warkentin, Andrei <
> > andrei.warken...@intel.com> wrote:
> >
> >> Do you mind sharing a GH branch with the patch set?
> >>
> > https://github.com/pttuan/edk2/tree/tphan/riscv_mmu_svpbmt
> > Tuan
> >
> >>
> >> A
> >>
> >> > -Original Message-
> >> > From: Tuan Phan 
> >> > Sent: Tuesday, February 6, 2024 7:29 PM
> >> > To: devel@edk2.groups.io
> >> > Cc: Kinney, Michael D ;
> >> > gaolim...@byosoft.com.cn; Liu, Zhiguang ;
> >> > kra...@redhat.com; ler...@redhat.com; Kumar, Rahul R
> >> > ; Ni, Ray ;
> >> > suni...@ventanamicro.com; Yao, Jiewen ;
> >> Warkentin,
> >> > Andrei ; ardb+tianoc...@kernel.org; Tuan
> >> Phan
> >> > 
> >> > Subject: [PATCH v2 0/3] RISC-V: Support Svpbmt extension
> >> >
> >> > This patchset adds support for RISC-V Svpbmt extension.
> >> >
> >> > The GCD EFI_MEMORY_UC and EFI_MEMORY_WC attributes will be mapped to
> >> > IO and NC mode defined in PBMT field.
> >> >
> >> > v2:
> >> >   - Generated patch for each package.
> >> >
> >> > Tuan Phan (3):
> >> >   MdePkg.dec: RISC-V: Define override bit for Svpbmt extension
> >> >   UefiCpuPkg: RISC-V: MMU: Support Svpbmt extension
> >> >   OvmfPkg/RiscVVirt: Override Svpbmt extension
> >> >
> >> >  MdePkg/MdePkg.dec |  2 ++
> >> >  OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc   |  2 +-
> >> >  .../Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c | 25 ++-
> >> >  .../BaseRiscVMmuLib/BaseRiscVMmuLib.inf   |  1 +
> >> >  4 files changed, 28 insertions(+), 2 deletions(-)
> >> >
> >> > --
> >> > 2.25.1
> >>
> >> 
> >
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116083): https://edk2.groups.io/g/devel/message/116083
Mute This Topic: https://groups.io/mt/104211191/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 04/10] OvmfPkg/ResetVector: split TDX BSP workflow

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Create a separate control flow for TDX BSP.
> 
> TdxPostBuildPageTables will now only be called when running in TDX
> mode, so the TDX check in that function is not needed any more.
> 
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/Ia32/IntelTdx.asm |  4 
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 15 ++-
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm 
> b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> index 06794baef81d..c6b86019dfb9 100644
> --- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> +++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> @@ -197,11 +197,7 @@ NotTdx:
>  ; Set byte[TDX_WORK_AREA_PGTBL_READY] to 1
>  ;
>  TdxPostBuildPageTables:
> -cmp byte[WORK_AREA_GUEST_TYPE], VM_GUEST_TDX
> -jne ExitTdxPostBuildPageTables
>  mov byte[TDX_WORK_AREA_PGTBL_READY], 1
> -
> -ExitTdxPostBuildPageTables:
>  OneTimeCallRet TdxPostBuildPageTables
>  
>  ;
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 14cc2c33aa3d..166e80293c89 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -112,7 +112,7 @@ SetCr3ForPageTables64:
>  ; is set.
>  OneTimeCall   CheckTdxFeaturesBeforeBuildPagetables
>  cmp   eax, TDX_BSP
> -jeClearOvmfPageTables
> +jeTdxBspInit
>  cmp   eax, TDX_AP
>  jeSetCr3
>  
> @@ -124,16 +124,21 @@ SetCr3ForPageTables64:
>  ; the page table build below.
>  OneTimeCall   GetSevCBitMaskAbove31
>  
> -ClearOvmfPageTables:
>  ClearOvmfPageTables
>  CreatePageTables4Level edx
>  
>  ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>  OneTimeCall   SevClearPageEncMaskForGhcbPage
> +jmp SetCr3
>  
> -; TDX will do some PostBuildPages task, such as setting
> -; byte[TDX_WORK_AREA_PGTBL_READY].
> -OneTimeCall   TdxPostBuildPageTables
> +TdxBspInit:
> +;
> +; TDX BSP workflow
> +;
> +ClearOvmfPageTables
> +CreatePageTables4Level 0
> +OneTimeCall TdxPostBuildPageTables
> +jmp SetCr3
>  
>  SetCr3:
>  ;

Reviewed-by: Laszlo Ersek 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116082): https://edk2.groups.io/g/devel/message/116082
Mute This Topic: https://groups.io/mt/104506797/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 03/10] OvmfPkg/ResetVector: add CreatePageTables4Level macro

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Move code to create 4-level page tables to a nasm macro.
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 70 +--
>  1 file changed, 39 insertions(+), 31 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 378ba2feeb4f..14cc2c33aa3d 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -63,6 +63,44 @@ BITS32
>  loop.clearPageTablesMemoryLoop
>  %endmacro
>  
> +;
> +; Create page tables for 4-level paging
> +;
> +; Argument: upper 32 bits of the page table entries
> +;
> +%macro CreatePageTables4Level 1
> +;
> +; Top level Page Directory Pointers (1 * 512GB entry)
> +;
> +mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
> +mov dword[PT_ADDR (4)], %1
> +
> +;
> +; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
> +;
> +mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + 
> PAGE_PDE_DIRECTORY_ATTR
> +mov dword[PT_ADDR (0x1004)], %1
> +mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + 
> PAGE_PDE_DIRECTORY_ATTR
> +mov dword[PT_ADDR (0x100C)], %1
> +mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + 
> PAGE_PDE_DIRECTORY_ATTR
> +mov dword[PT_ADDR (0x1014)], %1
> +mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + 
> PAGE_PDE_DIRECTORY_ATTR
> +mov dword[PT_ADDR (0x101C)], %1
> +
> +;
> +; Page Table Entries (2048 * 2MB entries => 4GB)
> +;
> +mov ecx, 0x800
> +.pageTableEntriesLoop4Level:
> +mov eax, ecx
> +dec eax
> +shl eax, 21
> +add eax, PAGE_PDE_LARGEPAGE_ATTR
> +mov dword[ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> +mov dword[(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], %1
> +loop.pageTableEntriesLoop4Level
> +%endmacro
> +
>  ;
>  ; Modified:  EAX, EBX, ECX, EDX
>  ;
> @@ -88,37 +126,7 @@ SetCr3ForPageTables64:
>  
>  ClearOvmfPageTables:
>  ClearOvmfPageTables
> -
> -;
> -; Top level Page Directory Pointers (1 * 512GB entry)
> -;
> -mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
> -mov dword[PT_ADDR (4)], edx
> -
> -;
> -; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
> -;
> -mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + 
> PAGE_PDE_DIRECTORY_ATTR
> -mov dword[PT_ADDR (0x1004)], edx
> -mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + 
> PAGE_PDE_DIRECTORY_ATTR
> -mov dword[PT_ADDR (0x100C)], edx
> -mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + 
> PAGE_PDE_DIRECTORY_ATTR
> -mov dword[PT_ADDR (0x1014)], edx
> -mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + 
> PAGE_PDE_DIRECTORY_ATTR
> -mov dword[PT_ADDR (0x101C)], edx
> -
> -;
> -; Page Table Entries (2048 * 2MB entries => 4GB)
> -;
> -mov ecx, 0x800
> -pageTableEntriesLoop:
> -mov eax, ecx
> -dec eax
> -shl eax, 21
> -add eax, PAGE_PDE_LARGEPAGE_ATTR
> -mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> -mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
> -looppageTableEntriesLoop
> +CreatePageTables4Level edx
>  
>  ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>  OneTimeCall   SevClearPageEncMaskForGhcbPage

Nice.

"--color-moved=zebra" is really useful for viewing this patch.

Same comment on the ".pageTableEntriesLoop4Level" label name as under
patch#2: can we use "..@pageTableEntriesLoop4Level"?

Reviewed-by: Laszlo Ersek 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116081): https://edk2.groups.io/g/devel/message/116081
Mute This Topic: https://groups.io/mt/104506791/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/10] OvmfPkg/ResetVector: add ClearOvmfPageTables macro

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Move code to clear the page tables to a nasm macro.
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 35 ---
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 6fec6f2beeea..378ba2feeb4f 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -45,6 +45,24 @@ BITS32
>  %define TDX_BSP 1
>  %define TDX_AP  2
>  
> +;
> +; For OVMF, build some initial page tables at
> +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
> +;
> +; This range should match with PcdOvmfSecPageTablesSize which is
> +; declared in the FDF files.
> +;
> +; At the end of PEI, the pages tables will be rebuilt into a
> +; more permanent location by DxeIpl.
> +;
> +%macro ClearOvmfPageTables 0
> +mov ecx, 6 * 0x1000 / 4
> +xor eax, eax
> +.clearPageTablesMemoryLoop:
> +mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
> +loop.clearPageTablesMemoryLoop
> +%endmacro
> +
>  ;
>  ; Modified:  EAX, EBX, ECX, EDX
>  ;

Ah, this made me read up on local labels:

https://www.nasm.us/xdoc/2.16.01/html/nasmdoc3.html#section-3.9

Should we rather call the label

  ..@clearPageTablesMemoryLoop

?

According to the documentation, that seems to be the safest / most
robust label type to be used inside macros.

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

> @@ -69,22 +87,7 @@ SetCr3ForPageTables64:
>  OneTimeCall   GetSevCBitMaskAbove31
>  
>  ClearOvmfPageTables:
> -;
> -; For OVMF, build some initial page tables at
> -; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
> -;
> -; This range should match with PcdOvmfSecPageTablesSize which is
> -; declared in the FDF files.
> -;
> -; At the end of PEI, the pages tables will be rebuilt into a
> -; more permanent location by DxeIpl.
> -;
> -
> -mov ecx, 6 * 0x1000 / 4
> -xor eax, eax
> -clearPageTablesMemoryLoop:
> -mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
> -loopclearPageTablesMemoryLoop
> +ClearOvmfPageTables
>  
>  ;
>  ; Top level Page Directory Pointers (1 * 512GB entry)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116080): https://edk2.groups.io/g/devel/message/116080
Mute This Topic: https://groups.io/mt/104506789/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: add struct MP_HAND_OFF_CONFIG

[Laszlo Ersek]

On 2/27/24 12:41, Gerd Hoffmann wrote:
> Move the WaitLoopExecutionMode and StartupSignalValue fields to a
> separate HOB with the new struct.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 13 ++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.h |  3 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c  | 34 +
>  4 files changed, 66 insertions(+), 22 deletions(-)

Ah, please also append a few sentences to the commit message -- we
should explain *why* this change is good:

WaitLoopExecutionMode and StartupSignalValue are independent of
processor index ranges; they are global to MpInitLib (i.e., the entire
system). Therefore they shouldn't be repeated in every MpHandOff GUID HOB.

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116079): https://edk2.groups.io/g/devel/message/116079
Mute This Topic: https://groups.io/mt/104600810/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] CodeQL Analysis in edk2

[Michael Kubacki]

On 2/27/2024 10:43 PM, Laszlo Ersek wrote:

On 2/27/24 17:04, Michael Kubacki wrote:

Hi Gerd,

There is a way to suppress results explained here:
https://github.com/tianocore/edk2/tree/master/BaseTools/Plugin/CodeQL#filter-patterns

A real-world example is here:
https://github.com/microsoft/mu_basecore/blob/release/202311/CodeQlFilters.yml

That can currently operate at the file and CodeQL rule level
granularity. In this case, the null pointer test rule
("cpp/missing-null-test" as shown in
https://github.com/tianocore/edk2/security/code-scanning/1277) could be
excluded in MpLib.c.

---

Taking a quick look at the code highlighted:

     MpHandOffConfig = GetMpHandOffConfigHob ();
     ASSERT (MpHandOffConfig != NULL);
     DEBUG ((
   DEBUG_INFO,
   "FirstMpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *):
%04d\n",
   MpHandOffConfig->WaitLoopExecutionMode,
   sizeof (VOID *)
   ));
     if (MpHandOffConfig->WaitLoopExecutionMode == sizeof (VOID *)) {

CodeQL flagged the two MpHandOffConfig dereferences. These is assigned
on the return value from GetMpHandOffConfigHob () defined as:

/**
   Get pointer to MP_HAND_OFF_CONFIG GUIDed HOB body.

   @return  The pointer to MP_HAND_OFF_CONFIG structure.
**/
MP_HAND_OFF_CONFIG *
GetMpHandOffConfigHob (
   VOID
   )
{
   EFI_HOB_GUID_TYPE  *GuidHob;

   GuidHob = GetFirstGuidHob ();
   if (GuidHob == NULL) {
     return NULL;
   }

   return (MP_HAND_OFF_CONFIG *)GET_GUID_HOB_DATA (GuidHob);
}

Can you please provide more context about why you believe a NULL return
value from the function is not a consideration? Generally, anything is
possible in the HOB list, for example, other code could overflow a HOB
boundary and mutate the this HOB's header preventing it from being found.


The GetMpHandOffConfigHob() function itself is right, when viewed in
isolation, to deal with the possibility of the HOB missing.

However, the GetMpHandOffConfigHob() *call site* is only reached if the
"FirstMpHandOff" variable is not NULL.

And if "FirstMpHandOff" is not NULL, then in the PEI phase, the
SaveCpuMpData() function [UefiCpuPkg/Library/MpInitLib/PeiMpLib.c] will
have prepared *both* at least one MpHandOff GUID HOB, *and* the sole
MpHandOffConfig GUID HOB.

In other words, the presence of at least one MpHandOff GUID HOB
guarantees that there is going to be exactly one MpHandOffConfig GUID HOB.

Therefore the ASSERT() is right -- it expresses an invariant.


Thanks. With those details, I agree ASSERT() is reasonable.


That said:



ASSERT() is useful for debug but it's control path is unpredictable in
core code based on platform policies that often have varying
perspectives of how to apply asserts and how they should be configured
in different scenarios. We introduced a "panic library" to use in rare
cases where we want more consistent behavior for fatal conditions.


I agree that making this more explicit wouldn't hurt.

The panic library is not upstream (yet? :))


I added Ken from my team as a reminder to prepare that for upstream soon.

, so I'll suggest a

CpuDeadLoop() under the patch itself:

[PATCH 1/1] UefiCpuPkg/MpInitLib: add struct MP_HAND_OFF_CONFIG
msgid <20240227114122.1140614-1-kra...@redhat.com>
https://edk2.groups.io/g/devel/message/116029

Thanks,
Laszlo



Thanks,
Michael

On 2/27/2024 6:39 AM, Gerd Hoffmann wrote:

    Hi,


I am hoping we can work together to improve the overall quality of the
code and minimize the number of CodeQL alerts.


Seems CodeQL now runs as part of CI and flags issues it has found.

It complains about a possible NULL pointer dereference:
https://github.com/tianocore/edk2/runs/22021016348

This is not correct, but I doubt code analysis will ever be clever
enough to figure this automatically.  So I've added an ASSERT()
explicitly saying so, which should help both human reviewers and
code analyzers.

Apparently that does not change anything for CodeQL though.  I guess
the CodeQL config must be updated so it knows what ASSERT() means?
Maybe it is ignored simply because it is upper case (unlike the
standard C library version which is lower case)?

thanks & take care,
    Gerd









-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116078): https://edk2.groups.io/g/devel/message/116078
Mute This Topic: https://groups.io/mt/102444916/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: add struct MP_HAND_OFF_CONFIG

[Laszlo Ersek]

On 2/27/24 12:41, Gerd Hoffmann wrote:
> Move the WaitLoopExecutionMode and StartupSignalValue fields to a
> separate HOB with the new struct.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 13 ++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.h |  3 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c  | 34 +
>  4 files changed, 66 insertions(+), 22 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h 
> b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> index 77854d6a81f8..ae93b7e3d7c9 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> @@ -15,7 +15,13 @@
>  0x11e2bd88, 0xed38, 0x4abd, {0xa3, 0x99, 0x21, 0xf2, 0x5f, 0xd0, 0x7a, 
> 0x60 } \
>}
>  
> +#define MP_HANDOFF_CONFIG_GUID \
> +  { \
> +0xdabbd793, 0x7b46, 0x4144, {0x8a, 0xd4, 0x10, 0x1c, 0x7c, 0x08, 0xeb, 
> 0xfa } \
> +  }
> +
>  extern EFI_GUID  mMpHandOffGuid;
> +extern EFI_GUID  mMpHandOffConfigGuid;
>  
>  //
>  // The information required to transfer from the PEI phase to the
> @@ -43,8 +49,11 @@ typedef struct {
>//
>UINT32ProcessorIndex;
>UINT32CpuCount;
> -  UINT32WaitLoopExecutionMode;
> -  UINT32StartupSignalValue;
>PROCESSOR_HAND_OFFInfo[];
>  } MP_HAND_OFF;
> +
> +typedef struct {
> +  UINT32WaitLoopExecutionMode;
> +  UINT32StartupSignalValue;
> +} MP_HAND_OFF_CONFIG;
>  #endif
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 3a7b9896cff4..d26035559f22 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -482,7 +482,8 @@ GetWakeupBuffer (
>  **/
>  VOID
>  SwitchApContext (
> -  IN CONST MP_HAND_OFF  *FirstMpHandOff
> +  IN CONST MP_HAND_OFF_CONFIG  *MpHandOffConfig,
> +  IN CONST MP_HAND_OFF *FirstMpHandOff
>);
>  
>  /**
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 8c186211fb38..a22bcfc0bc30 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -15,6 +15,7 @@
>  
>  EFI_GUID  mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
>  EFI_GUID  mMpHandOffGuid   = MP_HANDOFF_GUID;
> +EFI_GUID  mMpHandOffConfigGuid = MP_HANDOFF_CONFIG_GUID;
>  
>  /**
>Save the volatile registers required to be restored following INIT IPI.
> @@ -1935,11 +1936,13 @@ GetBspNumber (
>This procedure allows the AP to switch to another section of
>memory and continue its loop there.
>  
> -  @param[in] FirstMpHandOff  Pointer to first MP hand-off HOB body.
> +  @param[in] MpHandOffConfig  Pointer to MP hand-off config HOB body.
> +  @param[in] FirstMpHandOff   Pointer to first MP hand-off HOB body.
>  **/
>  VOID
>  SwitchApContext (
> -  IN CONST MP_HAND_OFF  *FirstMpHandOff
> +  IN CONST MP_HAND_OFF_CONFIG  *MpHandOffConfig,
> +  IN CONST MP_HAND_OFF *FirstMpHandOff
>)
>  {
>UINTN  Index;
> @@ -1955,7 +1958,7 @@ SwitchApContext (
>  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
>if (MpHandOff->ProcessorIndex + Index != BspNumber) {
>  *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = 
> (UINTN)SwitchContextPerAp;
> -*(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = 
> MpHandOff->StartupSignalValue;
> +*(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = 
> MpHandOffConfig->StartupSignalValue;
>}
>  }
>}
> @@ -1975,6 +1978,26 @@ SwitchApContext (
>}
>  }
>  
> +/**
> +  Get pointer to MP_HAND_OFF_CONFIG GUIDed HOB body.
> +
> +  @return  The pointer to MP_HAND_OFF_CONFIG structure.
> +**/
> +MP_HAND_OFF_CONFIG *
> +GetMpHandOffConfigHob (
> +  VOID
> +  )
> +{
> +  EFI_HOB_GUID_TYPE  *GuidHob;
> +
> +  GuidHob = GetFirstGuidHob ();
> +  if (GuidHob == NULL) {
> +return NULL;
> +  }
> +
> +  return (MP_HAND_OFF_CONFIG *)GET_GUID_HOB_DATA (GuidHob);
> +}
> +
>  /**
>Get pointer to next MP_HAND_OFF GUIDed HOB body.
>  
> @@ -2022,6 +2045,7 @@ MpInitLibInitialize (
>VOID
>)
>  {
> +  MP_HAND_OFF_CONFIG   *MpHandOffConfig;
>MP_HAND_OFF  *FirstMpHandOff;
>MP_HAND_OFF  *MpHandOff;
>CPU_INFO_IN_HOB  *CpuInfoInHob;
> @@ -2239,13 +2263,15 @@ MpInitLibInitialize (
>}
>  }
>  
> +MpHandOffConfig = GetMpHandOffConfigHob ();
> +ASSERT (MpHandOffConfig != NULL);

So, in connection to the other subthread

Re: [edk2-devel] CodeQL Analysis in edk2
msgid: <80abb140-9a9c-43b8-ba0b-d8ea631d9...@linux.microsoft.com>
https://edk2.groups.io/g/devel/message/116054

I suggest replacing this with:

if (MpHandOffConfig == NULL) {
  DEBUG ((
DEBUG_ERROR,
"%a: at least one MpHandOff HOB, but no MpHandOffConfig HOB\n",
   

Re: [edk2-devel] CodeQL Analysis in edk2

[Laszlo Ersek]

On 2/27/24 17:04, Michael Kubacki wrote:
> Hi Gerd,
> 
> There is a way to suppress results explained here:
> https://github.com/tianocore/edk2/tree/master/BaseTools/Plugin/CodeQL#filter-patterns
> 
> A real-world example is here:
> https://github.com/microsoft/mu_basecore/blob/release/202311/CodeQlFilters.yml
> 
> That can currently operate at the file and CodeQL rule level
> granularity. In this case, the null pointer test rule
> ("cpp/missing-null-test" as shown in
> https://github.com/tianocore/edk2/security/code-scanning/1277) could be
> excluded in MpLib.c.
> 
> ---
> 
> Taking a quick look at the code highlighted:
> 
>     MpHandOffConfig = GetMpHandOffConfigHob ();
>     ASSERT (MpHandOffConfig != NULL);
>     DEBUG ((
>   DEBUG_INFO,
>   "FirstMpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *):
> %04d\n",
>   MpHandOffConfig->WaitLoopExecutionMode,
>   sizeof (VOID *)
>   ));
>     if (MpHandOffConfig->WaitLoopExecutionMode == sizeof (VOID *)) {
> 
> CodeQL flagged the two MpHandOffConfig dereferences. These is assigned
> on the return value from GetMpHandOffConfigHob () defined as:
> 
> /**
>   Get pointer to MP_HAND_OFF_CONFIG GUIDed HOB body.
> 
>   @return  The pointer to MP_HAND_OFF_CONFIG structure.
> **/
> MP_HAND_OFF_CONFIG *
> GetMpHandOffConfigHob (
>   VOID
>   )
> {
>   EFI_HOB_GUID_TYPE  *GuidHob;
> 
>   GuidHob = GetFirstGuidHob ();
>   if (GuidHob == NULL) {
>     return NULL;
>   }
> 
>   return (MP_HAND_OFF_CONFIG *)GET_GUID_HOB_DATA (GuidHob);
> }
> 
> Can you please provide more context about why you believe a NULL return
> value from the function is not a consideration? Generally, anything is
> possible in the HOB list, for example, other code could overflow a HOB
> boundary and mutate the this HOB's header preventing it from being found.

The GetMpHandOffConfigHob() function itself is right, when viewed in
isolation, to deal with the possibility of the HOB missing.

However, the GetMpHandOffConfigHob() *call site* is only reached if the
"FirstMpHandOff" variable is not NULL.

And if "FirstMpHandOff" is not NULL, then in the PEI phase, the
SaveCpuMpData() function [UefiCpuPkg/Library/MpInitLib/PeiMpLib.c] will
have prepared *both* at least one MpHandOff GUID HOB, *and* the sole
MpHandOffConfig GUID HOB.

In other words, the presence of at least one MpHandOff GUID HOB
guarantees that there is going to be exactly one MpHandOffConfig GUID HOB.

Therefore the ASSERT() is right -- it expresses an invariant.

That said:

> 
> ASSERT() is useful for debug but it's control path is unpredictable in
> core code based on platform policies that often have varying
> perspectives of how to apply asserts and how they should be configured
> in different scenarios. We introduced a "panic library" to use in rare
> cases where we want more consistent behavior for fatal conditions.

I agree that making this more explicit wouldn't hurt.

The panic library is not upstream (yet? :)), so I'll suggest a
CpuDeadLoop() under the patch itself:

[PATCH 1/1] UefiCpuPkg/MpInitLib: add struct MP_HAND_OFF_CONFIG
msgid <20240227114122.1140614-1-kra...@redhat.com>
https://edk2.groups.io/g/devel/message/116029

Thanks,
Laszlo

> 
> Thanks,
> Michael
> 
> On 2/27/2024 6:39 AM, Gerd Hoffmann wrote:
>>    Hi,
>>
>>> I am hoping we can work together to improve the overall quality of the
>>> code and minimize the number of CodeQL alerts.
>>
>> Seems CodeQL now runs as part of CI and flags issues it has found.
>>
>> It complains about a possible NULL pointer dereference:
>> https://github.com/tianocore/edk2/runs/22021016348
>>
>> This is not correct, but I doubt code analysis will ever be clever
>> enough to figure this automatically.  So I've added an ASSERT()
>> explicitly saying so, which should help both human reviewers and
>> code analyzers.
>>
>> Apparently that does not change anything for CodeQL though.  I guess
>> the CodeQL config must be updated so it knows what ASSERT() means?
>> Maybe it is ignored simply because it is upper case (unlike the
>> standard C library version which is lower case)?
>>
>> thanks & take care,
>>    Gerd
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116076): https://edk2.groups.io/g/devel/message/116076
Mute This Topic: https://groups.io/mt/102444916/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 4/4] StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler

[Zhiguang Liu]

In last patch, we add code support to unregister MMI handler inside
itself. However, the code doesn't support unregister MMI handler
insider other MMI handler. While this is not a must-have usage.
So add check to disallow unregister MMI handler in other MMI handler.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Ray Ni 
Signed-off-by: Zhiguang Liu 
---
 StandaloneMmPkg/Core/Mmi.c | 35 ++-
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
index c1a1d76e85..54794c6b3d 100644
--- a/StandaloneMmPkg/Core/Mmi.c
+++ b/StandaloneMmPkg/Core/Mmi.c
@@ -36,8 +36,9 @@ typedef struct {
   MMI_ENTRY *MmiEntry;
 } MMI_HANDLER;
 
-LIST_ENTRY  mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE 
(mRootMmiHandlerList);
-LIST_ENTRY  mMmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
(mMmiEntryList);
+LIST_ENTRY   mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE 
(mRootMmiHandlerList);
+LIST_ENTRY   mMmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
(mMmiEntryList);
+MMI_HANDLER  *gCurrentMmiHandler = NULL;
 
 /**
   Finds the MMI entry for the requested handler type.
@@ -161,13 +162,19 @@ MmiManage (
 // get next node before handler is executed, since LIST_ENTRY that
 // Link points to may be freed if unregister MMI handler.
 //
-Link   = Link->ForwardLink;
-Status = MmiHandler->Handler (
-   (EFI_HANDLE)MmiHandler,
-   Context,
-   CommBuffer,
-   CommBufferSize
-   );
+Link = Link->ForwardLink;
+//
+// Assign gCurrentMmiHandle before calling the MMI handler and
+// set to NULL when it returns.
+//
+gCurrentMmiHandler = MmiHandler;
+Status = MmiHandler->Handler (
+   (EFI_HANDLE)MmiHandler,
+   Context,
+   CommBuffer,
+   CommBufferSize
+   );
+gCurrentMmiHandler = NULL;
 
 switch (Status) {
   case EFI_INTERRUPT_PENDING:
@@ -314,6 +321,16 @@ MmiHandlerUnRegister (
 return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // Check if unregister MMI handler inside a MMI Handler
+  //
+  if (gCurrentMmiHandler != NULL) {
+//
+// Only allow to unregister MMI Handler inside itself.
+//
+ASSERT (gCurrentMmiHandler == MmiHandler);
+  }
+
   MmiEntry = MmiHandler->MmiEntry;
 
   RemoveEntryList (>Link);
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116075): https://edk2.groups.io/g/devel/message/116075
Mute This Topic: https://groups.io/mt/104616995/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 3/4] StandaloneMmPkg: Support to unregister MMI handler inside MMI handler

[Zhiguang Liu]

To support unregister MMI handler inside MMI handler itself,
get next node before MMI handler is executed, since LIST_ENTRY that
Link points to may be freed if unregister MMI handler in MMI handler
itself.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Signed-off-by: Zhiguang Liu 
---
 StandaloneMmPkg/Core/Mmi.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
index 0de6fd17fc..c1a1d76e85 100644
--- a/StandaloneMmPkg/Core/Mmi.c
+++ b/StandaloneMmPkg/Core/Mmi.c
@@ -154,9 +154,14 @@ MmiManage (
 Head = >MmiHandlers;
   }
 
-  for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
+  for (Link = Head->ForwardLink; Link != Head;) {
 MmiHandler = CR (Link, MMI_HANDLER, Link, MMI_HANDLER_SIGNATURE);
-
+//
+// To support unregister MMI handler inside MMI handler itself,
+// get next node before handler is executed, since LIST_ENTRY that
+// Link points to may be freed if unregister MMI handler.
+//
+Link   = Link->ForwardLink;
 Status = MmiHandler->Handler (
(EFI_HANDLE)MmiHandler,
Context,
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116074): https://edk2.groups.io/g/devel/message/116074
Mute This Topic: https://groups.io/mt/104616994/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 2/4] MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler

[Zhiguang Liu]

In last patch, we add code support to unregister SMI handler inside
itself. However, the code doesn't support unregister SMI handler
insider other SMI handler. While this is not a must-have usage.
So add check to disallow unregister SMI handler in other SMI handler.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Signed-off-by: Zhiguang Liu 
---
 MdeModulePkg/Core/PiSmmCore/Smi.c | 32 +++
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c 
b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 3489c130fd..1bfbc635fc 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -8,7 +8,8 @@
 
 #include "PiSmmCore.h"
 
-LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
+SMI_HANDLER  *gCurrentSmiHandler = NULL;
+LIST_ENTRY   mSmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
(mSmiEntryList);
 
 SMI_ENTRY  mRootSmiEntry = {
   SMI_ENTRY_SIGNATURE,
@@ -142,13 +143,18 @@ SmiManage (
 // Link points to may be freed if unregister SMI handler.
 //
 Link = Link->ForwardLink;
-
-Status = SmiHandler->Handler (
-   (EFI_HANDLE)SmiHandler,
-   Context,
-   CommBuffer,
-   CommBufferSize
-   );
+//
+// Assign gCurrentSmiHandle before calling the SMI handler and
+// set to NULL when it returns.
+//
+gCurrentSmiHandler = SmiHandler;
+Status = SmiHandler->Handler (
+   (EFI_HANDLE)SmiHandler,
+   Context,
+   CommBuffer,
+   CommBufferSize
+   );
+gCurrentSmiHandler = NULL;
 
 switch (Status) {
   case EFI_INTERRUPT_PENDING:
@@ -328,6 +334,16 @@ SmiHandlerUnRegister (
 return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // Check if unregister SMI handler inside a SMI Handler
+  //
+  if (gCurrentSmiHandler != NULL) {
+//
+// Only allow to unregister SMI Handler inside itself.
+//
+ASSERT (gCurrentSmiHandler == SmiHandler);
+  }
+
   SmiEntry = SmiHandler->SmiEntry;
 
   RemoveEntryList (>Link);
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116073): https://edk2.groups.io/g/devel/message/116073
Mute This Topic: https://groups.io/mt/104616993/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 0/4] Support to unregister SMI handler inside SMI handler

[Zhiguang Liu]

This patch set is to support to unregister SMI handler inside SMI handler,
also add check to not allow unregister SMI handler in other SMI handler.
This patch set also have the same logic in StandaloneMmPkg.
Because no change on the first patch, I kept the R-B for it.

Zhiguang Liu (4):
  MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler
  MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler
  StandaloneMmPkg: Support to unregister MMI handler inside MMI handler
  StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler

 MdeModulePkg/Core/PiSmmCore/Smi.c | 40 ++---
 StandaloneMmPkg/Core/Mmi.c| 42 +++
 2 files changed, 63 insertions(+), 19 deletions(-)

-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116071): https://edk2.groups.io/g/devel/message/116071
Mute This Topic: https://groups.io/mt/104616991/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/4] MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler

[Zhiguang Liu]

To support unregister SMI handler inside SMI handler itself,
get next node before SMI handler is executed, since LIST_ENTRY that
Link points to may be freed if unregister SMI handler in SMI handler
itself.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Reviewed-by: Ray Ni 
Reviewed-by: Laszlo Ersek 
Signed-off-by: Zhiguang Liu 
---
 MdeModulePkg/Core/PiSmmCore/Smi.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c 
b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 2985f989c3..3489c130fd 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -134,8 +134,14 @@ SmiManage (
 
   Head = >SmiHandlers;
 
-  for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
+  for (Link = Head->ForwardLink; Link != Head;) {
 SmiHandler = CR (Link, SMI_HANDLER, Link, SMI_HANDLER_SIGNATURE);
+//
+// To support unregister SMI handler inside SMI handler itself,
+// get next node before handler is executed, since LIST_ENTRY that
+// Link points to may be freed if unregister SMI handler.
+//
+Link = Link->ForwardLink;
 
 Status = SmiHandler->Handler (
(EFI_HANDLE)SmiHandler,
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116072): https://edk2.groups.io/g/devel/message/116072
Mute This Topic: https://groups.io/mt/104616992/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/ConverterLib: check JSON value type

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Hi Nickle,
As this change is in the autogenerate source files. We have to update the 
script here: https://github.com/DMTF/Redfish-Schema-C-Struct-Generator
Not sure if you have access to this repo, however I am pleased inviting you to 
update the script on DMTF Github.
We will not merging this change on edk2-redfish-client repo.

Thanks
Abner


> -Original Message-
> From: Nickle Wang 
> Sent: Tuesday, February 27, 2024 8:53 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> ; Nick Ramirez 
> Subject: [edk2-redfish-client][PATCH] RedfishClientPkg/ConverterLib: check
> JSON value type
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Check and see if JSON object type is desired type before
> getting its value. According to the Redfish schema, attribute
> value can be null value. Add this error handling to avoid
> system assertion while getting attribute value.
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> Cc: Nick Ramirez 
> ---
>  RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c
> b/RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c
> index 250ef75e4..7ee3c86e1 100644
> --- a/RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c
> +++ b/RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c
> @@ -1,5 +1,7 @@
>  /** @file
>
> +  Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +
>Copyright Notice:
>Copyright 2019-2024 Distributed Management Task Force, Inc. All rights
> reserved.
>License: BSD 3-Clause License. For full text see link:
> https://github.com/DMTF/Redfish-JSON-C-Struct-
> Converter/blob/master/LICENSE.md
> @@ -829,6 +831,10 @@ GetRedfishPropertyStr (
>  return RedfishCS_status_not_found;
>}
>
> +  if (!json_is_string (TempJsonObj)) {
> +return RedfishCS_status_not_found;
> +  }
> +
>Status = allocateDuplicateStr (Cs, (char *)json_string_value (TempJsonObj),
> (void **)DstBuffer);
>return Status;
>  }
> @@ -914,6 +920,10 @@ GetRedfishPropertyInt64 (
>  return Status;
>}
>
> +  if (!json_is_integer (TempJsonObj)) {
> +return RedfishCS_status_not_found;
> +  }
> +
>**Dst = (RedfishCS_int64)json_integer_value (TempJsonObj);
>return RedfishCS_status_success;
>  }
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116070): https://edk2.groups.io/g/devel/message/116070
Mute This Topic: https://groups.io/mt/104601599/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Tagging and releases of edk2-basetools

[Rebecca Cran]

edk2-basetools is finally fixed and releases can once again be published 
to PyPI.


However, in moving from setup.py to pyproject.toml the process has 
changed, and Joey suggested the old way might have been chosen 
deliberately to be different from edk2-pytool-library and 
edk2-pytool-extensions.



Previously it fetched the list of releases from PyPI and incremented the 
version number before publishing a new version, while it now depends on 
the git repo being tagged.



Should I work on migrating the old code and figuring out how to make it 
work with pyproject.toml?



--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116069): https://edk2.groups.io/g/devel/message/116069
Mute This Topic: https://groups.io/mt/104615885/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 2/2] ShellPkg/SmbiosView: Support New ProcessorFamily for SMBIOS Type4

[Gao, Zhichao]

Reviewed-by: Zhichao Gao 

> -Original Message-
> From: gaoliming 
> Sent: Monday, February 26, 2024 9:33 PM
> To: Lou, Yun ; devel@edk2.groups.io
> Cc: Gao, Zhichao 
> Subject: 回复: [PATCH v4 2/2] ShellPkg/SmbiosView: Support New
> ProcessorFamily for SMBIOS Type4
> 
> Reviewed-by: Liming Gao 
> 
> > -邮件原件-
> > 发件人: Jason 
> > 发送时间: 2024年2月25日 17:06
> > 收件人: devel@edk2.groups.io
> > 抄送: Jason Lou ; Liming Gao
> > ; Zhichao Gao 
> > 主题: [PATCH v4 2/2] ShellPkg/SmbiosView: Support New ProcessorFamily
> > for SMBIOS Type4
> >
> > From: Jason Lou 
> >
> > The patch updates SmbiosView to support new ProcessorFamily for
> SMBIOS
> > Type4 based on SMBIOS 3.8.0.
> >
> > Signed-off-by: Jason Lou 
> > Cc: Liming Gao 
> > Cc: Zhichao Gao 
> > ---
> >  ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c |
> > 34 +++-
> >  1 file changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > index 4f29eedc58..f96317b314 100644
> > ---
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > +++
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >
> >Module for clarifying the content of the smbios structure element
> > information.
> >
> >
> >
> > -  Copyright (c) 2005 - 2018, Intel Corporation. All rights
> > reserved.
> >
> > +  Copyright (c) 2005 - 2024, Intel Corporation. All rights
> > + reserved.
> >
> >Copyright (c) 1985 - 2022, American Megatrends International
> > LLC.
> >
> >(C) Copyright 2014 Hewlett-Packard Development Company, L.P.
> >
> >(C) Copyright 2015-2019 Hewlett Packard Enterprise Development
> > LP
> >
> > @@ -2678,6 +2678,38 @@ DisplayProcessorFamily2 (
> >Print (L"MultiCoreLoongson3D\n");
> >
> >break;
> >
> >
> >
> > +case 0x300:
> >
> > +  Print (L"IntelCore3\n");
> >
> > +  break;
> >
> > +
> >
> > +case 0x301:
> >
> > +  Print (L"IntelCore5\n");
> >
> > +  break;
> >
> > +
> >
> > +case 0x302:
> >
> > +  Print (L"IntelCore7\n");
> >
> > +  break;
> >
> > +
> >
> > +case 0x303:
> >
> > +  Print (L"IntelCore9\n");
> >
> > +  break;
> >
> > +
> >
> > +case 0x304:
> >
> > +  Print (L"IntelCoreUltra3\n");
> >
> > +  break;
> >
> > +
> >
> > +case 0x305:
> >
> > +  Print (L"IntelCoreUltra5\n");
> >
> > +  break;
> >
> > +
> >
> > +case 0x306:
> >
> > +  Print (L"IntelCoreUltra7\n");
> >
> > +  break;
> >
> > +
> >
> > +case 0x307:
> >
> > +  Print (L"IntelCoreUltra9\n");
> >
> > +  break;
> >
> > +
> >
> >  default:
> >
> >ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_SMBIOSVIEW_PRINTINFO_UNDEFINED_PROC_FAMILY),
> > gShellDebug1HiiHandle);
> >
> >}
> >
> > --
> > 2.39.1.windows.1
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116068): https://edk2.groups.io/g/devel/message/116068
Mute This Topic: https://groups.io/mt/104615867/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/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names

[Michael D Kinney]

Thanks.  Not sure how I missed that.

I will refresh the series/PR and make sure there are no issues.

Mike

> -Original Message-
> From: Pedro Falcato 
> Sent: Tuesday, February 27, 2024 3:56 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> 
> Cc: Gao, Liming ; Liu, Zhiguang
> ; Oliver Smith-Denny
> ; Pop, Aaron 
> Subject: Re: [edk2-devel] [Patch v2 1/3]
> MdePkg/Include/IndustryStandard: Add Operator and Xor field names
> 
> On Tue, Feb 27, 2024 at 11:46 PM Michael D Kinney
>  wrote:
> >
> > Hi Pedro,
> >
> > Looks like this series got set aside waiting for some additional
> feedback.
> 
> Hi,
> 
> This is odd, I did reply back in June
> (https://edk2.groups.io/g/devel/message/105817). You probably missed
> the email or maybe for some unknown reason it never reached you.
> 
> --
> Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116067): https://edk2.groups.io/g/devel/message/116067
Mute This Topic: https://groups.io/mt/99226543/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2 Patch 1/1] BaseTools: Syntax warning invalid escape sequence \C

[Rebecca Cran]

Merged as 3e91e421365027ee3e655feab33c67a4f544c777.


--
Rebecca Cran


On 2/6/24 00:02, Jayaprakash N wrote:

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4666

This commit fixes the issue reported through BZ4666.
The Syntax warning related to invalid escape sequence
for \C is seen on Windows OS based builds of edk2 sources.
On Windows the path seperator needs to prefixed with \
so essentially we need to use \\ as path seperator.

Cc: Rebecca Cran 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Jayaprakash N 
Signed-off-by: Jayaprakash N 
---
  BaseTools/Source/Python/Workspace/DscBuildData.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 4768099343..b69d406249 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -2949,7 +2949,7 @@ class DscBuildData(PlatformBuildClassObject):
  for include_file in IncFileList:
  MakeApp += "$(OBJECTS) : %s\n" % include_file
  if sys.platform == "win32":
-PcdValueCommonPath = 
os.path.normpath(mws.join(GlobalData.gGlobalDefines["EDK_TOOLS_PATH"], 
"Source\C\Common\PcdValueCommon.c"))
+PcdValueCommonPath = 
os.path.normpath(mws.join(GlobalData.gGlobalDefines["EDK_TOOLS_PATH"], 
"Source\\C\\Common\\PcdValueCommon.c"))
  MakeApp = MakeApp + '%s\\PcdValueCommon.c : %s\n' % 
(self.OutputPath, PcdValueCommonPath)
  MakeApp = MakeApp + '\tcopy /y %s $@\n' % (PcdValueCommonPath)
  else:



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116066): https://edk2.groups.io/g/devel/message/116066
Mute This Topic: https://groups.io/mt/104193926/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/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names

[Pedro Falcato]

On Tue, Feb 27, 2024 at 11:46 PM Michael D Kinney
 wrote:
>
> Hi Pedro,
>
> Looks like this series got set aside waiting for some additional feedback.

Hi,

This is odd, I did reply back in June
(https://edk2.groups.io/g/devel/message/105817). You probably missed
the email or maybe for some unknown reason it never reached you.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116065): https://edk2.groups.io/g/devel/message/116065
Mute This Topic: https://groups.io/mt/99226543/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/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names

[Michael D Kinney]

Hi Pedro,

Looks like this series got set aside waiting for some additional feedback.

I would like to see this C++ Keyword issue resolved.

Thanks,

Mike

> -Original Message-
> From: Kinney, Michael D 
> Sent: Tuesday, June 6, 2023 10:57 AM
> To: Pedro Falcato 
> Cc: devel@edk2.groups.io; Gao, Liming ; Liu,
> Zhiguang ; Oliver Smith-Denny
> ; Pop, Aaron ;
> Kinney, Michael D 
> Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> Operator and Xor field names
> 
> Hi Pedro,
> 
> Based on this info, would you prefer we use xor_ style instead of Xor?
> 
> I can update the PR with that change.
> 
> Mike
> 
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Thursday, June 1, 2023 8:02 AM
> > To: Pedro Falcato 
> > Cc: devel@edk2.groups.io; Gao, Liming ;
> Liu,
> > Zhiguang ; Oliver Smith-Denny
> > ; Pop, Aaron ;
> Kinney,
> > Michael D 
> > Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> Operator
> > and Xor field names
> >
> > Hi Pedro,
> >
> > It appears other projects have run into this same issue with the
> > TPM specifications and have changed the field names by appending '_'.
> >
> > https://github.com/MIPS/external-
> >
> tpm2/blob/d704926273cf17498c95ff0dc50b4b17e523c109/generator/structure_
> gene
> > rator.py#L1183
> >
> > Mike
> >
> > > -Original Message-
> > > From: Kinney, Michael D 
> > > Sent: Wednesday, May 31, 2023 11:42 AM
> > > To: Pedro Falcato 
> > > Cc: devel@edk2.groups.io; Gao, Liming ;
> Liu,
> > > Zhiguang ; Oliver Smith-Denny
> > > ; Pop, Aaron ;
> Kinney,
> > > Michael D 
> > > Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> Operator
> > > and Xor field names
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Pedro Falcato 
> > > > Sent: Wednesday, May 31, 2023 11:17 AM
> > > > To: Kinney, Michael D 
> > > > Cc: devel@edk2.groups.io; Gao, Liming ;
> Liu,
> > > > Zhiguang ; Oliver Smith-Denny
> > > > ; Pop, Aaron 
> > > > Subject: Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> > Operator
> > > > and Xor field names
> > > >
> > > > On Tue, May 30, 2023 at 7:53 PM Michael D Kinney
> > > >  wrote:
> > > > >
> > > > > Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> > > > > operator and xor in C structures to support use of these
> > > > > include files when building with a C++ compiler.
> > > > >
> > > > > This patch temporarily introduces an anonymous union to add
> > > > > Operator and Xor fields to support migration from the current
> > > > > field names to the new field names.
> > > > >
> > > > > Warning 4201 is disabled for VS20xx tool chains is a temporary
> > > > > change to allow the use of anonymous unions.
> > > > >
> > > > > Cc: Liming Gao 
> > > > > Cc: Zhiguang Liu 
> > > > > Cc: Oliver Smith-Denny 
> > > > > Cc: Pedro Falcato 
> > > > > Cc: Aaron Pop 
> > > > > Signed-off-by: Michael D Kinney 
> > > > > ---
> > > > >  MdePkg/Include/IndustryStandard/Tpm12.h | 22
> --
> > > > >  MdePkg/Include/IndustryStandard/Tpm20.h | 25
> > +++--
> > > > >  2 files changed, 43 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > b/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > > index 155dcc9f5f99..56e89d9d0835 100644
> > > > > --- a/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > > +++ b/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > > @@ -9,6 +9,14 @@
> > > > >  #ifndef _TPM12_H_
> > > > >  #define _TPM12_H_
> > > > >
> > > > > +///
> > > > > +/// Temporary disable 4201 to support anonymous unions
> > > > > +///
> > > > > +#if defined (_MSC_EXTENSIONS)
> > > > > +#pragma warning( push )
> > > > > +#pragma warning ( disable : 4201 )
> > > > > +#endif
> > > > > +
> > > > >  ///
> > > > >  /// The start of TPM return codes
> > > > >  ///
> > > > > @@ -744,8 +752,11 @@ typedef struct tdTPM_PERMANENT_FLAGS {
> > > > >BOOLEAN  TPMpost;
> > > > >BOOLEAN  TPMpostLock;
> > > > >BOOLEAN  FIPS;
> > > > > -  BOOLEAN   operator;
> > > > > -  BOOLEAN   enableRevokeEK;
> > > > > +  union {
> > > > > +BOOLEANoperator;
> > > > > +BOOLEANOperator;
> > > > > +  };
> > > >
> > > > Do you know if this works cleanly for the other toolchains? i.e
> > > > supported GCCs and CLANGs?
> > > > I don't *think* there's a warning for anonymous unions beyond
> passing
> > > > -pedantic + -std=c, but it'd be good to know.
> > > > If so, we may need a pragma for this.
> > >
> > > I did not see any issues with my local testing or with EDK II CI.
> > >
> > > >
> > > > > +  BOOLEAN  enableRevokeEK;
> > > > >BOOLEAN  nvLocked;
> > > > >BOOLEAN  readSRKPub;
> > > > >BOOLEAN  tpmEstablished;
> > > > > @@ -2162,4 +2173,11 @@ typedef struct tdTPM_RSP_COMMAND_HDR {
> > > > >
> > > > >  #pragma pack ()
> > > > >
> > > 

Re: [edk2-devel] [v2] BaseTools/AutoGen: declare ProcessLibraryConstructorList() for SEC modules

[Rebecca Cran]

edk2-basetools v0.1.50 has just been published so we're back up and running.

I'll work my way through the backlog of BaseTools changes in the next 
few days.



--
Rebecca Cran


On 2/24/24 13:59, Laszlo Ersek wrote:

v1 posting:

   https://edk2.groups.io/g/devel/message/115193
   msgid <36593e23-d3e8-b71a-808d-ef94260b5...@redhat.com>

Bugzilla:

   https://bugzilla.tianocore.org/show_bug.cgi?id=991

In version 2, the feature is structured differently. Following Mike's
advice, for compatibility, the ProcessLibraryConstructorList()
declaration in AutoGen.h is now gated on the SEC module having
INF_VERSION >= 1.30.

Accordingly,

- I now update the Build specification and the Inf specification (see
   patch sets posted in response to this email),

- edk2 only receives a single patch (for AutoGen), for the time being,

- the same edk2 patch is being ported to edk2-basetools:
   https://github.com/tianocore/edk2-basetools/pull/120.

Next steps: once all of the above is merged, *and* an edk2-basetools
release has been tagged and published, I'll rework the C code patches
for edk2 and edk2-platforms, from the v1 patch sets, as follows:

- all those SEC modules will have to see their INF_VERSIONs bumped to
   1.30, for triggering the new code generation,

- pip-requirements.txt/edk2-basetools will need to reference the new
   edk2-basetools release, for exposing the feature in the first place.

Laszlo




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116063): https://edk2.groups.io/g/devel/message/116063
Mute This Topic: https://groups.io/mt/104553597/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel][PATCH v1 0/3] MdeModulePkg: ImagePropertiesRecordLib Fixes

[Oliver Smith-Denny]

ImagePropertiesRecordLib is currently creating Image Records that
are not accurate. It is setting the CodeSegmentSize to be the size
of the raw data in the image file, however, when the image is
loaded into memory, the raw data size is aligned to the
section alignment. This caused the memory attributes table to
have incorrect entries for systems, like ARM64, where the section
alignment is not 4k for all modules.

In fixing this, I noticed that MemoryProtection.c is using its own
version of image record creation where this logic was actually
correct. ImagePropertiesRecordLib was created to consolidate the
logic around creating and managing image records, so this patchset
also updates MemoryProtection.c to use ImagePropertiesRecordsLib
after making a few small adjustments to ensure the same functionality
is present.

This patchset was tested on ArmVirtQemu to ensure that all image
records were the same before and after this, other than fixing
the CodeSegmentSize.

Github PR: https://github.com/tianocore/edk2/pull/5402

Cc: Liming Gao 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Taylor Beebe 

Oliver Smith-Denny (3):
  MdeModulePkg: ImagePropertiesRecordLib: Use SectionAlignment for
CodeSize
  MdeModulePkg: ImagePropertiesRecordLib: Consolidate Usage
  MdeModulePkg: MemoryProtection: Use ImageRecordPropertiesLib

 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c| 241 
+++-
 MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c |  84 
+--
 2 files changed, 92 insertions(+), 233 deletions(-)

-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116059): https://edk2.groups.io/g/devel/message/116059
Mute This Topic: https://groups.io/mt/104610769/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch 0/4] PatchCheck Updates

[Rebecca Cran]

Merged as 
dae8c29dab546fad2801e70967855a9f6ae14ae0...6d571c0070161b1b96049410f8584c0955d73536


--
Rebecca Cran

On 2/18/2024 1:59 PM, Michael D Kinney via groups.io wrote:

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4679
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4680
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4693
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4694

Fix a collection of PatchCheck issues and add a new PatchCheck
featue to check for a commit that modifies files in multiple
packages with option to disable the multiple package check as
a command line option or a commit message tag.

* Reject patches that match Author email "devel@edk2.groups.io"
* Update the current check for " via Groups.Io" to perform a
   case insensitive match. It appears that groups.io has changed the
   format of this string to use all lower case.
* Update logic in CommitMessageCheck class to return errors
   detected in commit message signatures instead of silently
   passing the check.
* Genenerate an error if no Cc tags are present to make sure
   all patches Cc the required maintainers/reviewers.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Michael Kubacki 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 

Michael D Kinney (4):
   BaseTools/Scripts/PatchCheck: Update Author checks
   BaseTools/Scripts/PatchCheck: Return CommitMessageCheck errors
   BaseTools/Scripts/PatchCheck: Error if no Cc tags are present
   BaseTools/Scripts/PatchCheck: Error if commit modifies multiple
 packages

  BaseTools/Scripts/PatchCheck.py | 91 +++--
  1 file changed, 86 insertions(+), 5 deletions(-)





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116058): https://edk2.groups.io/g/devel/message/116058
Mute This Topic: https://groups.io/mt/104434581/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch 1/4] BaseTools/Scripts/PatchCheck: Update Author checks

[Rebecca Cran]

For the series:

Reviewed-by: Rebecca Cran 

--
Rebecca Cran


On 2/18/2024 1:59 PM, Michael D Kinney wrote:

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4680

* Reject patches that match Author email "devel@edk2.groups.io"
* Update the current check for " via Groups.Io" to perform a
   case insensitive match. It appears that groups.io has changed the
   format of this string to use all lower case.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Signed-off-by: Michael D Kinney 
Reviewed-by: Rebecca Cran 
Reviewed-by: Ard Biesheuvel 
---
  BaseTools/Scripts/PatchCheck.py | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 1675dcbd7321..e600e0be440f 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -85,7 +85,11 @@ class EmailAddressCheck:
  self.error("The email address cannot contain a space: " +
 mo.group(3))
  
-if ' via Groups.Io' in name and mo.group(3).endswith('@groups.io'):

+if mo.group(3) == 'devel@edk2.groups.io':
+self.error("Email rewritten by lists DMARC / DKIM / SPF: " +
+   email)
+
+if ' via groups.io' in name.lower() and 
mo.group(3).endswith('@groups.io'):
  self.error("Email rewritten by lists DMARC / DKIM / SPF: " +
 email)
  





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116057): https://edk2.groups.io/g/devel/message/116057
Mute This Topic: https://groups.io/mt/104434582/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/Scripts/LauterbachT32: Fix EfiLoadDxe.cmm

[Rebecca Cran]

Merged as e59a40b92ce92f17e3d8d99917868c5678d408b7.

--
Rebecca Cran


On 2/12/2024 10:41 AM, Leif Lindholm via groups.io wrote:

On Sun, Feb 11, 2024 at 16:10:32 -0700, Rebecca Cran wrote:

I'm guessing there haven't been any reviews because so few people are
familiar with the PRACTICE scripting language.

If there's nobody who can review it, could I get approval to commit it
anyway?

I'm good with that (after stable tag, of course):
Reviewed-by: Leif Lindholm 

It would make sense to also add you as a maintainer for
EmbeddedPkg/Scripts/LauterbachT32/

Regards,

Leif



--

Rebecca Cran


On 1/29/24 12:01, Rebecca Cran via groups.io wrote:

There have been many changes since EfiLoadDxe.cmm was last updated in
2011. The EFI_SYSTEM_TABLE can no longer be found by scanning memory on
4KB boundaries, so require users pass in its address instead. Update
various offsets so that the debug information can be found and loaded
with a recent version of TRACE32.

Signed-off-by: Rebecca Cran 
---
   EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm| 77 
+++-
   EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm |  7 +-
   EmbeddedPkg/Scripts/LauterbachT32/Readme.md |  8 +-
   3 files changed, 19 insertions(+), 73 deletions(-)

diff --git a/EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm 
b/EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm
index 1605a9b478df..06d307e12d8c 100644
--- a/EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm
+++ b/EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm
@@ -1,79 +1,22 @@
   ;
+; Copyright (c) 2024, Ampere Computing LLC. All rights reserved.
   ; Copyright (c) 2011, Hewlett-Packard Company. All rights reserved.
   ;
   ; SPDX-License-Identifier: BSD-2-Clause-Patent
   ;
-  LOCAL   
-
-  =0x2000   ; default to 512MB
-
-  gosub FindSystemTable 
-  ENTRY 
-
-  if !=0
-  (
-print "found system table at "
-gosub FindDebugInfo 
-  )
-  else
-  (
-print "ERROR: system table not found, check memory size"
-  )
+  PARAMETERS 
+
+  gosub FindDebugInfo 
 enddo
-FindSystemTable:
-  LOCAL
-  ENTRY   
-
-  print "FindSystemTable"
-  print "top of mem is $"
-
-  =
-
-  ; align to highest 4MB boundary
-  =&0xFFC0
-
-  ; start at top and look on 4MB boundaries for system table ptr structure
-  while >0
-  (
-; low signature match
-if Data.Long(a:)==0x20494249
-(
-  ; high signature match
-  if Data.Long(a:+4)==0x54535953
-  (
-; less than 4GB?
-if Data.Long(a:+0x0c)==0
-(
-  ; less than top of ram?
-  if Data.Long(a:+8)<
-  (
-return Data.Long(a:+8)
-  )
-)
-  )
-)
-
-if <0x40
-(
-  return 0
-)
-=
-  )
-
-  return 0
-
-
   FindDebugInfo:
 LOCAL  
  
 ENTRY   
-  print "FindDebugInfo"
-
 =0
-  =Data.Long(a:+0x40)
-  =Data.Long(a:+0x44)
+  =Data.Long(a:+0x68)
+  =Data.Long(a:+0x70)
 print "config table is at  ( entries)"
@@ -82,7 +25,7 @@ FindDebugInfo:
 =0
 while <
 (
-=+(*0x14)
+=+(*0x18)
   if Data.Long(a:)==0x49152E77
   (
 if Data.Long(a:+4)==0x47641ADA
@@ -120,8 +63,10 @@ FindDebugInfo:
   (
 if Data.Long(a:)==1 ; normal debug info type
 (
-=Data.Long(a:+4)
-do EfiProcessPeImage Data.Long(a:+0x20)
+=Data.Long(a:+8)
+=+0x40
+=Data.Long(a:)
+do /EfiProcessPeImage.cmm ""
 )
   )
   =+1
diff --git a/EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm 
b/EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm
index c3aab9d06a47..b0d97eec71b0 100644
--- a/EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm
+++ b/EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm
@@ -1,4 +1,5 @@
   ;
+; Copyright (c) 2024, Ampere Computing LLC. All rights reserved.
   ; Copyright (c) 2011, Hewlett-Packard Company. All rights reserved.
   ;
   ; SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -10,11 +11,11 @@
 =
 print "PE32 image found at "
-  ; offset from dos hdr to PE file hdr
+  ; offset from dos hdr to PE file hdr (i.e. 'PE\0\0' signature)
 =+Data.Long(c:+0x3C)
 ; offset to debug dir in PE hdrs
-  =Data.Long(c:+0xA8)
+  =Data.Long(c:+0xf10)
 if ==0
 (
   print "no debug dir for image at "
@@ -62,7 +63,7 @@
   =
 )
-  print "found path "
+  print "found path  with address "
   ON ERROR GOSUB
 return
 data.load.elf   /NOCODE /NOCLEAR
diff --git a/EmbeddedPkg/Scripts/LauterbachT32/Readme.md 
b/EmbeddedPkg/Scripts/LauterbachT32/Readme.md
index 51d2c8da5405..c30ec20a3d96 100644
--- a/EmbeddedPkg/Scripts/LauterbachT32/Readme.md
+++ b/EmbeddedPkg/Scripts/LauterbachT32/Readme.md
@@ -1,10 +1,10 @@
   # DXE Phase Debug
-Update the memsize variable in EfiLoadDxe.cmm for the actual amount of memory
-available in your system.  Allow your system to boot to the point that the DXE
+Allow your system to boot to the 

Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver earlier in DXE

[Albecki, Mateusz]

Is the idea to refactor PciSioSerialDxe to extract functions that access the HW 
and wrap it in the SerialPortLib instance? That solution would still save us 
some maintenance cost. However exploring the idea further I see following 
problems:

1. Relying on driver binding produces a fairly nice flow: platform driver 
initializes enough platform HW for UART to work -> platform driver calls 
ConnectController -> driver initializes UART. With SerialDxe a depex would have 
to be injected through our instance of SerialPortLib to stop the SerialDxe 
dispatch until platform driver made the platform ready.
2. I am wondering how it would work in case we want to allow dynamic 
configuration of debug port(basically selecting which UART controller would be 
used). With current solution we can select which one(or which ones) will be 
used by simply installing and connecting corresponding handles. With library 
solution I guess library would have to locate some protocols(possibly the same 
PCI_IO and DEVICE_PATH) that were installed by platform driver. It would also 
need to install notify on those protocols installation in case platform wants 
to add more uarts later on in the boot flow.
3. We still end up with 2 UART drivers in flash since PciSioSerialDxe is needed 
for PCI UARTs.

I also think this solution is comparable in effort to refactoring the 
PciSioSerialDxe so that it doesn't use driver binding when used as a DXE 
driver. In this solution driver would have one .c file for code with driver 
binding and one .c file for code when everything is done in entry point, it 
would still use DEVICE_PATH and PCI_IO/SIO. While I still think using the 
driver as is in DXE is best for us, in case that solution gets blocked I would 
like to understand if everyone would be ok with such refactoring.

Thanks,
Mateusz


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116055): https://edk2.groups.io/g/devel/message/116055
Mute This Topic: https://groups.io/mt/104469297/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] CodeQL Analysis in edk2

[Michael Kubacki]

Hi Gerd,

There is a way to suppress results explained here: 
https://github.com/tianocore/edk2/tree/master/BaseTools/Plugin/CodeQL#filter-patterns


A real-world example is here: 
https://github.com/microsoft/mu_basecore/blob/release/202311/CodeQlFilters.yml


That can currently operate at the file and CodeQL rule level 
granularity. In this case, the null pointer test rule 
("cpp/missing-null-test" as shown in 
https://github.com/tianocore/edk2/security/code-scanning/1277) could be 
excluded in MpLib.c.


---

Taking a quick look at the code highlighted:

MpHandOffConfig = GetMpHandOffConfigHob ();
ASSERT (MpHandOffConfig != NULL);
DEBUG ((
  DEBUG_INFO,
  "FirstMpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *): 
%04d\n",

  MpHandOffConfig->WaitLoopExecutionMode,
  sizeof (VOID *)
  ));
if (MpHandOffConfig->WaitLoopExecutionMode == sizeof (VOID *)) {

CodeQL flagged the two MpHandOffConfig dereferences. These is assigned 
on the return value from GetMpHandOffConfigHob () defined as:


/**
  Get pointer to MP_HAND_OFF_CONFIG GUIDed HOB body.

  @return  The pointer to MP_HAND_OFF_CONFIG structure.
**/
MP_HAND_OFF_CONFIG *
GetMpHandOffConfigHob (
  VOID
  )
{
  EFI_HOB_GUID_TYPE  *GuidHob;

  GuidHob = GetFirstGuidHob ();
  if (GuidHob == NULL) {
return NULL;
  }

  return (MP_HAND_OFF_CONFIG *)GET_GUID_HOB_DATA (GuidHob);
}

Can you please provide more context about why you believe a NULL return 
value from the function is not a consideration? Generally, anything is 
possible in the HOB list, for example, other code could overflow a HOB 
boundary and mutate the this HOB's header preventing it from being found.


ASSERT() is useful for debug but it's control path is unpredictable in 
core code based on platform policies that often have varying 
perspectives of how to apply asserts and how they should be configured 
in different scenarios. We introduced a "panic library" to use in rare 
cases where we want more consistent behavior for fatal conditions.


Thanks,
Michael

On 2/27/2024 6:39 AM, Gerd Hoffmann wrote:

   Hi,


I am hoping we can work together to improve the overall quality of the
code and minimize the number of CodeQL alerts.


Seems CodeQL now runs as part of CI and flags issues it has found.

It complains about a possible NULL pointer dereference:
https://github.com/tianocore/edk2/runs/22021016348

This is not correct, but I doubt code analysis will ever be clever
enough to figure this automatically.  So I've added an ASSERT()
explicitly saying so, which should help both human reviewers and
code analyzers.

Apparently that does not change anything for CodeQL though.  I guess
the CodeQL config must be updated so it knows what ASSERT() means?
Maybe it is ignored simply because it is upper case (unlike the
standard C library version which is lower case)?

thanks & take care,
   Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116054): https://edk2.groups.io/g/devel/message/116054
Mute This Topic: https://groups.io/mt/102444916/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 2/3] DynamicTablesPkg: Adds ACPI HPET Table generator

[PierreGondois]

Hello Abdul,

From the HPET spec:
"""
For the case where there may be additional Event Timer Blocks implemented in 
the system, their base
addresses would be described in ACPI Name space.
"""
So it seems it might be good (but not necessary) to to add a description in
a SSDT/DSDT table of the object (with _HID=PNP0103).

---

Same comment about than for the other patches about adding objects to
the common Arch namespace.`


On 2/20/24 07:48, Abdul Lateef Attar wrote:

From: Abdul Lateef Attar 

Adds generic ACPI HPET table generator library.
Register/Deregister HPET table.
Update the HPET table during boot as per specification.

Cc: Sami Mujawar 
Cc: Pierre Gondois 
Signed-off-by: Abdul Lateef Attar 
---
  DynamicTablesPkg/DynamicTables.dsc.inc|   2 +
  DynamicTablesPkg/DynamicTablesPkg.ci.yaml |   3 +-
  DynamicTablesPkg/Include/AcpiTableGenerator.h |   1 +
  .../Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf  |  38 
  .../Library/Acpi/AcpiHpetLib/HpetGenerator.c  | 208 ++
  5 files changed, 251 insertions(+), 1 deletion(-)
  create mode 100644 DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
  create mode 100644 DynamicTablesPkg/Library/Acpi/AcpiHpetLib/HpetGenerator.c

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc 
b/DynamicTablesPkg/DynamicTables.dsc.inc
index 5ec9ffac06..af70785520 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -35,6 +35,7 @@
# Generators
#
DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
+  DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
  
#

# Dynamic Table Factory Dxe
@@ -42,6 +43,7 @@
DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf {
  
NULL|DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
+  NULL|DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
}
  
  [Components.ARM, Components.AARCH64]

diff --git a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml 
b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
index 1ad5540e24..cacdaa1df6 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
+++ b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
@@ -53,7 +53,8 @@
  "EmbeddedPkg/EmbeddedPkg.dec",
  "DynamicTablesPkg/DynamicTablesPkg.dec",
  "MdeModulePkg/MdeModulePkg.dec",
-"MdePkg/MdePkg.dec"
+"MdePkg/MdePkg.dec",
+"PcAtChipsetPkg/PcAtChipsetPkg.dec"
  ],
  # For host based unit tests
  "AcceptableDependencies-HOST_APPLICATION":[
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h 
b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index d0eda011c3..18b5f99f47 100644and 
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h

+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -99,6 +99,7 @@ typedef enum StdAcpiTableId {
EStdAcpiTableIdSsdtCpuTopology,   ///< SSDT Cpu Topology
EStdAcpiTableIdSsdtPciExpress,///< SSDT Pci Express 
Generator
EStdAcpiTableIdPcct,  ///< PCCT Generator
+  EStdAcpiTableIdHpet,  ///< HPET Generator
EStdAcpiTableIdMax
  } ESTD_ACPI_TABLE_ID;
  
diff --git a/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf b/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf

new file mode 100644
index 00..74a1358ffe
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
@@ -0,0 +1,38 @@
+## @file
+#  HPET Table Generator
+#
+#  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION= 1.27
+  BASE_NAME  = AcpiHpetLib
+  FILE_GUID  = 4E75F653-C356-48B3-B32C-D1B901ECF90A
+  VERSION_STRING = 1.0
+  MODULE_TYPE= DXE_DRIVER
+  LIBRARY_CLASS  = NULL|DXE_DRIVER
+  CONSTRUCTOR= AcpiHpetLibConstructor
+  DESTRUCTOR = AcpiHpetLibDestructor
+
+[Sources]
+  HpetGenerator.c
+
+[Packages]
+  DynamicTablesPkg/DynamicTablesPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec


I think the dependency could be deleted, along with:
HpetGenerator.c:19:#include 


+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  PcAtChipsetPkg/PcAtChipsetPkg.dec


(for Sami)
A dependency over the PcAtChipsetPkg is introduced here.


+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  IoLib
+  PcdLib
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
+  gPcAtChipsetPkgTokenSpaceGuid.PcdHpetBaseAddress



[snip]


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116053): https://edk2.groups.io/g/devel/message/116053
Mute This Topic: https://groups.io/mt/104463460/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 3/3] DynamicTablesPkg: Adds ACPI WSMT Table generator

[PierreGondois]

Hello Abdul,
I reviewed this patch first, so comments here may apply to other patches.

---

AcpiView (ShellPkg/Library/UefiShellAcpiViewCommandLib) doesn't seem
to be able to parse this ACPI table. Maybe a parser could also be added
along with the table generation.


On 2/20/24 07:48, Abdul Lateef Attar wrote:

From: Abdul Lateef Attar 

Adds generic ACPI WSMT table generator library.
Register/Deregister WSMT table.
Update the WSMT table during boot as per specification.

Cc: Sami Mujawar 
Cc: Pierre Gondois 
Signed-off-by: Abdul Lateef Attar 
---
  DynamicTablesPkg/DynamicTables.dsc.inc|   2 +
  DynamicTablesPkg/Include/AcpiTableGenerator.h |   1 +
  .../Include/ArchNameSpaceObjects.h|  10 +
  .../Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf  |  34 +++
  .../Library/Acpi/AcpiWsmtLib/WsmtGenerator.c  | 221 ++
  5 files changed, 268 insertions(+)
  create mode 100644 DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf
  create mode 100644 DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/WsmtGenerator.c

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc 
b/DynamicTablesPkg/DynamicTables.dsc.inc
index af70785520..5e66f5cd02 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -36,6 +36,7 @@
#
DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
+  DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf
  
#

# Dynamic Table Factory Dxe
@@ -44,6 +45,7 @@
  
NULL|DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
NULL|DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
+  NULL|DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf


The table should be available for all architectures right ?
This should not be in a [Components] section instead of a more limiting
[Components.IA32, Components.X64] section I assume.


}
  
  [Components.ARM, Components.AARCH64]

diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h 
b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index 18b5f99f47..a32ef46ecb 100644
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -100,6 +100,7 @@ typedef enum StdAcpiTableId {
EStdAcpiTableIdSsdtPciExpress,///< SSDT Pci Express 
Generator
EStdAcpiTableIdPcct,  ///< PCCT Generator
EStdAcpiTableIdHpet,  ///< HPET Generator
+  EStdAcpiTableIdWsmt,  ///< WSMT Generator
EStdAcpiTableIdMax
  } ESTD_ACPI_TABLE_ID;
  
diff --git a/DynamicTablesPkg/Include/ArchNameSpaceObjects.h b/DynamicTablesPkg/Include/ArchNameSpaceObjects.h

index b421c4cd29..ad3045dfec 100644
--- a/DynamicTablesPkg/Include/ArchNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArchNameSpaceObjects.h
@@ -39,6 +39,7 @@ typedef enum ArchObjectID {
EArchObjFadtArmBootArch,///< 11 - ARM boot arch information
EArchObjFadtHypervisorVendorId, ///< 12 - Hypervisor vendor identity 
information
EArchObjFadtMiscInfo,   ///< 13 - Legacy fields; RTC, latency, 
flush stride, etc
+  EArchObjWsmtProtectionFlags,///< 14 - WSMT protection flags
EArchObjMax
  } E_ARCH_OBJECT_ID;
  
@@ -214,4 +215,13 @@ typedef struct CmArchFadtMiscInfo {

UINT8 Century;
  } CM_ARCH_FADT_MISC_INFO;
  
+/** A structure that describes the

+protection flags for the WSMT fields information.
+
+ID: EArchObjWsmtProtectionFlags
+*/
+typedef struct CmArchWsmtProtectionFlags {
+  UINT32ProtectionFlags;
+} CM_ARCH_WSMT_PROTECTION_FLAGS;
+


There is an ongoing effort to open the package a bit more to other
architectures (https://edk2.groups.io/g/devel/message/114790).
The common Arch namespace will be created in the (to be created) staging
branch.

Maybe we could add the object to the Arm namespace for now ?
Sami, what do you think ?


  #endif
diff --git a/DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf 
b/DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf
new file mode 100644
index 00..f8683a5005
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf
@@ -0,0 +1,34 @@
+## @file
+#  WSMT Table Generator
+#
+#  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION= 1.27
+  BASE_NAME  = AcpiWsmtLib
+  FILE_GUID  = D6C34086-C914-4F8E-B56A-08329B4D1271
+  VERSION_STRING = 1.0
+  MODULE_TYPE= DXE_DRIVER
+  LIBRARY_CLASS  = NULL|DXE_DRIVER
+  CONSTRUCTOR= AcpiWsmtLibConstructor
+  DESTRUCTOR = AcpiWsmtLibDestructor
+
+[Sources]
+  WsmtGenerator.c
+
+[Packages]
+  DynamicTablesPkg/DynamicTablesPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  PcdLib
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
+  

Re: [edk2-devel] [PATCH v1 1/3] DynamicTablesPkg: Adds ACPI FADT Table generator

[PierreGondois]

Hello Abdul,

There is an ongoing effort to open the package a bit more to other
architectures (https://edk2.groups.io/g/devel/message/114790).

This patch seems to duplicate some objects from the Arm namespace:
- EArmObjPowerManagementProfileInfo
- EArmObjBootArchInfo
to the Arch namespace:
- EArchObjFadtPreferredPmProfile
- EArchObjFadtArmBootArch
and adding many objects to the common Arch namespace.

The Fadt Generator will be moved to a common folder to allow other
architectures to re-use the code. I think it might be good to put
this patch aside for now and integrate it to the (to be created)
staging branch instead,

Regards,
Pierre

On 2/20/24 07:48, Abdul Lateef Attar wrote:

From: Abdul Lateef Attar 

Adds generic ACPI FADT table generator library.
Register/Deregister FADT table.
Adds Arch namespace ids.
Update the FADT table during boot as per specification.

Cc: Sami Mujawar 
Cc: Pierre Gondois 
Signed-off-by: Abdul Lateef Attar 
---
  DynamicTablesPkg/DynamicTables.dsc.inc|  10 +-
  DynamicTablesPkg/DynamicTablesPkg.ci.yaml |   4 +-
  .../Include/ArchNameSpaceObjects.h| 217 ++
  .../Include/ConfigurationManagerObject.h  |   6 +
  .../Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf  |  42 +
  .../Library/Acpi/AcpiFadtLib/FadtGenerator.c  | 737 ++
  .../Library/Acpi/AcpiFadtLib/FadtUpdate.h |  28 +
  .../Library/Acpi/AcpiFadtLib/X64/FadtUpdate.c |  32 +
  8 files changed, 1074 insertions(+), 2 deletions(-)
  create mode 100644 DynamicTablesPkg/Include/ArchNameSpaceObjects.h
  create mode 100644 DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
  create mode 100644 DynamicTablesPkg/Library/Acpi/AcpiFadtLib/FadtGenerator.c
  create mode 100644 DynamicTablesPkg/Library/Acpi/AcpiFadtLib/FadtUpdate.h
  create mode 100644 DynamicTablesPkg/Library/Acpi/AcpiFadtLib/X64/FadtUpdate.c



[snip]


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116052): https://edk2.groups.io/g/devel/message/116052
Mute This Topic: https://groups.io/mt/104463461/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 07/23] MdePkg: Avoid hardcoded value for number of Page State Change entries

[Lendacky, Thomas via groups.io]

On 2/27/24 04:18, Gerd Hoffmann wrote:

On Thu, Feb 22, 2024 at 11:29:46AM -0600, Tom Lendacky wrote:

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

The SNP_PAGE_STATE_MAX_ENTRY is based on the number of entries that can
fit in the GHCB shared buffer. As a result, the SNP_PAGE_STATE_CHANGE_INFO
structure maps the full GHCB shared buffer based on the shared buffer size
being 2032 bytes.

Instead of using a hardcoded value for SNP_PAGE_STATE_MAX_ENTRY, use a
build calculated value. Since the SNP_PAGE_STATE_CHANGE_INFO is used as a
mapping, eliminate the hardcoded array size so that the structure can be
used based on any size buffer.

Signed-off-by: Tom Lendacky 
---
  MdePkg/Include/Register/Amd/Ghcb.h | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h 
b/MdePkg/Include/Register/Amd/Ghcb.h
index 432d67e3e223..0cdc00627472 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -197,13 +197,14 @@ typedef struct {
UINT32Reserved;
  } SNP_PAGE_STATE_HEADER;
  
-#define SNP_PAGE_STATE_MAX_ENTRY  253

-
  typedef struct {
SNP_PAGE_STATE_HEADERHeader;
-  SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
+  SNP_PAGE_STATE_ENTRY Entry[];
  } SNP_PAGE_STATE_CHANGE_INFO;


Good.


+#define SNP_PAGE_STATE_MAX_ENTRY  \
+  ((sizeof (((GHCB *)0)->SharedBuffer) - sizeof (SNP_PAGE_STATE_HEADER)) / 
sizeof (SNP_PAGE_STATE_ENTRY))


Can be dropped I think, after applying patch #6 BaseMemEncryptSevLib
does not use SNP_PAGE_STATE_MAX_ENTRY any more.


It gets used again in patch #9 for the exit optimization support.

Thanks,
Tom



take care,
   Gerd




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116050): https://edk2.groups.io/g/devel/message/116050
Mute This Topic: https://groups.io/mt/104512946/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] BaseTools/GenFds: Resolve absolute workspace INF paths

[Rebecca Cran]

Reviewed-by: Rebecca Cran 

On 2/20/2024 9:22 PM, mikub...@linux.microsoft.com wrote:

From: Michael Kubacki 

Currently, if an INF path is an absolute path on Linux (begins with
"/"), the "/" character will be removed. If the path is an absolute
system path, this creates an invalid path.

An example of when this may be an issue is in external dependencies
where an INF is within the external dependency, the `set_build_var`
flag is set, and DSC files refer to files by its build variable
(e.g. `$(SHARED_BINARIES)/Module.inf`). INFs in a binary distribution
like this example may contain a [Binaries] section and refer to
different section files that can be used by a platform to compose an
FFS file. For example, the PE32 (.efi) and DEPEX (.depex) files.

In this case, `$(SHARED_BINARIES)` will be an absolute path to the
ext dep directory and `FfsInfStatement.__InfParse__` will remove the
leading "/" character so the path is invalid.

This change first checks if the absolute path will resolve into the
current workspace. If it does (as will happen in the shared crypto
ext dep example above), it modifies the path to be relative to the
workspace so later logic dependent on relative paths can operate on
it. If the absolute path is not within the current workspace, it
follows previous behavior for backward compatibility to that
scenario.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Signed-off-by: Michael Kubacki 
---
  BaseTools/Source/Python/GenFds/FfsInfStatement.py | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/GenFds/FfsInfStatement.py 
b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
index 568efb6d7685..6550d939d49c 100644
--- a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
+++ b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
@@ -19,6 +19,7 @@ from .GenFdsGlobalVariable import GenFdsGlobalVariable
  from .Ffs import SectionSuffix,FdfFvFileTypeToFileType
  import subprocess
  import sys
+from pathlib import Path
  from . import Section
  from . import RuleSimpleFile
  from . import RuleComplexFile
@@ -156,7 +157,12 @@ class FfsInfStatement(FfsInfStatementClassObject):
  if len(self.InfFileName) > 1 and self.InfFileName[0] == '\\' and 
self.InfFileName[1] == '\\':
  pass
  elif self.InfFileName[0] == '\\' or self.InfFileName[0] == '/' :
-self.InfFileName = self.InfFileName[1:]
+ws_path = Path(GenFdsGlobalVariable.WorkSpaceDir)
+inf_path = Path(self.InfFileName)
+if ws_path in inf_path.parents:
+self.InfFileName = str(inf_path.relative_to(ws_path))
+else:
+self.InfFileName = self.InfFileName[1:]
  
  if self.InfFileName.find('$') == -1:

  InfPath = NormPath(self.InfFileName)





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116049): https://edk2.groups.io/g/devel/message/116049
Mute This Topic: https://groups.io/mt/104482915/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] .github/workflows/codeql.yml: Update actions being deprecated

[Michael Kubacki]

On 2/27/2024 1:33 AM, Michael D Kinney wrote:

-Original Message-
From: mikub...@linux.microsoft.com 
Sent: Monday, February 26, 2024 8:39 PM
To: devel@edk2.groups.io
Cc: Sean Brogan ; Joey Vagedes
; Kinney, Michael D

Subject: [PATCH v1 1/1] .github/workflows/codeql.yml: Update actions
being deprecated

From: Michael Kubacki 

Currently CodeQL runs have the following warnings:

   Node.js 16 actions are deprecated. Please update the following
   actions to use Node.js 20: actions/setup-python@v4,
   actions/upload-artifact@v3, actions/cache@v3. For more information
   see:
   https://github.blog/changelog/2023-09-22-github-actions-
transitioning-from-node-16-to-node-20/.

And:

   CodeQL Action v2 will be deprecated on December 5th, 2024. Please
   update all occurrences of the CodeQL Action in your workflow files
   to v3. For more information, see:
   https://github.blog/changelog/2024-01-12-code-scanning-deprecation-
of-codeql-action-v2/

The first is resolved by updating the actions to the latest versions
that were released to use Node.js 20. The second is specifically
referring to the codeql-action/upload-sarif action which is at v2.

This change updates all of the actions to the latest releases to
prevent deprecated versions from continuing to be used.

Cc: Sean Brogan 
Cc: Joey Vagedes 
Cc: Michael D Kinney 
Signed-off-by: Michael Kubacki 
---
  .github/workflows/codeql.yml | 37 +---
  1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/.github/workflows/codeql.yml
b/.github/workflows/codeql.yml
index c91e9d4dbeb3..e0c5f69f6cdf 100644
--- a/.github/workflows/codeql.yml
+++ b/.github/workflows/codeql.yml
@@ -79,7 +79,7 @@ jobs:
uses: actions/checkout@v4

  - name: Install Python
-  uses: actions/setup-python@v4
+  uses: actions/setup-python@v5
with:
  python-version: '3.11'
  cache: 'pip'
@@ -136,15 +136,26 @@ jobs:

print(f'ci_setup_supported={str(ci_setup_supported).lower()}', file=fh)
  print(f'setup_supported={str(setup_supported).lower()}',
file=fh)

+- name: Convert Arch to Log Format
+  id: convert_arch_hyphen
+  env:
+ARCH_LIST: ${{ matrix.ArchList }}
+  shell: python
+  run: |
+import os
+
+with open(os.environ['GITHUB_OUTPUT'], 'a') as fh:
+print(f'arch_list={os.environ["ARCH_LIST"].replace(",", "-
")}', file=fh)


I do not see this change described in the commit message.  Is it related?



Yes. A breaking change 
(https://github.com/actions/upload-artifact?tab=readme-ov-file#breaking-changes) 
in the actions/upload-artifact action is:


"Due to how Artifacts are created in this new version, it is no longer 
possible to upload to the same named Artifact multiple times. You must 
either split the uploads into multiple Artifacts with different names, 
or only upload once. Otherwise you will encounter an error."


We depended on that behavior previously to append multiple logs (e.g. 
setup log, update log, build log) to the same named artifact (named per 
package). These were appended after each operation so they are readily 
available if the operation failed and no further actions are run.


Now the artifacts must be unique in name. The hyphenation comes in 
because edk2 further builds some packages with both architectures in 
single build vs separate builds (e.g. IA32 and X64 vs IA32,X64). To 
uniquely name artifacts resulting from those builds, the architecture is 
also placed in the artifact name. For builds with multiple architectures 
the artifact name captures each architecture separated by a hyphen.


The final result is shown here: 
https://github.com/tianocore/edk2/actions/runs/8059750360


I will update the commit message to mention a behavior change in the 
underlying action caused a change in artifact naming convention.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116048): https://edk2.groups.io/g/devel/message/116048
Mute This Topic: https://groups.io/mt/104597854/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] RedfishPkg/RestJsonStructureDxe: Refine REST JSON C Structure DXE driver

[Igor Kulchytskyy via groups.io]

Reviewed-by: Igor Kulchytskyy 

Regards,
Igor

-Original Message-
From: abner.ch...@amd.com 
Sent: Sunday, February 25, 2024 9:44 PM
To: devel@edk2.groups.io
Cc: Nickle Wang ; Igor Kulchytskyy 
Subject: [EXTERNAL] [PATCH] RedfishPkg/RestJsonStructureDxe: Refine REST JSON C 
Structure DXE driver


**CAUTION: The e-mail below is from an external source. Please exercise caution 
before opening attachments, clicking links, or following guidance.**

From: Abner Chang 

BZ #: 4711
- Add mode debug messages.
- This driver shouldn't have a dependency on Redfish package and
  the references of "Redfish" terminology.
  Remove the references of "Redfish" from this driver.
- Fix the missing parameter of DEBUG macros used in this
  driver.

Signed-off-by: Abner Chang 
Cc: Nickle Wang 
Cc: Igor Kulchytskyy 
---
 .../RestJsonStructureDxe.inf  |  3 +-
 .../RestJsonStructureInternal.h   |  3 +-
 .../RestJsonStructureDxe.c| 96 ++-
 3 files changed, 96 insertions(+), 6 deletions(-)

diff --git a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.inf 
b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.inf
index 61e6253d318..e74c9dfd38b 100644
--- a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.inf
+++ b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.inf
@@ -2,6 +2,7 @@
 # Implementation of EFI REST JSON Structure Protocol.
 #
 #  (C) Copyright 2020 Hewlett Packard Enterprise Development LP
+#  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 ##

@@ -17,7 +18,6 @@
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
-  RedfishPkg/RedfishPkg.dec

 [Sources]
   RestJsonStructureDxe.c
@@ -26,6 +26,7 @@
 [LibraryClasses]
   BaseLib
   BaseMemoryLib
+  DebugLib
   MemoryAllocationLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
diff --git a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureInternal.h 
b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureInternal.h
index 8d7175125c1..04be5cc80b5 100644
--- a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureInternal.h
+++ b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureInternal.h
@@ -3,6 +3,7 @@
   Protocol.

   (C) Copyright 2020 Hewlett Packard Enterprise Development LP
+  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.

   SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -25,7 +26,7 @@
 typedef struct _REST_JSON_STRUCTURE_INSTANCE {
   LIST_ENTRY   NextRestJsonStructureInstance; 
///< Next convertor instance
   UINTNNumberOfNameSpaceToConvert;
///< Number of resource type this convertor supports.
-  EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER   *SupportedRsrcIndentifier; 
///< The resource type linklist
+  EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER   *SupportedRsrcIndentifier; 
///< The supported resource type array.
   EFI_REST_JSON_STRUCTURE_TO_STRUCTURE JsonToStructure;   
///< JSON to C structure function
   EFI_REST_JSON_STRUCTURE_TO_JSON  StructureToJson;   
///< C structure to JSON function
   EFI_REST_JSON_STRUCTURE_DESTORY_STRUCTUREDestroyStructure;  
///< Destory C struture function.
diff --git a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.c 
b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.c
index 404866fb319..0da5132e5ae 100644
--- a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.c
+++ b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.c
@@ -4,12 +4,14 @@
   Protocol.

   (C) Copyright 2020 Hewlett Packard Enterprise Development LP
+  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.

   SPDX-License-Identifier: BSD-2-Clause-Patent

 **/

 #include 
+#include 
 #include 
 #include "RestJsonStructureInternal.h"

@@ -72,6 +74,8 @@ RestJsonStructureRegister (
 }
   }

+  DEBUG ((DEBUG_MANAGEABILITY, "%a: %d REST JSON-C interpreter(s) to register 
for the name spaces.\n", __func__, NumberOfNS));
+
   Instance =
 (REST_JSON_STRUCTURE_INSTANCE *)AllocateZeroPool (sizeof 
(REST_JSON_STRUCTURE_INSTANCE) + NumberOfNS * sizeof 
(EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER));
   if (Instance == NULL) {
@@ -88,6 +92,10 @@ RestJsonStructureRegister (
   ThisSupportedInterp= JsonStructureSupported;
   for (Index = 0; Index < NumberOfNS; Index++) {
 CopyMem ((VOID *)CloneSupportedInterpId, (VOID 
*)>RestResourceInterp, sizeof 
(EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER));
+DEBUG ((DEBUG_MANAGEABILITY, "  Resource type : %a\n", 
ThisSupportedInterp->RestResourceInterp.NameSpace.ResourceTypeName));
+DEBUG ((DEBUG_MANAGEABILITY, "  Major version : %a\n", 
ThisSupportedInterp->RestResourceInterp.NameSpace.MajorVersion));
+DEBUG ((DEBUG_MANAGEABILITY, "  Minor version : %a\n", 
ThisSupportedInterp->RestResourceInterp.NameSpace.MinorVersion));
+DEBUG ((DEBUG_MANAGEABILITY, "  Errata 

Re: [edk2-devel] [PATCH v3 0/6] Introduce Redfish http protocol

[Igor Kulchytskyy via groups.io]

Reviewed-by: Igor Kulchytskyy 
Reviewed the whole patch v3.

Regards,
Igor

-Original Message-
From: Nickle Wang 
Sent: Monday, February 26, 2024 7:44 PM
To: devel@edk2.groups.io
Cc: Abner Chang ; Igor Kulchytskyy ; Nick 
Ramirez 
Subject: [EXTERNAL] [PATCH v3 0/6] Introduce Redfish http protocol


**CAUTION: The e-mail below is from an external source. Please exercise caution 
before opening attachments, clicking links, or following guidance.**

v3: address review comments.
1) handle the condition while memory allocation failure happens to
HTTP headers. Make sure that HTTP headers can be released properly while
calling HttpFreeHeaderFields().
2) Create macro ASCII_STR_DUPLICATE() to copy ASCII string.
3) Fix typos.

v2: address review comments.
1) add comment to show the source file of MathFtol.c
2) update macro to follow file name in RedfishHttpOperation.h

This patch series introduce Redfish HTTP protocol to RedfishPkg.
This is to improve Redfish performance and communication stability
between BIOS and Redfish service. Two big functions are introduced:
1) HTTP cache mechanism is implemented to reduce the number of
communiocation between BIOS and BMC.
2) HTTP request retry mechanism is implemented to handle communication
failure. With retry mechanism, Redfish feature driver can finish its job
as best as possible.

Several libraries are updated to fix build issues. Pull request is created:
https://github.com/tianocore/edk2/pull/5348

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
Cc: Nick Ramirez 

Nickle Wang (6):
  RedfishPkg: introduce Redfish HTTP protocol
  RedfishPkg: implement Redfish HTTP protocol
  RedfishPkg: introduce RedfishHttpLib
  RedfishPkg/RedfishLib: include RedfishServiceData.h
  RedfishPkg/RedfishDebugLib: use RedfishHttpLib
  RedfishPkg/RedfishCrtLib: fix unresolved external symbol issue

 RedfishPkg/RedfishPkg.dec |   32 +-
 RedfishPkg/RedfishComponents.dsc.inc  |3 +-
 RedfishPkg/RedfishLibs.dsc.inc|3 +-
 RedfishPkg/RedfishPkg.dsc |5 +-
 .../RedfishDebugLib/RedfishDebugLib.inf   |4 +-
 .../Library/RedfishHttpLib/RedfishHttpLib.inf |   43 +
 .../RedfishCrtLib/RedfishCrtLib.inf   |7 +-
 RedfishPkg/RedfishHttpDxe/RedfishHttpDxe.inf  |   73 +
 RedfishPkg/Include/Library/RedfishDebugLib.h  |2 +-
 RedfishPkg/Include/Library/RedfishHttpLib.h   |  326 
 RedfishPkg/Include/Library/RedfishLib.h   |   17 +-
 .../Protocol/EdkIIRedfishHttpProtocol.h   |  308 
 RedfishPkg/Include/RedfishServiceData.h   |   43 +
 RedfishPkg/RedfishHttpDxe/RedfishHttpData.h   |  256 
 RedfishPkg/RedfishHttpDxe/RedfishHttpDxe.h|   44 +
 .../RedfishHttpDxe/RedfishHttpOperation.h |   77 +
 .../Library/RedfishDebugLib/RedfishDebugLib.c |1 +
 .../Library/RedfishHttpLib/RedfishHttpLib.c   |  585 +++
 .../RedfishCrtLib/Ia32/MathFtol.c |   37 +
 RedfishPkg/RedfishHttpDxe/RedfishHttpData.c   |  667 
 RedfishPkg/RedfishHttpDxe/RedfishHttpDxe.c| 1344 +
 .../RedfishHttpDxe/RedfishHttpOperation.c |  692 +
 RedfishPkg/Redfish.fdf.inc|3 +-
 RedfishPkg/RedfishPkg.ci.yaml |2 +
 24 files changed, 4549 insertions(+), 25 deletions(-)
 create mode 100644 RedfishPkg/Library/RedfishHttpLib/RedfishHttpLib.inf
 create mode 100644 RedfishPkg/RedfishHttpDxe/RedfishHttpDxe.inf
 create mode 100644 RedfishPkg/Include/Library/RedfishHttpLib.h
 create mode 100644 RedfishPkg/Include/Protocol/EdkIIRedfishHttpProtocol.h
 create mode 100644 RedfishPkg/Include/RedfishServiceData.h
 create mode 100644 RedfishPkg/RedfishHttpDxe/RedfishHttpData.h
 create mode 100644 RedfishPkg/RedfishHttpDxe/RedfishHttpDxe.h
 create mode 100644 RedfishPkg/RedfishHttpDxe/RedfishHttpOperation.h
 create mode 100644 RedfishPkg/Library/RedfishHttpLib/RedfishHttpLib.c
 create mode 100644 RedfishPkg/PrivateLibrary/RedfishCrtLib/Ia32/MathFtol.c
 create mode 100644 RedfishPkg/RedfishHttpDxe/RedfishHttpData.c
 create mode 100644 RedfishPkg/RedfishHttpDxe/RedfishHttpDxe.c
 create mode 100644 RedfishPkg/RedfishHttpDxe/RedfishHttpOperation.c

--
2.34.1

-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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116046): https://edk2.groups.io/g/devel/message/116046
Mute This Topic: https://groups.io/mt/104594891/21656
Group Owner: 

Re: [edk2-devel] [edk2-redfish-client][PATCH] edk2-Redfish-client: Clarify HTTP method used for provisioning

[Igor Kulchytskyy via groups.io]

Reviewed-by: Igor Kulchytskyy 

Regards,
Igor

-Original Message-
From: abner.ch...@amd.com 
Sent: Sunday, February 25, 2024 11:55 PM
To: devel@edk2.groups.io
Cc: Nickle Wang ; Igor Kulchytskyy ; Mike 
Maslenkin 
Subject: [EXTERNAL] [edk2-redfish-client][PATCH] edk2-Redfish-client: Clarify 
HTTP method used for provisioning


**CAUTION: The e-mail below is from an external source. Please exercise caution 
before opening attachments, clicking links, or following guidance.**

From: Abner Chang 

Clarify the HTTP method that is used to provision BIOS
managed Redfish resource.

Signed-off-by: Abner Chang 
Cc: Nickle Wang 
Cc: Igor Kulchytskyy 
Cc: Mike Maslenkin 
---
 RedfishClientPkg/Readme.md| 35 ---
 .../Media/redfish-call-flow-provisioning.svg  |  2 +-
 .../Media/redfish-synchronization-design.svg  |  4 +--
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/RedfishClientPkg/Readme.md b/RedfishClientPkg/Readme.md
index 82cb9c8c99..1789dff6f8 100644
--- a/RedfishClientPkg/Readme.md
+++ b/RedfishClientPkg/Readme.md
@@ -310,21 +310,32 @@ job.

 Several interfaces defined in EDKII Redfish Resource Config Protocol work 
together to support Redfish synchronization:
 - Identify()
-  - This function is used to check if the given Redfish resource is the one 
the feature driver wants to manage. A platform
-library `RedfishResourceIdentifyLib` is introduced for platform to 
implement its own policy to identify Redfish resource.
+  - This function is used to check if the given Redfish resource is the one 
the feature driver
+wants to manage. A platform library `RedfishResourceIdentifyLib` is 
introduced for
+platform to implement its own policy to identify Redfish resource.
 - Check()
-  - This function is used to check the attribute status on Redfish service. If 
all attributes the feature driver manages
-are presented in Redfish service, feature driver must provision them 
already. Otherwise, Provisioning() will be called
-to perform resource provisioning job.
+  - This function is used to check the attribute status on Redfish service. If 
all attributes
+the feature driver manages are presented in Redfish service, feature 
driver must provision
+them already. Otherwise, Provisioning() will be called to perform resource 
provisioning
+job.
 - Provisioning()
-  - When this function is called, feature driver will provision all attributes 
that it managed to Redfish service. This
-operation usually create new resource at Redfish service and require 
different operation that specified by Redfish service.
+  - When this function is called, feature driver will provision all attributes 
that it managed
+to Redfish service. This operation usually creates the new Redfish 
properties at the
+existing URI in Redfish service. Use HTTP PATCH to provision Redfish 
properties as BIOS
+may only manage some but not all of the properties of the resource. See 
[Redfish-edk2 
implementation](#Redfish-Service-Implementation-that-Incorporates-with-EDK2-Redfish)
 for
+the details. HTTP POST is still used for creating a collection member, 
such as the
+collection member of processor or memory for the Redfish inventory 
management.
+However, HTTP PUT to overwrite an entire Redfish resource is not used in 
edk2 Redfish
+implementation as edk2 Redfish implementation has no idea of whether the 
Redfish resource
+is entirely managed by BIOS or not.
 - Consume()
-  - When there is pending settings in Redfish service, this function is called 
for feature driver to consume pending settings
-requested by user.
+  - When there is pending settings in Redfish service, this function is called 
for feature
+driver to consume pending settings requested by user. HTTP GET is the 
method used
+to retrieve Redfish properties.
 - Update()
-  - When platform configuration is updated, this function is called to update 
configuration changes to Redfish service and
-Redfish service can show the latest settings on platform.
+  - When platform configuration is updated, this function is called to update 
configuration
+changes to Redfish service and Redfish service can show the latest 
settings on platform.
+HTTP PATCH is the method used to update the properties of Redfish resource.

 The EDKII Redfish Resource Addendum Protocol is introduced to provide platform 
addendum data that Redfish service requires.
 This protocol will be called at Provisioning() and Update() functions so 
platform can add OEM attribute or any other attribute
@@ -338,7 +349,7 @@ struct _EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL {
 };
 ```

-### Redfish Service Implementation that Incorporates with EDK2 Redfish
+### Redfish
 Service Implementation that Incorporates with EDK2 Redfish
 The idea of Redfish synchronization design is to manage Redfish resource 
directly by platform host
 firmware. To do this, Redfish synchronization functions have to 

Re: [edk2-devel] [edk2-test v2] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c

[Heinrich Schuchardt]

On 27.02.24 07:37, Chao Li wrote:

DevicePathFromTextBBTextCoverage.c function CreateDNSDeviceNode has a
bug, code:

SctStrToIPv4Addr (, (EFI_IPv4_ADDRESS *)(DNS + sizeof 
(DNS_DEVICE_PATH)));

DNS is a pointer, which is increased by a structure size and converted
to EFI_IPv4_ADDRESS*, which will point to an unknown address. So fix it.

Fixes: 847e0363e846 ("SctPkg: Fix the UefiSct-Wincompatible-pointer-types 
warnings")

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

Cc: G Edhaya Chandran 
Cc: Barton Gao 
Cc: Carolyn Gjertsen 
Cc: Samer El-Haj-Mahmoud 
Cc: Eric Jin 
Cc: Arvin Chen 
Cc: Supreeth Venkatesh 
Cc: Heinrich Schuchardt 
Signed-off-by: Chao Li 
---
  .../BlackBoxTest/DevicePathFromTextBBTestCoverage.c   | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
index c96ee246..bd11c25a 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
@@ -1734,13 +1734,13 @@ CreateDNSDeviceNode (
}
  
if (DNS->IsIPv6 == 0) {

-SctStrToIPv4Addr (, (EFI_IPv4_ADDRESS *)(DNS + sizeof 
(DNS_DEVICE_PATH)));
-SctStrToIPv4Addr (, (EFI_IPv4_ADDRESS *)(DNS + sizeof 
(DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS)));
+SctStrToIPv4Addr (, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + sizeof 
(DNS_DEVICE_PATH)));
+SctStrToIPv4Addr (, (EFI_IPv4_ADDRESS *)((UINT8 *)DNS + sizeof 
(DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS)));
}
  
if (DNS->IsIPv6 == 1) {

-SctStrToIPv6Addr (, (EFI_IPv6_ADDRESS *)(DNS + sizeof 
(DNS_DEVICE_PATH)));
-SctStrToIPv6Addr (, (EFI_IPv6_ADDRESS *)(DNS + sizeof 
(DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS)));
+SctStrToIPv6Addr (, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + sizeof 
(DNS_DEVICE_PATH)));
+SctStrToIPv6Addr (, (EFI_IPv6_ADDRESS *)((UINT8 *)DNS + sizeof 
(DNS_DEVICE_PATH) + sizeof(EFI_IP_ADDRESS)));
}
  
return (EFI_DEVICE_PATH_PROTOCOL *) DNS;


Reviewed-by: Heinrich Schuchardt 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116044): https://edk2.groups.io/g/devel/message/116044
Mute This Topic: https://groups.io/mt/104598721/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 4/4] OvmfPkg/PlatformPei: log pei memory cap details

[Laszlo Ersek]

On 2/27/24 09:44, Gerd Hoffmann wrote:
> On Thu, Feb 15, 2024 at 09:24:41AM +0100, Laszlo Ersek wrote:
>>
>> Reviewed-by: Laszlo Ersek 
>>
>> This series is now ready for merging, as far as I'm concerned (thanks
>> for the updates in the other patches too, I've re-checked them). Please
>> send a reminder after the stable tag.
> 
> Friendly post-freeze ping ;)

Thanks!

Merged as commit range ba9c3ceaf83d..aceb3490a2a3, via
.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116043): https://edk2.groups.io/g/devel/message/116043
Mute This Topic: https://groups.io/mt/104350494/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 0/2] Make StandaloneMmCpu architecture independent

[Sami Mujawar]

Apologies for the delay.

Merged as 74b5309da9fb..ba9c3ceaf83d

Thanks.

Regards,

Sami Mujawar


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116042): https://edk2.groups.io/g/devel/message/116042
Mute This Topic: https://groups.io/mt/101646675/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/3] OvmfPkg: Add support for 5-level paging

[Laszlo Ersek]

Hi Liming,

can you please check the first two patches in the series? They should be
easy to review.

Thanks!
Laszlo

On 2/22/24 11:54, Gerd Hoffmann wrote:
> Patch #1 + #2 fix MdeModulePkg/DxeIplPeim to not assert in case a
> 5-level enabled build runs in 4-level paging mode.
> 
> Patch #3 updates PlatformInitLib for 5-level paging support (update
> PhysBits calculation).
> 
> v4:
>  - drop OvmfPkg/ResetVecor changes, they will be sent as
>separate patch series.
>  - add Ard's ack.
> v3:
>  - add resetvector fixes for sev and tdx
> v2 changes:
>  - fix sev/tdx handling with 5-level paging.
>  - more comments for 5-level page table setup.
>  - improve PAGE_* naming (new patch #3).
>  - rename Page5LevelSupported to Page5LevelEnabled (new patch #2).
>  - pick up some review tags.
> 
> Gerd Hoffmann (3):
>   MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
>   MdeModulePkg/DxeIplPeim: rename variable
>   OvmfPkg/PlatformInitLib: add 5-level paging support
> 
>  .../Core/DxeIplPeim/X64/VirtualMemory.c   | 24 +++
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c   | 63 +--
>  2 files changed, 57 insertions(+), 30 deletions(-)
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116041): https://edk2.groups.io/g/devel/message/116041
Mute This Topic: https://groups.io/mt/104506109/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/ConverterLib: check JSON value type

[Nickle Wang via groups.io]

Check and see if JSON object type is desired type before
getting its value. According to the Redfish schema, attribute
value can be null value. Add this error handling to avoid
system assertion while getting attribute value.

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
Cc: Nick Ramirez 
---
 RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c 
b/RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c
index 250ef75e4..7ee3c86e1 100644
--- a/RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c
+++ b/RedfishClientPkg/ConverterLib/src/RedfishCsCommon.c
@@ -1,5 +1,7 @@
 /** @file
 
+  Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
   Copyright Notice:
   Copyright 2019-2024 Distributed Management Task Force, Inc. All rights 
reserved.
   License: BSD 3-Clause License. For full text see link: 
https://github.com/DMTF/Redfish-JSON-C-Struct-Converter/blob/master/LICENSE.md
@@ -829,6 +831,10 @@ GetRedfishPropertyStr (
 return RedfishCS_status_not_found;
   }
 
+  if (!json_is_string (TempJsonObj)) {
+return RedfishCS_status_not_found;
+  }
+
   Status = allocateDuplicateStr (Cs, (char *)json_string_value (TempJsonObj), 
(void **)DstBuffer);
   return Status;
 }
@@ -914,6 +920,10 @@ GetRedfishPropertyInt64 (
 return Status;
   }
 
+  if (!json_is_integer (TempJsonObj)) {
+return RedfishCS_status_not_found;
+  }
+
   **Dst = (RedfishCS_int64)json_integer_value (TempJsonObj);
   return RedfishCS_status_success;
 }
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116040): https://edk2.groups.io/g/devel/message/116040
Mute This Topic: https://groups.io/mt/104601599/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] MdePkg: Update GetImage , GetImageInfo description details

[Pethaiyan Madhan]

1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
Add the following sentence at the end of the Image parameter
description. "May be NULL with a zero ImageSize in order to determine
the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as "The
ImageSize is not too small and Image is NULL."

2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
Add the following sentence at the end of the ImageInfo parameter
description."May be NULL with a zero ImageInfoSize in order to
determine the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as "The
ImageInfoSize is not too small and Image is NULL." and add new
descriptions for "EFI_INVALID_PARAMETER" return code.

 REF: UEFI spec v2.10 23.1.2

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Yi Li 
Cc: GuoX Xu 
Signed-off-by: Pethaiyan Madhan 
---
 MdePkg/Include/Protocol/FirmwareManagement.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h 
b/MdePkg/Include/Protocol/FirmwareManagement.h
index e535bb697d..90b7d83c8f 100644
--- a/MdePkg/Include/Protocol/FirmwareManagement.h
+++ b/MdePkg/Include/Protocol/FirmwareManagement.h
@@ -294,6 +294,8 @@ EFI_STATUS
  to contain the image(s) information if 
the buffer was too small.
   @param[in, out] ImageInfo  A pointer to the buffer in which firmware 
places the current image(s)
  information. The information is an array 
of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
+ May be NULL with a zero ImageInfoSize in 
order to determine the size of the
+ buffer needed.
   @param[out] DescriptorVersion  A pointer to the location in which 
firmware returns the version number
  associated with the 
EFI_FIRMWARE_IMAGE_DESCRIPTOR.
   @param[out] DescriptorCountA pointer to the location in which 
firmware returns the number of
@@ -314,7 +316,12 @@ EFI_STATUS
   @retval EFI_SUCCESSThe device was successfully updated with 
the new image.
   @retval EFI_BUFFER_TOO_SMALL   The ImageInfo buffer was too small. The 
current buffer size
  needed to hold the image(s) information 
is returned in ImageInfoSize.
-  @retval EFI_INVALID_PARAMETER  ImageInfoSize is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is not too small and 
ImageInfo is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorVersion is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorCount is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorSize is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
PackageVersion is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
PackageVersionName is NULL.
   @retval EFI_DEVICE_ERROR   Valid information could not be returned. 
Possible corrupted image.
 
 **/
@@ -341,6 +348,9 @@ EFI_STATUS
   @param[in]  ImageIndex A unique number identifying the firmware 
image(s) within the device.
  The number is between 1 and DescriptorCount.
   @param[out] Image  Points to the buffer where the current image 
is copied to.
+ May be NULL with a zero ImageSize in order to 
determine the size of the
+ buffer needed.
+
   @param[in, out] ImageSize  On entry, points to the size of the buffer 
pointed to by Image, in bytes.
  On return, points to the length of the image, 
in bytes.
 
@@ -348,7 +358,7 @@ EFI_STATUS
   @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too 
small to hold the
  image. The current buffer size needed to hold 
the image is returned
  in ImageSize.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and Image is 
NULL.
   @retval EFI_NOT_FOUND  The current image is not copied to the buffer.
   @retval EFI_UNSUPPORTEDThe operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to 
an authentication failure.
-- 
2.38.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116038): https://edk2.groups.io/g/devel/message/116038
Mute This Topic: https://groups.io/mt/104601542/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH V4] FmpDevicePkg: GetImageInfo Add missing conditions

[Pethaiyan Madhan]

1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
Add the following sentence at the end of the Image parameter
description. "May be NULL with a zero ImageSize in order to determine
the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as "The
ImageSize is not too small and Image is NULL."

2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
Add the following sentence at the end of the ImageInfo parameter
description."May be NULL with a zero ImageInfoSize in order to
determine the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as "The
ImageInfoSize is not too small and Image is NULL." and add new
descriptions for "EFI_INVALID_PARAMETER" return code.

 REF: UEFI spec v2.10 23.1.2

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Yi Li 
Cc: GuoX Xu 
Signed-off-by: Pethaiyan Madhan 
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 20 +++-
 FmpDevicePkg/FmpDxe/FmpDxe.h | 15 ---
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 1e7ec4a09e..1d580c9f69 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -43,7 +43,7 @@ const FIRMWARE_MANAGEMENT_PRIVATE_DATA  
mFirmwareManagementPrivateDataTemplate =
   FIRMWARE_MANAGEMENT_PRIVATE_DATA_SIGNATURE, // Signature
   NULL,   // Handle
   {// Fmp
-GetTheImageInfo,
+GetImageInfo,
 GetTheImage,
 SetTheImage,
 CheckTheImage,
@@ -417,6 +417,8 @@ PopulateDescriptor (
  to contain the image(s) information if 
the buffer was too small.
   @param[in, out] ImageInfo  A pointer to the buffer in which firmware 
places the current image(s)
  information. The information is an array 
of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
+ May be NULL with a zero ImageInfoSize in 
order to determine the size of the
+ buffer needed.
   @param[out] DescriptorVersion  A pointer to the location in which 
firmware returns the version number
  associated with the 
EFI_FIRMWARE_IMAGE_DESCRIPTOR.
   @param[out] DescriptorCountA pointer to the location in which 
firmware returns the number of
@@ -437,13 +439,18 @@ PopulateDescriptor (
   @retval EFI_SUCCESSThe device was successfully updated with 
the new image.
   @retval EFI_BUFFER_TOO_SMALL   The ImageInfo buffer was too small. The 
current buffer size
  needed to hold the image(s) information 
is returned in ImageInfoSize.
-  @retval EFI_INVALID_PARAMETER  ImageInfoSize is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is not too small and 
ImageInfo is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorVersion is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorCount is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorSize is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
PackageVersion is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
PackageVersionName is NULL.
   @retval EFI_DEVICE_ERROR   Valid information could not be returned. 
Possible corrupted image.
 
 **/
 EFI_STATUS
 EFIAPI
-GetTheImageInfo (
+GetImageInfo (
   IN EFI_FIRMWARE_MANAGEMENT_PROTOCOL  *This,
   IN OUT UINTN *ImageInfoSize,
   IN OUT EFI_FIRMWARE_IMAGE_DESCRIPTOR *ImageInfo,
@@ -495,7 +502,7 @@ GetTheImageInfo (
   // Confirm that buffer isn't null
   //
   if (  (ImageInfo == NULL) || (DescriptorVersion == NULL) || (DescriptorCount 
== NULL) || (DescriptorSize == NULL)
- || (PackageVersion == NULL))
+ || (PackageVersion == NULL) || (PackageVersionName == NULL))
   {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImageInfo() - Pointer Parameter is 
NULL.\n", mImageIdName));
 Status = EFI_INVALID_PARAMETER;
@@ -544,6 +551,9 @@ cleanup:
   @param[in]  ImageIndex A unique number identifying the firmware 
image(s) within the device.
  The number is between 1 and DescriptorCount.
   @param[in, out] Image  Points to the buffer where the current image 
is copied to.
+ May be NULL with a zero ImageSize in order to 
determine the size of the
+ buffer needed.
+
   @param[in, out] ImageSize  On entry, points to the size of the buffer 
pointed to by Image, in bytes.
  On return, points to the length of the image, 
in bytes.
 
@@ -551,7 +561,7 @@ cleanup:
   @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too 
small to hold the

[edk2-devel] [PATCH] SignedCapsulePkg: Update GetImage and GetImageInfo description details

[Pethaiyan Madhan]

1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
Add the following sentence at the end of the Image parameter
description. "May be NULL with a zero ImageSize in order to determine
the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as "The
ImageSize is not too small and Image is NULL."

2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
Add the following sentence at the end of the ImageInfo parameter
description."May be NULL with a zero ImageInfoSize in order to
determine the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as "The
ImageInfoSize is not too small and Image is NULL." and add new
descriptions for "EFI_INVALID_PARAMETER" return code.

 REF: UEFI spec v2.10 23.1.2

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Yi Li 
Cc: GuoX Xu 
Signed-off-by: Pethaiyan Madhan 
---
 .../SystemFirmwareUpdate/SystemFirmwareCommonDxe.c  | 13 +++--
 .../SystemFirmwareUpdate/SystemFirmwareDxe.h| 13 +++--
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git 
a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c 
b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
index 077bd0cb31..6e394d85d4 100644
--- a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
+++ b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
@@ -34,6 +34,8 @@ EFI_FIRMWARE_MANAGEMENT_PROTOCOL  mFirmwareManagementProtocol 
= {
  to contain the image(s) information if 
the buffer was too small.
   @param[in, out] ImageInfo  A pointer to the buffer in which firmware 
places the current image(s)
  information. The information is an array 
of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
+ May be NULL with a zero ImageInfoSize in 
order to determine the size of the
+ buffer needed.
   @param[out] DescriptorVersion  A pointer to the location in which 
firmware returns the version number
  associated with the 
EFI_FIRMWARE_IMAGE_DESCRIPTOR.
   @param[out] DescriptorCountA pointer to the location in which 
firmware returns the number of
@@ -54,7 +56,12 @@ EFI_FIRMWARE_MANAGEMENT_PROTOCOL  
mFirmwareManagementProtocol = {
   @retval EFI_SUCCESSThe device was successfully updated with 
the new image.
   @retval EFI_BUFFER_TOO_SMALL   The ImageInfo buffer was too small. The 
current buffer size
  needed to hold the image(s) information 
is returned in ImageInfoSize.
-  @retval EFI_INVALID_PARAMETER  ImageInfoSize is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is not too small and 
ImageInfo is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorVersion is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorCount is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
DescriptorSize is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
PackageVersion is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and 
PackageVersionName is NULL.
   @retval EFI_DEVICE_ERROR   Valid information could not be returned. 
Possible corrupted image.
 
 **/
@@ -153,6 +160,8 @@ FmpGetImageInfo (
   @param[in] ImageIndex  A unique number identifying the firmware 
image(s) within the device.
  The number is between 1 and DescriptorCount.
   @param[in,out] Image   Points to the buffer where the current image 
is copied to.
+ May be NULL with a zero ImageSize in order to 
determine the size of the
+ buffer needed.
   @param[in,out] ImageSize   On entry, points to the size of the buffer 
pointed to by Image, in bytes.
  On return, points to the length of the image, 
in bytes.
 
@@ -160,7 +169,7 @@ FmpGetImageInfo (
   @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too 
small to hold the
  image. The current buffer size needed to hold 
the image is returned
  in ImageSize.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and Image is 
NULL
   @retval EFI_NOT_FOUND  The current image is not copied to the buffer.
   @retval EFI_UNSUPPORTEDThe operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to 
an authentication failure.
diff --git 
a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareDxe.h 
b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareDxe.h
index c8443865cb..b2b2c78318 

[edk2-devel] [PATCH] MdeModulePkg: FirmwarePerformanceCommonEntryPoint error handling

[Sasikanth Valaparla]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4651

In FirmwarePerformanceCommonEntryPoint there is a MmLocateProtocol
which will locate the gEfiMmRscHandlerProtocolGuid and then
ASSERT_EFI_ERROR(Status) if the locate protocol fails when build is
debug enabled.

if the build is debug disabled then code will execute - register the
report status code using the API provided by
gEfiMmRscHandlerProtocolGuid.

Adding a proper error handling mechanism to handle the error gracefully
when debug is not enabled and to properly report the error if it cannot
continue.

Signed-off-by: Sasikanth Valaparla 
---
 .../FirmwarePerformanceCommon.c  | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceCommon.c
 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceCommon.c
index 9046896c55..7b13215ade 100644
--- 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceCommon.c
+++ 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceCommon.c
@@ -152,7 +152,10 @@ FirmwarePerformanceCommonEntryPoint (
 NULL,
 (VOID **)
 );
-  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR(Status)) {
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
 
   //
   // Register report status code listener for BootRecords and S3 Suspend Start 
and End.
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116036): https://edk2.groups.io/g/devel/message/116036
Mute This Topic: https://groups.io/mt/104601540/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 3/3] OvmfPkg/TdxDxe: Clear the registers before tdcall

[Isaku Yamahata]

> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4696
> 
> Refer to the [GHCI] spec, TDVF should clear the BIT5 for RBP in the mask.
> And TDVF should clear the regitsers to avoid leaking secrets to VMM.
> 
> Reference:
> [GHCI]: TDX Guest-Host-Communication Interface v1.5
> https://cdrdv2.intel.com/v1/dl/getContent/726792
> 
> Cc: Erdem Aktas 
> Cc: James Bottomley 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Tom Lendacky 
> Cc: Michael Roth 
> Cc: Gerd Hoffmann 
> Cc: Erdem Aktas 
> Cc: Isaku Yamahata 
> Signed-off-by: Ceping Sun 
> ---
>  OvmfPkg/TdxDxe/X64/ApRunLoop.nasm | 30 ++
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm 
> b/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm
> index 0bef89c48552..57560015f491 100644
> --- a/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm
> +++ b/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm
> @@ -20,7 +20,7 @@ SECTION .text
> 
>  BITS 64
> 
> -%define TDVMCALL_EXPOSE_REGS_MASK   0xffec
> +%define TDVMCALL_EXPOSE_REGS_MASK   0xffcc
>  %define TDVMCALL0x0
>  %define EXIT_REASON_CPUID   0xa
> 
> @@ -28,6 +28,30 @@ BITS 64
>db  0x66, 0x0f, 0x01, 0xcc
>  %endmacro
> 
> +%macro tdcall_regs_preamble 2
> +mov rax, %1
> +
> +xor rcx, rcx
> +mov ecx, %2
> +
> +; R10 = 0 (standard TDVMCALL)
> +
> +xor r10d, r10d
> +
> +; Zero out unused (for standard TDVMCALL) registers to avoid leaking
> +; secrets to the VMM.
> +
> +xor esi, esi
> +xor edi, edi
> +
> +xor edx, edx
> +xor ebp, ebp
> +xor r8d, r8d
> +xor r9d, r9d
> +xor r14, r14
> +xor r15, r15

We can just clear the corresponding bit of TDVMCALL_EXPOSE_REGS_MASK in 
addition to RBP.
Same to 1/3 and 3/3. We can eliminate tdcall_regs_postamble.
Any reason to bother to zero those registers and pass them to VMM?

Thanks,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116035): https://edk2.groups.io/g/devel/message/116035
Mute This Topic: https://groups.io/mt/104577524/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 0/6] UefiCpuPkg/MpInitLib: Add support for multiple MP_HAND_OFF HOBs

[Laszlo Ersek]

On 2/27/24 07:28, Ni, Ray wrote:
> Thank you, Laszlo!
> The changes you made look good to me.
> 
> By the way, is the following a common way to call out additional changes?
>> +[ler...@redhat.com: define one local variable per line [Ray]]

I don't know of any other project that uses it.

I must have seen Jordan a decade ago (?) use this way for marking
last-minute maintainer changes, and I've been following the pattern since.

I think it's preferable to silently editing a patch; after all, the
contributor did not verbatim post the patch that's about to be merged.

Laszlo

> 
> Thanks,
> Ray
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of Laszlo
>> Ersek
>> Sent: Monday, February 26, 2024 11:20 PM
>> To: devel@edk2.groups.io; kra...@redhat.com
>> Cc: Ni, Ray ; Kumar, Rahul R ;
>> Oliver Steffen 
>> Subject: Re: [edk2-devel] [PATCH v3 0/6] UefiCpuPkg/MpInitLib: Add support
>> for multiple MP_HAND_OFF HOBs
>>
>> On 2/22/24 17:01, Gerd Hoffmann wrote:
>>> Needed to boot guests with thousands of vcpus.
>>>
>>> v3:
>>>  - refine comments and commit messages.
>>>  - fix MaxCpusPerHob calculation.
>>>  - pick up review tags.
>>>  - add patch to speed up GetBspNumber a bit.
>>> v2:
>>>  - rework HOB loops for better performance: O(n) instead of O(n^2).
>>>
>>> Gerd Hoffmann (6):
>>>   UefiCpuPkg/MpInitLib: Add support for multiple HOBs to
>> GetMpHandOffHob
>>>   UefiCpuPkg/MpInitLib: Add support for multiple HOBs to
>> GetBspNumber()
>>>   UefiCpuPkg/MpInitLib: Add support for multiple HOBs to
>>> SwitchApContext()
>>>   UefiCpuPkg/MpInitLib: Add support for multiple HOBs to
>>> MpInitLibInitialize
>>>   UefiCpuPkg/MpInitLib: Add support for multiple HOBs to
>> SaveCpuMpData()
>>>   UefiCpuPkg/MpInitLib: return early in GetBspNumber()
>>>
>>>  UefiCpuPkg/Library/MpInitLib/MpLib.h|  14 ++-
>>>  UefiCpuPkg/Library/MpInitLib/MpLib.c| 157
>> +++-
>>>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c |  44 ---
>>>  3 files changed, 142 insertions(+), 73 deletions(-)
>>>
>>
>> BTW, differences in PR#5410 relative to v3 as posted:
>>
>> 1:  678ed78d24a3 ! 1:  ecd6c4bb3396 UefiCpuPkg/MpInitLib: Add support
>> for multiple HOBs to GetMpHandOffHob
>> @@ Commit message
>>
>>  Signed-off-by: Gerd Hoffmann 
>>  Message-Id: <20240222160106.686484-2-kra...@redhat.com>
>> +Reviewed-by: Ray Ni 
>> +Reviewed-by: Laszlo Ersek 
>>
>>   ## UefiCpuPkg/Library/MpInitLib/MpLib.h ##
>>  @@ UefiCpuPkg/Library/MpInitLib/MpLib.h: SwitchApContext (
>> 2:  23b3e66f9935 = 2:  189467980103 UefiCpuPkg/MpInitLib: Add support
>> for multiple HOBs to GetBspNumber()
>> 3:  e712d36775d0 = 3:  8ab0f63c0f04 UefiCpuPkg/MpInitLib: Add support
>> for multiple HOBs to SwitchApContext()
>> 4:  9a81417f4b76 ! 4:  995a8ace7801 UefiCpuPkg/MpInitLib: Add support
>> for multiple HOBs to MpInitLibInitialize
>> @@ Commit message
>>  Signed-off-by: Gerd Hoffmann 
>>  Reviewed-by: Ray Ni 
>>  Message-Id: <20240222160106.686484-5-kra...@redhat.com>
>> +Reviewed-by: Laszlo Ersek 
>>
>>   ## UefiCpuPkg/Library/MpInitLib/MpLib.c ##
>>  @@ UefiCpuPkg/Library/MpInitLib/MpLib.c: MpInitLibInitialize (
>> 5:  3a089b25725e ! 5:  f23c0d125e48 UefiCpuPkg/MpInitLib: Add support
>> for multiple HOBs to SaveCpuMpData()
>> @@ Commit message
>>
>>  Signed-off-by: Gerd Hoffmann 
>>  Message-Id: <20240222160106.686484-6-kra...@redhat.com>
>> +Reviewed-by: Ray Ni 
>> +Reviewed-by: Laszlo Ersek 
>> +[ler...@redhat.com: define one local variable per line [Ray]]
>>
>>   ## UefiCpuPkg/Library/MpInitLib/PeiMpLib.c ##
>>  @@ UefiCpuPkg/Library/MpInitLib/PeiMpLib.c: SaveCpuMpData (
>> IN CPU_MP_DATA  *CpuMpData
>> )
>>   {
>> -+  UINT32   MaxCpusPerHob, CpusInHob;
>> ++  UINT32   MaxCpusPerHob;
>> ++  UINT32   CpusInHob;
>> UINT64   Data64;
>>  -  UINTNIndex;
>> -+  UINT32   Index, HobBase;
>> ++  UINT32   Index;
>> ++  UINT32   HobBase;
>> CPU_INFO_IN_HOB  *CpuInfoInHob;
>> MP_HAND_OFF  *MpHandOff;
>> UINTNMpHandOffSize;
>> 6:  09435495e6e1 ! 6:  fbd8a114cd6e UefiCpuPkg/MpInitLib: return early
>> in GetBspNumber()
>> @@ Commit message
>>  Suggested-by: Laszlo Ersek 
>>  Signed-off-by: Gerd Hoffmann 
>>  Message-Id: <20240222160106.686484-7-kra...@redhat.com>
>> +Reviewed-by: Ray Ni 
>> +Reviewed-by: Laszlo Ersek 
>> +[ler...@redhat.com: s/ASSERT (FALSE)/ASSERT_EFI_ERROR
>> (EFI_NOT_FOUND)/ [Ray]]
>>
>>   ## UefiCpuPkg/Library/MpInitLib/MpLib.c ##
>>  @@ UefiCpuPkg/Library/MpInitLib/MpLib.c: GetBspNumber (
>> @@ UefiCpuPkg/Library/MpInitLib/MpLib.c: GetBspNumber (
>>  -  ASSERT (BspNumber != MAX_UINT32);
>>  -
>>  -  return BspNumber;
>> -+  ASSERT 

Re: [edk2-devel] [PATCH] NetworkPkg:Resolved Consecutive Pxe-Http Boot Issue

[Laszlo Ersek]

On 2/27/24 05:49, Sivaraman Nainar wrote:
> Hi Laszlo,
> 
> We can see the issue not only with SLES, it can be seen with Ubuntu 22 also.
> 
> Do we have any channel to work with grub team to fix this issue?

No particular channel. Oliver has been participating in upstream grub2
development (CC'd), so I figure bug analysis and bugfix posting should
occur on their normal development mailing list.

Laszlo


> -Original Message-
> From: Sivaraman Nainar
> Sent: Monday, February 26, 2024 4:01 PM
> To: devel@edk2.groups.io; Sivaraman Nainar ; Laszlo Ersek 
> ; Santhosh Kumar V ; Saloni 
> Kasbekar ; Zachary Clark-williams 
> 
> Cc: Raj V Akilan ; Soundharia R 
> Subject: RE: [EXTERNAL] Re: [edk2-devel] [PATCH] NetworkPkg:Resolved 
> Consecutive Pxe-Http Boot Issue
> 
> @Saloni Kasbekar, @Zachary Clark-williams,
> 
> Could you please add your feedback on the changes proposed?
> 
> Thanks
> Siva
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Sivaraman 
> Nainar via groups.io
> Sent: Thursday, February 22, 2024 7:33 AM
> To: Laszlo Ersek ; devel@edk2.groups.io; Santhosh Kumar V 
> ; Saloni Kasbekar ; 
> Zachary Clark-williams 
> Cc: Raj V Akilan ; Soundharia R 
> Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] NetworkPkg:Resolved Consecutive 
> Pxe-Http Boot Issue
> 
> 
> **CAUTION: The e-mail below is from an external source. Please exercise 
> caution before opening attachments, clicking links, or following guidance.**
> 
> Laszlo:
> 
> Thanks for the detailed feedback on the changes for this issue. Since we are 
> not sure if this change are valid / violate some purpose of SNP driver, it 
> mentioned as Workaround.
> 
> @Saloni Kasbekar and @Clark-williams, Zachary can add more on these changes.
> 
> As you recommended, we can have PCD which controls these changes till the 
> changes are addressed in grub.
> 
> @Santhosh Kumar V is this issue can be seen only in SLES 15 or it can be 
> found in any OS having Grub 2.x?
> 
> Thanks
> Siva
> -Original Message-
> From: Laszlo Ersek 
> Sent: Thursday, February 22, 2024 5:15 AM
> To: devel@edk2.groups.io; Santhosh Kumar V 
> Cc: Sivaraman Nainar ; Raj V Akilan ; 
> Soundharia R ; Saloni Kasbekar 
> ; Zachary Clark-williams 
> 
> Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] NetworkPkg:Resolved Consecutive 
> Pxe-Http Boot Issue
> 
> 
> **CAUTION: The e-mail below is from an external source. Please exercise 
> caution before opening attachments, clicking links, or following guidance.**
> 
> On 2/21/24 18:15, Santhosh Kumar V via groups.io wrote:
>> The customer has a server environment where PXE and HTTP service run in same 
>> Linux Server. In this environment a SUT trying to boot to SLES 15 OS via PXE 
>> from the Boot Menu. After PXE Boot file downloaded and grub Loaded without 
>> continuing for installation Exit is pressed and control back to Setup.
>> Now the HTTP boot to SLES 15 OS tried in the same environment and failed to 
>> download the file. If there is a reconnect -r performed before this HTTP 
>> Boot then boot file download and installation is getting success.
>> Root cause of the issue is, when Exit from grub performed, boot Loader Stops 
>> the SNP Driver and starts the same.
> 
> This sentence feels like the key one.
> 
> Are you saying that grub calls Snp->Start() just before it exits?
> 
> If so, am I right to suspect that that's a grub bug? It sounds like a 
> resource leak, after all.
> 
> Can you perhaps include a grub source code location / pointer in the commit 
> message?
> 
>> During this process SNP is in Initialized State. When HTTP boot is performed 
>> immediately after PXE Failure, the MNP configure method initiates the SNP 
>> Start again. Since the SNP already started by grub it returns 
>> EFI_ALREADY_STARTED and none of the upper Layer drivers are getting started.
>> As a work around in MNPConfigure(), if the SNP Start failed with Already 
>> Started and in Initialized state we can return success so that rest of the 
>> drivers can be loaded and HTTP boot can work.
>>
>>
>> Cc: Saloni Kasbekar 
>> Cc: Zachary Clark-williams 
>>
>> Signed-off-by: SanthoshKumar 
>> ---
>>  NetworkPkg/MnpDxe/MnpConfig.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/NetworkPkg/MnpDxe/MnpConfig.c
>> b/NetworkPkg/MnpDxe/MnpConfig.c index 93587d53aa..0f2df28d73 100644
>> --- a/NetworkPkg/MnpDxe/MnpConfig.c
>> +++ b/NetworkPkg/MnpDxe/MnpConfig.c
>> @@ -1120,7 +1120,9 @@ MnpStartSnp (
>>// Start the simple network.
>>
>>//
>>
>>Status = Snp->Start (Snp);
>>
>> -
>>
>> +  if ((Status == EFI_ALREADY_STARTED ) && (Snp->Mode->State ==
>> + EfiSimpleNetworkInitialized)) {
>>
>> +  return EFI_SUCCESS;
>>
>> +  }
>>
>>if (!EFI_ERROR (Status)) {
>>
>>  //
>>
>>  // Initialize the simple network.
>>
> 
> The commit message does say this is a workaround, and I don't immediately any 
> see why this workaround (in the code) would be problematic in practice, but 
> it still 

Re: [edk2-devel] [PATCH v2 13/23] UefiPayloadPkg: Prepare UefiPayloadPkg to use the CcSvsmLib library

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:52AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> The MpInitLib library will be updated to use the new CcSvsmLib library.
> To prevent any build breakage, update the UefiPayloadPkg DSC file to
> include the CcSvsmLib NULL library.
> 
> Signed-off-by: Tom Lendacky 

Acked-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116032): https://edk2.groups.io/g/devel/message/116032
Mute This Topic: https://groups.io/mt/104512961/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 12/23] UefiCpuPkg/CcSvsmLib: Create the CcSvsmLib library to support an SVSM

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:51AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> In order to support an SEV-SNP guest running under an SVSM at VMPL1 or
> lower, a new CcSvsmLib library must be created.
> 
> This library includes an interface to detect if running under an SVSM, an
> interface to return the current VMPL, an interface to perform memory
> validation and an interface to set or clear the attribute that allows a
> page to be used as a VMSA.
> 
> Signed-off-by: Tom Lendacky 

Acked-by: Gerd Hoffmann 

> ---
>  UefiCpuPkg/UefiCpuPkg.dec  |   5 +-
>  UefiCpuPkg/UefiCpuPkg.dsc  |   4 +-
>  UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.inf |  27 +
>  UefiCpuPkg/Include/Library/CcSvsmLib.h | 101 ++
>  UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.c   | 108 
>  UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.uni |  13 +++
>  6 files changed, 256 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index 571b59b36f0a..4a383c6d1d4d 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -2,7 +2,7 @@
>  # This Package provides UEFI compatible CPU modules and libraries.
>  #
>  # Copyright (c) 2007 - 2023, Intel Corporation. All rights reserved.
> -# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
> +# Copyright (C) 2023 - 2024, Advanced Micro Devices, Inc. All rights 
> reserved.
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -52,6 +52,9 @@ [LibraryClasses.IA32, LibraryClasses.X64]
>##  @libraryclass  Provides function to support CcExit processing.
>CcExitLib|Include/Library/CcExitLib.h
>  
> +  ##  @libraryclass  Provides function to support CcSvsm processing.
> +  CcSvsmLib|Include/Library/CcSvsmLib.h
> +
>##  @libraryclass  Provides function to get CPU cache information.
>CpuCacheInfoLib|Include/Library/CpuCacheInfoLib.h
>  
> diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
> index 10b33594e586..1ee726e6c6b5 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dsc
> +++ b/UefiCpuPkg/UefiCpuPkg.dsc
> @@ -2,7 +2,7 @@
>  #  UefiCpuPkg Package
>  #
>  #  Copyright (c) 2007 - 2023, Intel Corporation. All rights reserved.
> -#  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
> +#  Copyright (C) 2023 - 2024, Advanced Micro Devices, Inc. All rights 
> reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -61,6 +61,7 @@ [LibraryClasses]
>
> PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf
>
> TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>CcExitLib|UefiCpuPkg/Library/CcExitLibNull/CcExitLibNull.inf
> +  CcSvsmLib|UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.inf
>MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
>
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.inf
>CpuPageTableLib|UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableLib.inf
> @@ -159,6 +160,7 @@ [Components.IA32, Components.X64]
>UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
>UefiCpuPkg/Library/SmmCpuSyncLib/SmmCpuSyncLib.inf
>UefiCpuPkg/Library/CcExitLibNull/CcExitLibNull.inf
> +  UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.inf
>UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
>UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
>UefiCpuPkg/SecCore/SecCore.inf
> diff --git a/UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.inf 
> b/UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.inf
> new file mode 100644
> index ..b45a75941a8a
> --- /dev/null
> +++ b/UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.inf
> @@ -0,0 +1,27 @@
> +## @file
> +#  CcSvsm Base Support Library.
> +#
> +#  Copyright (C) 2024, Advanced Micro Devices, Inc. All rights reserved.
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION= 1.29
> +  BASE_NAME  = CcSvsmLibNull
> +  MODULE_UNI_FILE= CcSvsmLibNull.uni
> +  FILE_GUID  = 62b45e0f-c9b4-45ce-a5b3-41762709b3d9
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = CcSvsmLib
> +
> +[Sources.common]
> +  CcSvsmLibNull.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +
> diff --git a/UefiCpuPkg/Include/Library/CcSvsmLib.h 
> b/UefiCpuPkg/Include/Library/CcSvsmLib.h
> new file mode 100644
> index ..4715f4db3bd1
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Library/CcSvsmLib.h
> @@ -0,0 +1,101 @@
> +/** @file
> +  Public header file for the CcSvsmLib.
> +
> +  This library class defines some routines used for invoking an SVSM when the
> +  

Re: [edk2-devel] [PATCH v2 11/23] MdePkg/BaseLib: Add a new VMGEXIT instruction invocation for SVSM

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:50AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> The SVSM specification relies on a specific register calling convention to
> hold the parameters that are associated with the SVSM request. The SVSM is
> invoked by requesting the hypervisor to run the VMPL0 VMSA of the guest
> using the GHCB MSR Protocol or a GHCB NAE event.
> 
> Create a new version of the VMGEXIT instruction that will adhere to this
> calling convention and load the SVSM function arguments into the proper
> register before invoking the VMGEXIT instruction. On return, perform the
> atomic exchange on the SVSM call pending value as specified in the SVSM
> specification.
> 
> Signed-off-by: Tom Lendacky 

Acked-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116030): https://edk2.groups.io/g/devel/message/116030
Mute This Topic: https://groups.io/mt/104512956/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: add struct MP_HAND_OFF_CONFIG

[Gerd Hoffmann]

Move the WaitLoopExecutionMode and StartupSignalValue fields to a
separate HOB with the new struct.

Signed-off-by: Gerd Hoffmann 
---
 UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 13 ++--
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  3 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c  | 34 +
 4 files changed, 66 insertions(+), 22 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h 
b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
index 77854d6a81f8..ae93b7e3d7c9 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
@@ -15,7 +15,13 @@
 0x11e2bd88, 0xed38, 0x4abd, {0xa3, 0x99, 0x21, 0xf2, 0x5f, 0xd0, 0x7a, 
0x60 } \
   }
 
+#define MP_HANDOFF_CONFIG_GUID \
+  { \
+0xdabbd793, 0x7b46, 0x4144, {0x8a, 0xd4, 0x10, 0x1c, 0x7c, 0x08, 0xeb, 
0xfa } \
+  }
+
 extern EFI_GUID  mMpHandOffGuid;
+extern EFI_GUID  mMpHandOffConfigGuid;
 
 //
 // The information required to transfer from the PEI phase to the
@@ -43,8 +49,11 @@ typedef struct {
   //
   UINT32ProcessorIndex;
   UINT32CpuCount;
-  UINT32WaitLoopExecutionMode;
-  UINT32StartupSignalValue;
   PROCESSOR_HAND_OFFInfo[];
 } MP_HAND_OFF;
+
+typedef struct {
+  UINT32WaitLoopExecutionMode;
+  UINT32StartupSignalValue;
+} MP_HAND_OFF_CONFIG;
 #endif
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 3a7b9896cff4..d26035559f22 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -482,7 +482,8 @@ GetWakeupBuffer (
 **/
 VOID
 SwitchApContext (
-  IN CONST MP_HAND_OFF  *FirstMpHandOff
+  IN CONST MP_HAND_OFF_CONFIG  *MpHandOffConfig,
+  IN CONST MP_HAND_OFF *FirstMpHandOff
   );
 
 /**
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 8c186211fb38..a22bcfc0bc30 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -15,6 +15,7 @@
 
 EFI_GUID  mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
 EFI_GUID  mMpHandOffGuid   = MP_HANDOFF_GUID;
+EFI_GUID  mMpHandOffConfigGuid = MP_HANDOFF_CONFIG_GUID;
 
 /**
   Save the volatile registers required to be restored following INIT IPI.
@@ -1935,11 +1936,13 @@ GetBspNumber (
   This procedure allows the AP to switch to another section of
   memory and continue its loop there.
 
-  @param[in] FirstMpHandOff  Pointer to first MP hand-off HOB body.
+  @param[in] MpHandOffConfig  Pointer to MP hand-off config HOB body.
+  @param[in] FirstMpHandOff   Pointer to first MP hand-off HOB body.
 **/
 VOID
 SwitchApContext (
-  IN CONST MP_HAND_OFF  *FirstMpHandOff
+  IN CONST MP_HAND_OFF_CONFIG  *MpHandOffConfig,
+  IN CONST MP_HAND_OFF *FirstMpHandOff
   )
 {
   UINTN  Index;
@@ -1955,7 +1958,7 @@ SwitchApContext (
 for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
   if (MpHandOff->ProcessorIndex + Index != BspNumber) {
 *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = 
(UINTN)SwitchContextPerAp;
-*(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = 
MpHandOff->StartupSignalValue;
+*(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = 
MpHandOffConfig->StartupSignalValue;
   }
 }
   }
@@ -1975,6 +1978,26 @@ SwitchApContext (
   }
 }
 
+/**
+  Get pointer to MP_HAND_OFF_CONFIG GUIDed HOB body.
+
+  @return  The pointer to MP_HAND_OFF_CONFIG structure.
+**/
+MP_HAND_OFF_CONFIG *
+GetMpHandOffConfigHob (
+  VOID
+  )
+{
+  EFI_HOB_GUID_TYPE  *GuidHob;
+
+  GuidHob = GetFirstGuidHob ();
+  if (GuidHob == NULL) {
+return NULL;
+  }
+
+  return (MP_HAND_OFF_CONFIG *)GET_GUID_HOB_DATA (GuidHob);
+}
+
 /**
   Get pointer to next MP_HAND_OFF GUIDed HOB body.
 
@@ -2022,6 +2045,7 @@ MpInitLibInitialize (
   VOID
   )
 {
+  MP_HAND_OFF_CONFIG   *MpHandOffConfig;
   MP_HAND_OFF  *FirstMpHandOff;
   MP_HAND_OFF  *MpHandOff;
   CPU_INFO_IN_HOB  *CpuInfoInHob;
@@ -2239,13 +2263,15 @@ MpInitLibInitialize (
   }
 }
 
+MpHandOffConfig = GetMpHandOffConfigHob ();
+ASSERT (MpHandOffConfig != NULL);
 DEBUG ((
   DEBUG_INFO,
   "FirstMpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *): %04d\n",
-  FirstMpHandOff->WaitLoopExecutionMode,
+  MpHandOffConfig->WaitLoopExecutionMode,
   sizeof (VOID *)
   ));
-if (FirstMpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
+if (MpHandOffConfig->WaitLoopExecutionMode == sizeof (VOID *)) {
   ASSERT (CpuMpData->ApLoopMode != ApInHltLoop);
 
   CpuMpData->FinishedCount= 0;
@@ -2261,7 +2287,7 @@ MpInitLibInitialize (
   // enables the APs to switch to a different memory section and continue 
their
   // looping process there.
   //
-  SwitchApContext 

Re: [edk2-devel] CodeQL Analysis in edk2

[Gerd Hoffmann]

  Hi,

> I am hoping we can work together to improve the overall quality of the
> code and minimize the number of CodeQL alerts.

Seems CodeQL now runs as part of CI and flags issues it has found.

It complains about a possible NULL pointer dereference:
https://github.com/tianocore/edk2/runs/22021016348

This is not correct, but I doubt code analysis will ever be clever
enough to figure this automatically.  So I've added an ASSERT()
explicitly saying so, which should help both human reviewers and
code analyzers.

Apparently that does not change anything for CodeQL though.  I guess
the CodeQL config must be updated so it knows what ASSERT() means?
Maybe it is ignored simply because it is upper case (unlike the
standard C library version which is lower case)?

thanks & take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116028): https://edk2.groups.io/g/devel/message/116028
Mute This Topic: https://groups.io/mt/102444916/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 09/23] OvmfPkg/BaseMemEncryptSevLib: Maximize Page State Change efficiency

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:48AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> When building the Page State Change entries for a range of memory, it can
> happen that multiple calls to BuildPageStateBuffer() need to be made. If
> the size of the input work area passed to BuildPageStateBuffer() exceeds
> the number of entries that can be passed to the hypervisor using the GHCB
> shared buffer, the Page State Change VMGEXIT support will issue multiple
> VMGEXITs to process all entries in the buffer.
> 
> However, it could be that the final VMGEXIT for each round of Page State
> Changes is only for a small number of entries and subsequent VMGEXITs may
> still be issued to handle the full range of memory requested. To maximize
> the number of entries processed during the Page State Change VMGEXIT,
> limit BuildPageStateBuffer() to not build entries that exceed the maximum
> number of entries that can be handled in a single Page State Change
> VMGEXIT.
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116027): https://edk2.groups.io/g/devel/message/116027
Mute This Topic: https://groups.io/mt/104512950/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 08/23] OvmfPkg/BaseMemEncryptSevLib: Re-organize page state change support

[Gerd Hoffmann]

> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -23,6 +23,8 @@ STATIC BOOLEAN  mAddressEncMaskChecked = FALSE;
>  STATIC UINT64   mAddressEncMask;
>  STATIC PAGE_TABLE_POOL  *mPageTablePool = NULL;
>  
> +STATIC VOID  *mPscBuffer = NULL;
> +
>  typedef enum {
>SetCBit,
>ClearCBit

Oh.  Global variable in PEI code (both pre-existing and newly added).

This is problematic because in OVMF PEI is executed in-place and the
firmware volumes is measured by TPM PEIM.  Global variables modify
the PEI firmware volume and break the measurement.

A while back OVMF added EFI_HOB_PLATFORM_INFO (see
OvmfPkg/Include/Library/PlatformInitLib.h) to fix that.  Most fields
in that struct used to be global variables.

> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
> @@ -17,6 +17,8 @@
>  #include "SnpPageStateChange.h"
>  #include "VirtualMemory.h"
>  
> +STATIC UINT8  mPscBufferPage[EFI_PAGE_SIZE];
> +

Same problem.

Given this is a pre-exising problem, affects SEV only and the rest of
the patch looks fine:
Acked-by: Gerd Hoffmann 

But it should be cleaned up at some point.  BaseMemEncryptSevLib needs
an update anyway (use CpuPageTableLib, support 5-level paging).

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116026): https://edk2.groups.io/g/devel/message/116026
Mute This Topic: https://groups.io/mt/104512949/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/2] MdePkg: Add TCG PFP 1.06 support.

[Wenxing Hou]

Add support for
TCG PC Client Platform Firmware Profile Specification 1.06.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Jiewen Yao 

Signed-off-by: Wenxing Hou 
---
 .../IndustryStandard/UefiTcgPlatform.h| 186 +-
 1 file changed, 184 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h 
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index e07840c9dd..61bd4e4667 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -1,8 +1,8 @@
 /** @file
   TCG EFI Platform Definition in TCG_EFI_Platform_1_20_Final and
-  TCG PC Client Platform Firmware Profile Specification, Revision 1.05
+  TCG PC Client Platform Firmware Profile Specification, Revision 1.06
 
-  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -53,6 +53,18 @@
 #define EV_EFI_VARIABLE_AUTHORITY (EV_EFI_EVENT_BASE + 0xE0)
 #define EV_EFI_SPDM_FIRMWARE_BLOB (EV_EFI_EVENT_BASE + 0xE1)
 #define EV_EFI_SPDM_FIRMWARE_CONFIG   (EV_EFI_EVENT_BASE + 0xE2)
+#define EV_EFI_SPDM_DEVICE_BLOB   EV_EFI_SPDM_FIRMWARE_BLOB
+#define EV_EFI_SPDM_DEVICE_CONFIG EV_EFI_SPDM_FIRMWARE_CONFIG
+//
+// The SPDM policy database for SPDM verification.
+// It goes to PCR7
+//
+#define EV_EFI_SPDM_DEVICE_POLICY  (EV_EFI_EVENT_BASE + 0xE3)
+//
+// The SPDM policy authority for SPDM verification for the signature
+// of GET_MEASUREMENT or CHALLENGE_AUTH. It goes to PCR7.
+//
+#define EV_EFI_SPDM_DEVICE_AUTHORITY  (EV_EFI_EVENT_BASE + 0xE4)
 
 #define EFI_CALLING_EFI_APPLICATION \
   "Calling EFI Application from Boot Option"
@@ -374,6 +386,7 @@ typedef struct {
 #define TCG_EfiSpecIDEventStruct_SPEC_VERSION_MINOR_TPM2   0
 #define TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2  0
 #define TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105  105
+#define TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_106  106
 
 typedef struct {
   UINT8 signature[16];
@@ -492,4 +505,173 @@ typedef struct tdTCG_EfiStartupLocalityEvent {
 //
 #pragma pack ()
 
+//
+// 
==
+// Event TypePCR  Event Log
   Usage
+// 
==
+// EV_EFI_SPDM_DEVICE_BLOB   2SPDM_MEASUREMENT_BLOCK (subtype) 
   MEASUREMENT from device
+// EV_EFI_SPDM_DEVICE_CONFIG 3SPDM_MEASUREMENT_BLOCK (subtype) 
   MEASUREMENT from device
+// EV_EFI_SPDM_DEVICE_BLOB   2SPDM_MEASUREMENT_SUMMARY_HASH.TCB 
(subtype) SUMMARY_HASH from device
+
+// EV_EFI_SPDM_DEVICE_POLICY 7UEFI_VARIABLE_DATA with 
EFI_SIGNATURE_LIST  Provisioned device public cert.
+// EV_EFI_SPDM_DEVICE_AUTHORITY  7UEFI_VARIABLE_DATA with 
EFI_SIGNATURE_DATA  CHALLENGE_AUTH signature verification
+// 
==
+//
+
+#define PCR_INDEX_FOR_SIGNATURE_DB  7
+
+#pragma pack(1)
+
+#define TCG_DEVICE_SECURITY_EVENT_DATA_VERSION_11
+#define TCG_DEVICE_SECURITY_EVENT_DATA_VERSION_22
+#define TCG_DEVICE_SECURITY_EVENT_DATA_SIGNATURE_2  "SPDM Device Sec2"
+
+typedef struct {
+  UINT8 Signature[16];
+  UINT16Version;
+  UINT8 AuthState;
+  UINT8 Reserved;
+  UINT32Length;  // Length in bytes for all following 
structures.
+  UINT32DeviceType;
+  UINT32SubHeaderType;
+  UINT32SubHeaderLength;  // Length in bytes of the 
sub header followed by.
+  UINT64SubHeaderUID; // Universal identifier 
assigned by the event log creator. It can be used to bind two sub header 
structure together.
+  // UINT64 DevicePathLength;
+  // UINT8  DevicePath[DevicePathLength];
+} TCG_DEVICE_SECURITY_EVENT_DATA_HEADER2;
+
+#define TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_AUTH_STATE_SUCCESS   0
+#define TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_AUTH_STATE_NO_AUTH   1
+#define TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_AUTH_STATE_NO_BINDING2
+#define TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_AUTH_STATE_FAIL_NO_SIG   3
+#define TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_AUTH_STATE_FAIL_INVALID  4
+#define TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_AUTH_STATE_NO_SPDM   0xFF
+
+#define 
TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_SUB_HEADER_TYPE_SPDM_MEASUREMENT_BLOCK  0
+#define TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_SUB_HEADER_TYPE_SPDM_CERT_CHAIN  
   1
+
+typedef struct {
+  UINT16SpdmVersion;
+  UINT8 SpdmMeasurementBlockCount;
+  UINT8 Reserved;
+  UINT32SpdmMeasurementHashAlgo;

[edk2-devel] [PATCH 1/2] MdePkg: Add SPDM1.2 support.

[Wenxing Hou]

Update Spdm.h to support 1.2 new features, such as:
Authentication and measurement. It wil be used in DeviceSecurity.
The DeviceSecurity feature is from
TCG PC Client Platform Firmware Profile Specification 1.06.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Jiewen Yao 

Signed-off-by: Wenxing Hou 
---
 MdePkg/Include/IndustryStandard/Spdm.h | 1110 ++--
 1 file changed, 1061 insertions(+), 49 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Spdm.h 
b/MdePkg/Include/IndustryStandard/Spdm.h
index 4ec7a5ed1f..7940caa95e 100644
--- a/MdePkg/Include/IndustryStandard/Spdm.h
+++ b/MdePkg/Include/IndustryStandard/Spdm.h
@@ -1,8 +1,8 @@
 /** @file
-  Definitions of Security Protocol & Data Model Specification (SPDM)
-  version 1.0.0 in Distributed Management Task Force (DMTF).
+  Definitions of DSP0274 Security Protocol & Data Model Specification (SPDM)
+  version 1.2.0 in Distributed Management Task Force (DMTF).
 
-Copyright (c) 2019, Intel Corporation. All rights reserved.
+Copyright (c) 2019 - 2024, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -12,29 +12,72 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #pragma pack(1)
 
+#define SPDM_MAX_SLOT_COUNT8
+#define SPDM_MAX_OPAQUE_DATA_SIZE  1024
+#define SPDM_NONCE_SIZE32
+#define SPDM_RANDOM_DATA_SIZE  32
 ///
-/// SPDM response code
+/// SPDM response code (1.0)
 ///
-#define SPDM_DIGESTS0x01
-#define SPDM_CERTIFICATE0x02
-#define SPDM_CHALLENGE_AUTH 0x03
-#define SPDM_VERSION0x04
-#define SPDM_MEASUREMENTS   0x60
-#define SPDM_CAPABILITIES   0x61
-#define SPDM_SET_CERT_RESPONSE  0x62
-#define SPDM_ALGORITHMS 0x63
-#define SPDM_ERROR  0x7F
+#define SPDM_DIGESTS  0x01
+#define SPDM_CERTIFICATE  0x02
+#define SPDM_CHALLENGE_AUTH   0x03
+#define SPDM_VERSION  0x04
+#define SPDM_MEASUREMENTS 0x60
+#define SPDM_CAPABILITIES 0x61
+#define SPDM_ALGORITHMS   0x63
+#define SPDM_VENDOR_DEFINED_RESPONSE  0x7E
+#define SPDM_ERROR0x7F
 ///
-/// SPDM request code
+/// SPDM response code (1.1)
 ///
-#define SPDM_GET_DIGESTS   0x81
-#define SPDM_GET_CERTIFICATE   0x82
-#define SPDM_CHALLENGE 0x83
-#define SPDM_GET_VERSION   0x84
-#define SPDM_GET_MEASUREMENTS  0xE0
-#define SPDM_GET_CAPABILITIES  0xE1
-#define SPDM_NEGOTIATE_ALGORITHMS  0xE3
-#define SPDM_RESPOND_IF_READY  0xFF
+#define SPDM_KEY_EXCHANGE_RSP   0x64
+#define SPDM_FINISH_RSP 0x65
+#define SPDM_PSK_EXCHANGE_RSP   0x66
+#define SPDM_PSK_FINISH_RSP 0x67
+#define SPDM_HEARTBEAT_ACK  0x68
+#define SPDM_KEY_UPDATE_ACK 0x69
+#define SPDM_ENCAPSULATED_REQUEST   0x6A
+#define SPDM_ENCAPSULATED_RESPONSE_ACK  0x6B
+#define SPDM_END_SESSION_ACK0x6C
+///
+/// SPDM response code (1.2)
+///
+#define SPDM_CSR  0x6D
+#define SPDM_SET_CERTIFICATE_RSP  0x6E
+#define SPDM_CHUNK_SEND_ACK   0x05
+#define SPDM_CHUNK_RESPONSE   0x06
+///
+/// SPDM request code (1.0)
+///
+#define SPDM_GET_DIGESTS 0x81
+#define SPDM_GET_CERTIFICATE 0x82
+#define SPDM_CHALLENGE   0x83
+#define SPDM_GET_VERSION 0x84
+#define SPDM_GET_MEASUREMENTS0xE0
+#define SPDM_GET_CAPABILITIES0xE1
+#define SPDM_NEGOTIATE_ALGORITHMS0xE3
+#define SPDM_VENDOR_DEFINED_REQUEST  0xFE
+#define SPDM_RESPOND_IF_READY0xFF
+///
+/// SPDM request code (1.1)
+///
+#define SPDM_KEY_EXCHANGE   0xE4
+#define SPDM_FINISH 0xE5
+#define SPDM_PSK_EXCHANGE   0xE6
+#define SPDM_PSK_FINISH 0xE7
+#define SPDM_HEARTBEAT  0xE8
+#define SPDM_KEY_UPDATE 0xE9
+#define SPDM_GET_ENCAPSULATED_REQUEST   0xEA
+#define SPDM_DELIVER_ENCAPSULATED_RESPONSE  0xEB
+#define SPDM_END_SESSION0xEC
+///
+/// SPDM request code (1.2)
+///
+#define SPDM_GET_CSR  0xED
+#define SPDM_SET_CERTIFICATE  0xEE
+#define SPDM_CHUNK_SEND   0x85
+#define SPDM_CHUNK_GET0x86
 
 ///
 /// SPDM message header
@@ -46,13 +89,18 @@ typedef struct {
   UINT8Param2;
 } SPDM_MESSAGE_HEADER;
 
-#define SPDM_MESSAGE_VERSION  0x10
+#define SPDM_MESSAGE_VERSION_10  0x10
+#define SPDM_MESSAGE_VERSION_11  0x11
+#define SPDM_MESSAGE_VERSION_12  0x12
+#define SPDM_MESSAGE_VERSION SPDM_MESSAGE_VERSION_10
 
 ///
 /// SPDM GET_VERSION request
 ///
 typedef struct {
   SPDM_MESSAGE_HEADERHeader;
+  // Param1 == RSVD
+  // Param2 == RSVD
 } SPDM_GET_VERSION_REQUEST;
 
 ///
@@ -60,6 +108,8 @@ typedef struct {
 ///
 typedef struct {
   SPDM_MESSAGE_HEADERHeader;
+  // Param1 == RSVD
+  // Param2 == RSVD
   UINT8  Reserved;
   UINT8  

[edk2-devel] [PATCH 0/2] Add support for TCG PFP 1.06

[Wenxing Hou]

Update Spdm.h to support 1.2 new features,
such as: Authentication and measurement.
It wil be used in DeviceSecurity.
The DeviceSecurity is from
TCG PC Client Platform Firmware Profile Specification 1.06.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Jiewen Yao 
Signed-off-by: Wenxing Hou 


Wenxing Hou (2):
  MdePkg: Add SPDM1.2 support.
  MdePkg: Add TCG PFP 1.06 support.

 MdePkg/Include/IndustryStandard/Spdm.h| 1110 -
 .../IndustryStandard/UefiTcgPlatform.h|  186 ++-
 2 files changed, 1245 insertions(+), 51 deletions(-)

-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116023): https://edk2.groups.io/g/devel/message/116023
Mute This Topic: https://groups.io/mt/104600442/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-test v2] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c

[G Edhaya Chandran]

The patch is upstreamed through the commit:
https://github.com/tianocore/edk2-test/commit/cabb98d44be94e7547605435a0be7c4946d10f8b


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116022): https://edk2.groups.io/g/devel/message/116022
Mute This Topic: https://groups.io/mt/104598721/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 07/23] MdePkg: Avoid hardcoded value for number of Page State Change entries

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:46AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> The SNP_PAGE_STATE_MAX_ENTRY is based on the number of entries that can
> fit in the GHCB shared buffer. As a result, the SNP_PAGE_STATE_CHANGE_INFO
> structure maps the full GHCB shared buffer based on the shared buffer size
> being 2032 bytes.
> 
> Instead of using a hardcoded value for SNP_PAGE_STATE_MAX_ENTRY, use a
> build calculated value. Since the SNP_PAGE_STATE_CHANGE_INFO is used as a
> mapping, eliminate the hardcoded array size so that the structure can be
> used based on any size buffer.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  MdePkg/Include/Register/Amd/Ghcb.h | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/MdePkg/Include/Register/Amd/Ghcb.h 
> b/MdePkg/Include/Register/Amd/Ghcb.h
> index 432d67e3e223..0cdc00627472 100644
> --- a/MdePkg/Include/Register/Amd/Ghcb.h
> +++ b/MdePkg/Include/Register/Amd/Ghcb.h
> @@ -197,13 +197,14 @@ typedef struct {
>UINT32Reserved;
>  } SNP_PAGE_STATE_HEADER;
>  
> -#define SNP_PAGE_STATE_MAX_ENTRY  253
> -
>  typedef struct {
>SNP_PAGE_STATE_HEADERHeader;
> -  SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
> +  SNP_PAGE_STATE_ENTRY Entry[];
>  } SNP_PAGE_STATE_CHANGE_INFO;

Good.

> +#define SNP_PAGE_STATE_MAX_ENTRY  \
> +  ((sizeof (((GHCB *)0)->SharedBuffer) - sizeof (SNP_PAGE_STATE_HEADER)) / 
> sizeof (SNP_PAGE_STATE_ENTRY))

Can be dropped I think, after applying patch #6 BaseMemEncryptSevLib
does not use SNP_PAGE_STATE_MAX_ENTRY any more.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116021): https://edk2.groups.io/g/devel/message/116021
Mute This Topic: https://groups.io/mt/104512946/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 06/23] OvmfPkg/BaseMemEncryptSevLib: Calculate memory size for Page State Change

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:45AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> Calculate the amount of memory that can be use to build the Page State
> Change data (SNP_PAGE_STATE_CHANGE_INFO) instead of using a hard-coded
> size. This allows for changes to the GHCB shared buffer size without
> having to make changes to the page state change code.
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116020): https://edk2.groups.io/g/devel/message/116020
Mute This Topic: https://groups.io/mt/104512944/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 05/23] OvmfPkg/BaseMemEncryptSevLib: Fix uncrustify errors

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:44AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> In prep for follow-on patches, fix an area of the code that does not meet
> the uncrustify coding standards.
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116019): https://edk2.groups.io/g/devel/message/116019
Mute This Topic: https://groups.io/mt/104512941/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 04/23] UefiCpuPkg/MpInitLib: Always use AP Create if PcdSevSnpApicIds is set

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:43AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> Currently, the first time an AP is started for an SEV-SNP guest, it relies
> on the VMSA as set by the hypervisor. If the list of APIC IDs has been
> retrieved, this is not necessary. Instead, use the SEV-SNP AP Create
> protocol to start the AP for the first time and thereafter using the VMPL
> at which the BSP is running.
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116018): https://edk2.groups.io/g/devel/message/116018
Mute This Topic: https://groups.io/mt/104512938/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Question about SMM code privilege switch

[Yoshinoya]

Hi,I found smm code privilege switch sample code in project mu.For example : 
switch ring0 to ring3 through tss gate call.So, is there any plan to introduce 
it to udk smm sample code?

Thanks

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116017): https://edk2.groups.io/g/devel/message/116017
Mute This Topic: https://groups.io/mt/104600076/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 03/23] OvmfPkg/PlatformPei: Retrieve APIC IDs from the hypervisor

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:42AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> If the hypervisor supports retrieval of the vCPU APIC IDs, retrieve
> them before any APs are actually started. The APIC IDs can be used
> to start the APs for any SEV-SNP guest, but is a requirement for an
> SEV-SNP guest that is running under an SVSM.
> 
> After retrieving the APIC IDs, save the address of the APIC ID data
> structure in a GUIDed HOB.
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116016): https://edk2.groups.io/g/devel/message/116016
Mute This Topic: https://groups.io/mt/104512934/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 02/23] MdePkg: GHCB APIC ID retrieval support definitions

[Gerd Hoffmann]

On Fri, Feb 23, 2024 at 12:16:54AM +, Ni, Ray wrote:
> 
> > +//
> > +// Get APIC IDs
> > +//
> > +#define EFI_APIC_IDS_GUID  \
> > +  { 0xbc964338, 0xee39, 0x4fc8, { 0xa2, 0x24, 0x10, 0x10, 0x8b, 0x17, 0x80,
> > 0x1b }}
> > +extern EFI_GUID  gEfiApicIdsGuid;
> 
> Since the above GUID is associated with the structure below, how about
> rename the GUID from "gEfiApicIdsGuid" which is very general to a specific
> name "gGhcbApicIdsGuid"?

Same comment goes for EFI_APIC_IDS_GUID.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116015): https://edk2.groups.io/g/devel/message/116015
Mute This Topic: https://groups.io/mt/104512930/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 01/23] OvmfPkg/BaseMemEncryptLib: Fix error check from AsmRmpAdjust()

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:40AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> The AsmRmpAdjust() function returns a UINT32, however in SevSnpIsVmpl0()
> the return value is checked with EFI_ERROR() when it should just be
> compared to 0. Fix the error check.
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116014): https://edk2.groups.io/g/devel/message/116014
Mute This Topic: https://groups.io/mt/104512928/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-test v2] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c

[Chao Li]

Hi Edhay,

OK, thank you!


Thanks,
Chao
On 2024/2/27 17:19, G Edhaya Chandran wrote:


Hi Li Chao,

    As per the maintenance process, I will raise a PR on your behalf 
based on your patch.


And will later close your PR.

With Warm Regards,
Edhay

*From:* Chao Li 
*Sent:* Tuesday, February 27, 2024 1:34 PM
*To:* devel@edk2.groups.io; G Edhaya Chandran 
*Subject:* Re: [edk2-devel] [edk2-test v2] SctPkg: Fixed a pinter 
error in DevicePathFromTextBBTestCoverage.c


Hi Edhaya,

Thanks for you review, I have created a PR on Github: 
https://github.com/tianocore/edk2-test/pull/87


Thanks,
Chao

On 2024/2/27 15:50, G Edhaya Chandran wrote:

Hi Li Chao, Thank you for the solution.
Reviewed OK.

Reviewed-by: G Edhaya Chandran 




IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended 
recipient, please notify the sender immediately and do not disclose 
the contents to any other person, use it for any purpose, or store or 
copy the information in any medium. Thank you. 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116013): https://edk2.groups.io/g/devel/message/116013
Mute This Topic: https://groups.io/mt/104598721/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-test v2] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c

[G Edhaya Chandran]

Hi Li Chao,

As per the maintenance process, I will raise a PR on your behalf based on 
your patch.
And will later close your PR.

With Warm Regards,
Edhay


From: Chao Li 
Sent: Tuesday, February 27, 2024 1:34 PM
To: devel@edk2.groups.io; G Edhaya Chandran 
Subject: Re: [edk2-devel] [edk2-test v2] SctPkg: Fixed a pinter error in 
DevicePathFromTextBBTestCoverage.c


Hi Edhaya,

Thanks for you review, I have created a PR on Github: 
https://github.com/tianocore/edk2-test/pull/87

Thanks,
Chao
On 2024/2/27 15:50, G Edhaya Chandran wrote:
Hi Li Chao, Thank you for the solution.
Reviewed OK.

Reviewed-by: G Edhaya Chandran 


IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116012): https://edk2.groups.io/g/devel/message/116012
Mute This Topic: https://groups.io/mt/104598721/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 4/4] OvmfPkg/PlatformPei: log pei memory cap details

[Gerd Hoffmann]

On Thu, Feb 15, 2024 at 09:24:41AM +0100, Laszlo Ersek wrote:
> 
> Reviewed-by: Laszlo Ersek 
> 
> This series is now ready for merging, as far as I'm concerned (thanks
> for the updates in the other patches too, I've re-checked them). Please
> send a reminder after the stable tag.

Friendly post-freeze ping ;)

thanks & take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116011): https://edk2.groups.io/g/devel/message/116011
Mute This Topic: https://groups.io/mt/104350494/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 1/1] UefiPayloadPkg: Make Dsc accomodative of other archs

[Dhaval Sharma]

Current DSC files contains a lot of files which are
specific to X86 arch. Need to move around files under
arch specific sections.

Cc: Guo Dong 
Cc: Sean Rhodes 
Cc: James Lu 
Cc: Gua Guo 
Signed-off-by: Dhaval Sharma 
Reviewed-by: Gua Guo 
---

Notes:
v1:
- Updated RB tab

 UefiPayloadPkg/UefiPayloadPkg.dsc | 48 +++-
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc 
b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 95417cec6aff..433fb51a5695 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -259,24 +259,6 @@ [LibraryClasses]
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
-
-  #
-  # CPU
-  #
-  MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
-  LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
-  MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
-  CpuPageTableLib|UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableLib.inf
-  SmmCpuSyncLib|UefiCpuPkg/Library/SmmCpuSyncLib/SmmCpuSyncLib.inf
-
-  #
-  # Platform
-  #
-!if $(CPU_TIMER_LIB_ENABLE) == TRUE && $(UNIVERSAL_PAYLOAD) == TRUE
-  TimerLib|UefiCpuPkg/Library/CpuTimerLib/BaseCpuTimerLib.inf
-!else
-  TimerLib|UefiPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.inf
-!endif
   ResetSystemLib|UefiPayloadPkg/Library/ResetSystemLib/ResetSystemLib.inf
 !if $(USE_CBMEM_FOR_CONSOLE) == TRUE
   SerialPortLib|UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf
@@ -339,6 +321,22 @@ [LibraryClasses.common]
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
 !endif
 
+[LibraryClasses.X64]
+  #
+  # CPU
+  #
+  MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
+  LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
+  MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
+  IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf
+!if $(CPU_TIMER_LIB_ENABLE) == TRUE && $(UNIVERSAL_PAYLOAD) == TRUE
+  TimerLib|UefiCpuPkg/Library/CpuTimerLib/BaseCpuTimerLib.inf
+!else
+  TimerLib|UefiPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.inf
+!endif
+  
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
+  CpuPageTableLib|UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableLib.inf
+
 [LibraryClasses.common.SEC]
   HobLib|UefiPayloadPkg/Library/PayloadEntryHobLib/HobLib.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
@@ -355,6 +353,8 @@ [LibraryClasses.common.DXE_CORE]
   
MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
   
ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
   
ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
+
+[LibraryClasses.X64.DXE_CORE]
 !if $(SOURCE_DEBUG_ENABLE)
   DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
 !endif
@@ -368,6 +368,10 @@ [LibraryClasses.common.DXE_DRIVER]
   
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   
ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
   
ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
+
+[LibraryClasses.X64.DXE_DRIVER]
+  
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
+  MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
 !if $(SOURCE_DEBUG_ENABLE)
   DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
 !endif
@@ -394,7 +398,7 @@ 
[LibraryClasses.common.UEFI_DRIVER,LibraryClasses.common.UEFI_APPLICATION]
   PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
 !endif
 
-[LibraryClasses.common.SMM_CORE]
+[LibraryClasses.X64.SMM_CORE]
 !if $(SMM_SUPPORT) == TRUE
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
   
SmmServicesTableLib|MdeModulePkg/Library/PiSmmCoreSmmServicesTableLib/PiSmmCoreSmmServicesTableLib.inf
@@ -408,7 +412,7 @@ [LibraryClasses.common.SMM_CORE]
 !endif
 !endif
 
-[LibraryClasses.common.DXE_SMM_DRIVER]
+[LibraryClasses.X64.DXE_SMM_DRIVER]
 !if $(SMM_SUPPORT) == TRUE
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
 
@@ -438,13 +442,15 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
 #
 

 [PcdsFeatureFlag]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   ## This PCD specified whether ACPI SDT protocol is installed.
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdHiiOsRuntimeSupport|FALSE
   

[edk2-devel] [PATCH v1 0/1] UefiPayloadPkg: Make Dsc accomodative of other archs

[Dhaval Sharma]

Current DSC files contains a lot of files which are
specific to X86 arch. Need to move around files under
arch specific sections.

Cc: Guo Dong 
Cc: Sean Rhodes 
Cc: James Lu 
Cc: Gua Guo 
Signed-off-by: Dhaval Sharma 

Dhaval (1):
  UefiPayloadPkg: Make Dsc accomodative of other archs

 UefiPayloadPkg/UefiPayloadPkg.dsc | 48 +++-
 1 file changed, 27 insertions(+), 21 deletions(-)

-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116009): https://edk2.groups.io/g/devel/message/116009
Mute This Topic: https://groups.io/mt/104564121/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-test v2] SctPkg: Fixed a pinter error in DevicePathFromTextBBTestCoverage.c

[Chao Li]

Hi Edhaya,

Thanks for you review, I have created a PR on Github: 
https://github.com/tianocore/edk2-test/pull/87



Thanks,
Chao
On 2024/2/27 15:50, G Edhaya Chandran wrote:

Hi Li Chao, Thank you for the solution.
Reviewed OK.

Reviewed-by: G Edhaya Chandran 




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116008): https://edk2.groups.io/g/devel/message/116008
Mute This Topic: https://groups.io/mt/104598721/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-