date:20230608

Ard,
https://github.com/tianocore/edk2/blob/8314a85893f5b75baa0031a5138028188a626243/UefiCpuPkg/SecCore/SecMain.c#L637
 is changed by @Wu, Jiaxin to create page table in permanent memory.

(I didn't check your patch in detail. But it sounds like you try to do the same 
thing that above code has already done.)

Thanks,
Ray

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Friday, June 9, 2023 1:23 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao,
> Jiewen ; Gerd Hoffmann ; Taylor
> Beebe ; Oliver Smith-Denny ;
> Bi, Dandan ; Tan, Dun ; Gao,
> Liming ; Kinney, Michael D
> ; Michael Kubacki
> ; Dong, Eric ; Kumar,
> Rahul R ; Kun Qin 
> Subject: [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in
> permanent DRAM
> 
> Currently, we rely on the logic in DXE IPL to create new page tables
> from scratch when executing in X64 mode, which means that we run with
> the initial page tables all throughout PEI, and never enable protections
> such as the CPU stack guard, even though the logic is already in place
> for IA32.
> 
> So let's enable the existing logic for X64 as well. This will permit us
> to apply stricter memory permissions to code and data allocations, as
> well as the stack, when executing in PEI. It also makes the DxeIpl logic
> redundant, and should allow us to make the PcdDxeIplBuildPageTables
> feature PCD limited to IA32 DxeIpl loading the x64 DXE core.
> 
> When running in long mode, use the same logic that DxeIpl uses to
> determine the size of the address space, whether or not to use 1 GB leaf
> entries and whether or not to use 5 level paging. Note that in long
> mode, PEI is entered with paging enabled, and given that switching
> between 4 and 5 levels of paging is not currently supported without
> dropping out of 64-bit mode temporarily, all we can do is carry on
> without changing the number of levels.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  UefiCpuPkg/CpuMpPei/CpuMpPei.inf |   2 +
>  UefiCpuPkg/CpuMpPei/CpuPaging.c  | 163 
>  2 files changed, 139 insertions(+), 26 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> index 865be5627e8551ee..77eecaa0ea035b38 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> @@ -65,6 +65,8 @@ [Ppis]
>  [Pcd]
> 
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMas
> k## CONSUMES
> 
>gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ##
> CONSUMES
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable  ##
> SOMETIMES_CONSUMES
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable  ##
> SOMETIMES_CONSUMES
> 
>gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList  ##
> SOMETIMES_CONSUMES
> 
>gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize##
> SOMETIMES_CONSUMES
> 
>gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize   ##
> SOMETIMES_CONSUMES
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index 175e47ccd737a0c1..2a901e44253434c2 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -302,7 +302,7 @@ ConvertMemoryPageAttributes (
>  return RETURN_INVALID_PARAMETER;
> 
>}
> 
> 
> 
> -  MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_UINT32;
> 
> +  MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_ADDRESS;
> 
>if ((BaseAddress > MaximumAddress) ||
> 
>(Length > MaximumAddress) ||
> 
>(BaseAddress > MaximumAddress - (Length - 1)))
> 
> @@ -350,16 +350,91 @@ ConvertMemoryPageAttributes (
>return RETURN_SUCCESS;
> 
>  }
> 
> 
> 
> +/*
> 
> + * Get physical address bits supported.
> 
> + *
> 
> + * @return The number of supported physical address bits.
> 
> + */
> 
> +STATIC
> 
> +UINT8
> 
> +GetPhysicalAddressBits (
> 
> +  VOID
> 
> +  )
> 
> +{
> 
> +  EFI_HOB_CPU  *Hob;
> 
> +  UINT32   RegEax;
> 
> +
> 
> +  Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
> 
> +  if (Hob != NULL) {
> 
> +return Hob->SizeOfMemorySpace;
> 
> +  }
> 
> +
> 
> +  AsmCpuid (0x8000, &RegEax, NULL, NULL, NULL);
> 
> +  if (RegEax >= 0x8008) {
> 
> +AsmCpuid (0x8008, &RegEax, NULL, NULL, NULL);
> 
> +return (UINT8)RegEax;
> 
> +  }
> 
> +
> 
> +  return 36;
> 
> +}
> 
> +
> 
> +/*
> 
> + * Determine and return the paging mode to be used in long mode, based on
> PCD
> 
> + * configuration and CPU support for 1G leaf descriptors and 5 level paging.
> 
> + *
> 
> + * @return The paging mode
> 
> + */
> 
> +STATIC
> 
> +PAGING_MODE
> 
> +GetPagingMode (
> 
> +  VOID
> 
> +  )
> 
> +{
> 
> +  BOOLEAN   Page5LevelSupport;
> 
> +  BOOLEAN   Page1GSupport;
> 
> +  UINT32RegEax;
> 
> +  UINT32RegEdx;
> 
> +  IA32_CR4  Cr4;
> 
> +
> 
> +  Cr4.UintN = AsmReadCr4 ();
> 
> +  Page5LevelSupport = (Cr4.Bits.LA57 != 0);
> 
> +  ASSERT (PcdGetBool (PcdUse5LevelPageTable) =

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM

On Thu, 8 Jun 2023 at 21:32, Michael Kubacki
 wrote:
>
> I think Sean's point aligns more closely with traditional PI boot phase
> separation goals. Btw, in the project we discussed, this issue was meant
> to capture the DXE memory protection init dependencies -
> https://github.com/tianocore/edk2/issues/4502 if someone would like to
> update that at some point.
>

Yeah, I think that makes sense. But the current status quo (on X64) is
to remove *all* protections when handing over to DXE, and so dropping
that and running with whatever PEI left behind is hardly going to be
worse. (and this is what we do on ARM)

But I agree that it makes sense for these manipulations to be scoped.
So we might manage a separate set of shadow page tables in the CPU
PEIM that also produces the memattr PPI, and manipulations can apply
to the active set only or to both sets (for, e.g., the DXE core, DXE
IPL and DXE mode stack).

That would at least permit use to drop the current kludge in DxeIpl,
which only knows how to create an unrestricted 1:1 mapping of the
entire address space, with 1x a NX mapping (for the stack) and 1x a
non-encrypted mapping (for the GHCB page on confidential VMs).

It also provides a better basis for architectures to carry their own
specific logic for this, instead of having a subdirectory for each
arch under DxeIpl/



> On 6/8/2023 2:27 PM, Sean wrote:
> > On 6/8/2023 10:48 AM, Ard Biesheuvel wrote:
> >> On Thu, 8 Jun 2023 at 19:39, Oliver Smith-Denny
> >>  wrote:
> >>> On 6/8/2023 10:23 AM, Ard Biesheuvel wrote:
>  Currently, we rely on the logic in DXE IPL to create new page tables
>  from scratch when executing in X64 mode, which means that we run with
>  the initial page tables all throughout PEI, and never enable
>  protections
>  such as the CPU stack guard, even though the logic is already in place
>  for IA32.
> 
>  So let's enable the existing logic for X64 as well. This will permit us
>  to apply stricter memory permissions to code and data allocations, as
>  well as the stack, when executing in PEI. It also makes the DxeIpl
>  logic
>  redundant, and should allow us to make the PcdDxeIplBuildPageTables
>  feature PCD limited to IA32 DxeIpl loading the x64 DXE core.
> 
>  When running in long mode, use the same logic that DxeIpl uses to
>  determine the size of the address space, whether or not to use 1 GB
>  leaf
>  entries and whether or not to use 5 level paging. Note that in long
>  mode, PEI is entered with paging enabled, and given that switching
>  between 4 and 5 levels of paging is not currently supported without
>  dropping out of 64-bit mode temporarily, all we can do is carry on
>  without changing the number of levels.
> 
> >>> I certainly agree with extending the ability to have memory protections
> >>> in PEI (and trying to unify across x86 and ARM (and beyond :)).
> >>>
> >>> A few things I am trying to understand:
> >>>
> >>> Does ARM today rebuild the page table in DxeIpl? Or is it using an
> >>> earlier built page table?
> >>>
> >> No. Most platforms run without any page tables until the permanent
> >> memory is installed, at which point it essentially maps what the
> >> platform describes as device memory and as normal memory.
> >>
> >>
> >>> If I understand your proposal correctly, with the addition of this
> >>> patch, you are suggesting we can drop creating new page tables in DxeIpl
> >>> and use only one page table throughout.
> >> Yes.
> >>
> >>> Again, I like the idea of having
> >>> mapped memory protections that continue through, but do you have
> >>> concerns that we may end up with garbage from PEI in DXE in the page
> >>> table? For OEMs, they may not control PEI and therefore be at the whim
> >>> of another's PEI page table. Would you envision the GCD gets built from
> >>> the existing page table or that the GCD gets built according to resource
> >>> descriptor HOBs and DxeCore ensures that the page table reflects what
> >>> the HOBs indicated?
> >>>
> >> If there is a reason to start with a clean slate when DxeIpl hands
> >> over to DXE core, I'd prefer that to be a conscious decision rather
> >> than a consequence of the X64 vs IA32 legacy.
> >>
> >> I think you can make a case for priming the GCD map based on resource
> >> descriptors rather than current mappings, with the exception of DXE
> >> core itself and the DXE mode stack. But I'd like to understand better
> >> what we think might be wrong with the page tables as PEI leaves them.
> >
> >
> > On many platforms there are different "owners" for these different parts
> > of firmware code.  The PEI phase is a place where the Silicon vendor and
> > Platform teams must work together.  The Dxe Phase may have a different
> > set of partners.  Industry trends definitely show more silicon vendor
> > driven diversity in the PEI phase of the boot process and with this
> > diversity it is much harder to make solid assumption

Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/Variable: TcgMorLockSmm Key Mismatch changes lock state


Acked-by: Michael Kubacki 

Inline code comment below.

On 4/12/2023 5:25 PM, Abhimanyu Singh wrote:

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

Inside TcgMorLockSmm.c, the SetVariableCheckHandlerMorLock() function
contains a scenario to prevent a possible dictionary attack on the MorLock
Key in accordance with the TCG Platform Reset Mitigation Spec v1.10.

The mechanism to prevent this attack must also change the MorLock Variable
Value to 0x01 to indicate Locked Without Key.

Cc: Jian J Wang 
Cc: Liming Gao 
Signed-off-by: Abhi Singh 
---
  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
index da1105ff073e..a76db18ef877 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
@@ -312,6 +312,10 @@ SetVariableCheckHandlerMorLock (
mMorLockState= MorLockStateLocked;

mMorLockKeyEmpty = TRUE;

ZeroMem (mMorLockKey, sizeof (mMorLockKey));

+  //

+  // Update value to reflect locked without key

+  //

+  SetMorLockVariable (MOR_LOCK_DATA_LOCKED_WITHOUT_KEY);


I know the TCG Reset Attack Mitigation Specification requires 
EFI_ACCESS_DENIED to be returned from this function in this case but 
SetMorLockVariable() returns a status code.


I suggest capturing that followed by an ASSERT_EFI_ERROR (Status) to at 
least help raise visibility of unexpected errors in builds with asserts 
enabled.




return EFI_ACCESS_DENIED;

  }

}




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105958): https://edk2.groups.io/g/devel/message/105958
Mute This Topic: https://groups.io/mt/98228898/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] MdeModulePkg/Variable: TcgMorLockSmm Key Mismatch changes lock state

2023-06-08 Thread Abhimanyu Singh

Jian and Liming,

I was wondering if y'all could review this patch? It is a small change so 
hopefully there is not much downtime to review/test.

Thank you,
Abhi


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

[edk2-devel] [PATCH v1 2/2] MdeModulePkg: Variable: Introduce MM based variable read service in PEI

From: Kun Qin 

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

This change introduced the Standalone MM based variable read capability
in PEI phase for applicable platforms (such as ARM platforms).

Similar to the x86 counterpart, MM communicate PPI is used to request
variable information from Standalone MM environment.

Cc: Hao A Wu 
Cc: Liming Gao 
Cc: Jian J Wang 

Co-authored-by: Ronny Hansen 
Co-authored-by: Shriram Masanamuthu Chinnathurai 
Co-authored-by: Preshit Harlikar 
Signed-off-by: Kun Qin 
---
 MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.c   | 381 

 MdeModulePkg/MdeModulePkg.dsc   |   1 +
 MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.h   | 134 +++
 MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.inf |  43 +++
 4 files changed, 559 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.c 
b/MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.c
new file mode 100644
index ..4f937e22e89e
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.c
@@ -0,0 +1,381 @@
+/** @file -- MmVariablePei.c
+  Provides interface for reading Secure System Variables during PEI.
+
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "MmVariablePei.h"
+
+#define MM_VARIABLE_COMM_BUFFER_OFFSET  (SMM_COMMUNICATE_HEADER_SIZE + 
SMM_VARIABLE_COMMUNICATE_HEADER_SIZE)
+
+//
+// Module globals
+//
+EFI_PEI_READ_ONLY_VARIABLE2_PPI  mPeiSecureVariableRead = {
+  PeiMmGetVariable,
+  PeiMmGetNextVariableName
+};
+
+EFI_PEI_PPI_DESCRIPTOR  mPeiMmVariablePpi = {
+  (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEfiPeiReadOnlyVariable2PpiGuid,
+  &mPeiSecureVariableRead
+};
+
+/**
+  Entry point of PEI Secure Variable read driver
+
+  @param  FileHandle   Handle of the file being invoked.
+   Type EFI_PEI_FILE_HANDLE is defined in 
FfsFindNextFile().
+  @param  PeiServices  General purpose services available to every PEIM.
+
+  @retval EFI_SUCCESS  If the interface could be successfully installed
+  @retval Others   Returned from PeiServicesInstallPpi()
+**/
+EFI_STATUS
+EFIAPI
+PeiMmVariableInitialize (
+  IN   EFI_PEI_FILE_HANDLE  FileHandle,
+  IN CONST EFI_PEI_SERVICES **PeiServices
+  )
+{
+  return PeiServicesInstallPpi (&mPeiMmVariablePpi);
+}
+
+/**
+  Helper function to populate MM communicate header and variable communicate 
header
+  and then communicate to PEI.
+
+  @param[in, out] CommunicateBuffer   Size of the variable name.
+  @param[in]  CommunicateBufferSize   The entire buffer size to be sent to 
MM.
+  @param[in]  FunctionThe MM variable function value.
+
+  @retval EFI_INVALID_PARAMETER  Invalid parameter.
+  @retval EFI_SUCCESSFind the specified variable.
+  @retval Others Errors returned by MM communicate or 
variable service.
+
+**/
+EFI_STATUS
+PopulateHeaderAndCommunicate (
+  IN OUT  UINT8  *CommunicateBuffer,
+  IN UINTN   CommunicateBufferSize,
+  IN UINTN   Function
+  )
+{
+  EFI_STATUS   Status;
+  EFI_PEI_MM_COMMUNICATION_PPI *MmCommunicationPpi;
+  EFI_MM_COMMUNICATE_HEADER*MmCommunicateHeader;
+  SMM_VARIABLE_COMMUNICATE_HEADER  *MmVarCommsHeader;
+
+  // Minimal sanity check
+  if ((CommunicateBuffer == NULL) ||
+  (CommunicateBufferSize < MM_VARIABLE_COMM_BUFFER_OFFSET))
+  {
+Status = EFI_INVALID_PARAMETER;
+DEBUG ((DEBUG_ERROR, "%a: Invalid incoming parameters: %p and 0x%x\n", 
__func__, CommunicateBuffer, CommunicateBufferSize));
+goto Exit;
+  }
+
+  if ((Function != SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME) &&
+  (Function != SMM_VARIABLE_FUNCTION_GET_VARIABLE))
+  {
+Status = EFI_INVALID_PARAMETER;
+DEBUG ((DEBUG_ERROR, "%a: Invalid function value: 0x%x\n", __func__, 
Function));
+goto Exit;
+  }
+
+  Status = PeiServicesLocatePpi (&gEfiPeiMmCommunicationPpiGuid, 0, NULL, 
(VOID **)&MmCommunicationPpi);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a: Failed to locate PEI MM Communication PPI: 
%r\n", __func__, Status));
+goto Exit;
+  }
+
+  // Zero the entire Communication Buffer Header
+  MmCommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)CommunicateBuffer;
+
+  ZeroMem (MmCommunicateHeader, SMM_COMMUNICATE_HEADER_SIZE);
+
+  // Use gEfiSmmVariableProtocolGuid to request the MM variable service in 
Standalone MM
+  CopyMem ((VOID *)&MmCommunicateHeader->HeaderGuid, 
&gEfiSmmVariableProtocolGuid, sizeof (GUID));
+
+  // Program the MM header size
+  MmCommunicateHeader->MessageLength = CommunicateBufferSize - 
SMM_COMMUNICATE_HEADER_SIZE;
+
+  MmVarCommsHeader = (SMM_VARIABLE_COMMUNICATE_HEADER *)(CommunicateBuffer + 
SMM_COMMUNICATE_HEADER_SIZE);
+
+  // We are onl

[edk2-devel] [PATCH v1 0/2] Support MM based variable services in PEI for ARM

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

As of today, there has been a void in the variable service in PEI phase
on ARM systems that support PEI phase and standalone MM hosted variable
service.

This change adds the support through:
1. Add MM communication services in PEI phase for ARM platforms. This
module is based on SMC calls to standalone MM environments, similar to
"ArmPkg/Drivers/MmCommunicationDxe".

2. A service module that installs `gEfiPeiReadOnlyVariable2PpiGuid` based
on step 1. Note that this driver will not have special dependency on ARM
specific code, thus will be ideally added to MdeModulePkg.

Patch v1 branch: https://github.com/kuqin12/edk2/tree/arm_var_pei_v1

Cc: Hao A Wu 
Cc: Liming Gao 
Cc: Jian J Wang 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 

Kun Qin (2):
  ArmPkg: MmCommunicationPei: Introduce MM communicate in PEI
  MdeModulePkg: Variable: Introduce MM based variable read service in
PEI

 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c  | 178 +
 MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.c   | 381 

 ArmPkg/ArmPkg.dsc   |   2 +
 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.h  |  76 
 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf|  41 +++
 MdeModulePkg/MdeModulePkg.dsc   |   1 +
 MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.h   | 134 +++
 MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.inf |  43 +++
 8 files changed, 856 insertions(+)
 create mode 100644 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
 create mode 100644 
MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.c
 create mode 100644 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.h
 create mode 100644 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf
 create mode 100644 
MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.h
 create mode 100644 
MdeModulePkg/Universal/Variable/MmVariablePei/MmVariablePei.inf

-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105954): https://edk2.groups.io/g/devel/message/105954
Mute This Topic: https://groups.io/mt/99415823/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/2] ArmPkg: MmCommunicationPei: Introduce MM communicate in PEI

From: Kun Qin 

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

This change introduced the MM communicate support in PEI phase for ARM
based platforms. Similar to the DXE counterpart, `PcdMmBufferBase` is
used as communicate buffer and SMC will be invoked to communicate to
TrustZone when MMI is requested.

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 

Co-authored-by: Ronny Hansen 
Co-authored-by: Shriram Masanamuthu Chinnathurai 
Co-authored-by: Preshit Harlikar 
Signed-off-by: Kun Qin 
---
 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c   | 178 

 ArmPkg/ArmPkg.dsc|   2 +
 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.h   |  76 +
 ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf |  41 +
 4 files changed, 297 insertions(+)

diff --git a/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c 
b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
new file mode 100644
index ..0f1f763a347d
--- /dev/null
+++ b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
@@ -0,0 +1,178 @@
+/** @file -- MmCommunicationPei.c
+  Provides an interface to send MM request in PEI
+
+  Copyright (c) 2016-2021, Arm Limited. All rights reserved.
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "MmCommunicationPei.h"
+
+//
+// Module globals
+//
+EFI_PEI_MM_COMMUNICATION_PPI  mPeiMmCommunication = {
+  MmCommunicationPeim
+};
+
+EFI_PEI_PPI_DESCRIPTOR  mPeiMmCommunicationPpi = {
+  (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEfiPeiMmCommunicationPpiGuid,
+  &mPeiMmCommunication
+};
+
+/**
+  Entry point of PEI MM Communication driver
+
+  @param  FileHandle   Handle of the file being invoked.
+   Type EFI_PEI_FILE_HANDLE is defined in 
FfsFindNextFile().
+  @param  PeiServices  General purpose services available to every PEIM.
+
+  @retval EFI_SUCCESS  If the interface could be successfully installed
+  @retval Others   Returned from PeiServicesInstallPpi()
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicationPeiInitialize (
+  IN   EFI_PEI_FILE_HANDLE  FileHandle,
+  IN CONST EFI_PEI_SERVICES **PeiServices
+  )
+{
+  return PeiServicesInstallPpi (&mPeiMmCommunicationPpi);
+}
+
+/**
+  MmCommunicationPeim
+  Communicates with a registered handler.
+  This function provides a service to send and receive messages from a 
registered UEFI service during PEI.
+
+  @param[in]  ThisThe EFI_PEI_MM_COMMUNICATION_PPI instance.
+  @param[in, out] CommBuffer  Pointer to the data buffer
+  @param[in, out] CommSizeThe size of the data buffer being passed in. 
On exit, the
+  size of data being returned. Zero if the 
handler does not
+  wish to reply with any data.
+
+  @retval EFI_SUCCESS The message was successfully posted.
+  @retval EFI_INVALID_PARAMETER   CommBuffer was NULL or *CommSize does not 
match
+  MessageLength + sizeof 
(EFI_MM_COMMUNICATE_HEADER).
+  @retval EFI_BAD_BUFFER_SIZE The buffer is too large for the MM 
implementation.
+  If this error is returned, the MessageLength 
field
+  in the CommBuffer header or the integer 
pointed by
+  CommSize, are updated to reflect the maximum 
payload
+  size the implementation can accommodate.
+  @retval EFI_ACCESS_DENIED   The CommunicateBuffer parameter or CommSize 
parameter,
+  if not omitted, are in address range that 
cannot be
+  accessed by the MM environment.
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicationPeim (
+  IN CONST EFI_PEI_MM_COMMUNICATION_PPI  *This,
+  IN OUT VOID*CommBuffer,
+  IN OUT UINTN   *CommSize
+  )
+{
+  EFI_MM_COMMUNICATE_HEADER  *CommunicateHeader;
+  ARM_SMC_ARGS   CommunicateSmcArgs;
+  EFI_STATUS Status;
+  UINTN  BufferSize;
+
+  Status = EFI_ACCESS_DENIED;
+  BufferSize = 0;
+
+  ZeroMem (&CommunicateSmcArgs, sizeof (ARM_SMC_ARGS));
+
+  // Check that our static buffer is looking good.
+  // We are using PcdMmBufferBase to transfer variable data.
+  // We are not using the full size of the buffer since there is a cost
+  // of copying data between Normal and Secure World.
+  ASSERT (PcdGet64 (PcdMmBufferSize) > 0 && PcdGet64 (PcdMmBufferBase) != 0);
+
+  //
+  // Check parameters
+  //
+  if (CommBuffer == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  // If the length of the CommBuffer is 0 then return the expected length.
+  // This case can be used by the consumer of this driver to find out the
+  // max size that can be used for allocating CommBuffer.
+  if ((CommSize != NULL) &&

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM


On 6/8/2023 1:23 PM, Ard Biesheuvel wrote:

Currently, we rely on the logic in DXE IPL to create new page tables
from scratch when executing in X64 mode, which means that we run with
the initial page tables all throughout PEI, and never enable protections
such as the CPU stack guard, even though the logic is already in place
for IA32.

So let's enable the existing logic for X64 as well. This will permit us
to apply stricter memory permissions to code and data allocations, as
well as the stack, when executing in PEI. It also makes the DxeIpl logic
redundant, and should allow us to make the PcdDxeIplBuildPageTables
feature PCD limited to IA32 DxeIpl loading the x64 DXE core.

When running in long mode, use the same logic that DxeIpl uses to
determine the size of the address space, whether or not to use 1 GB leaf
entries and whether or not to use 5 level paging. Note that in long
mode, PEI is entered with paging enabled, and given that switching
between 4 and 5 levels of paging is not currently supported without
dropping out of 64-bit mode temporarily, all we can do is carry on
without changing the number of levels.

Signed-off-by: Ard Biesheuvel 
---
  UefiCpuPkg/CpuMpPei/CpuMpPei.inf |   2 +
  UefiCpuPkg/CpuMpPei/CpuPaging.c  | 163 
  2 files changed, 139 insertions(+), 26 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
index 865be5627e8551ee..77eecaa0ea035b38 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
@@ -65,6 +65,8 @@ [Ppis]
  [Pcd]

gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask## 
CONSUMES

gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES

+  gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable  ## 
SOMETIMES_CONSUMES

+  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable  ## 
SOMETIMES_CONSUMES

gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList  ## 
SOMETIMES_CONSUMES

gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize## 
SOMETIMES_CONSUMES

gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize   ## 
SOMETIMES_CONSUMES

diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index 175e47ccd737a0c1..2a901e44253434c2 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -302,7 +302,7 @@ ConvertMemoryPageAttributes (
  return RETURN_INVALID_PARAMETER;

}

  


-  MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_UINT32;

+  MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_ADDRESS;

if ((BaseAddress > MaximumAddress) ||

(Length > MaximumAddress) ||

(BaseAddress > MaximumAddress - (Length - 1)))

@@ -350,16 +350,91 @@ ConvertMemoryPageAttributes (
return RETURN_SUCCESS;

  }

  


+/*

+ * Get physical address bits supported.

+ *

+ * @return The number of supported physical address bits.

+ */

+STATIC

+UINT8

+GetPhysicalAddressBits (

+  VOID

+  )

+{

+  EFI_HOB_CPU  *Hob;

+  UINT32   RegEax;

+

+  Hob = GetFirstHob (EFI_HOB_TYPE_CPU);

+  if (Hob != NULL) {

+return Hob->SizeOfMemorySpace;

+  }

+

+  AsmCpuid (0x8000, &RegEax, NULL, NULL, NULL);

+  if (RegEax >= 0x8008) {

+AsmCpuid (0x8008, &RegEax, NULL, NULL, NULL);

+return (UINT8)RegEax;

+  }

+

+  return 36;

+}

+


I saw a similar patch come through earlier today adding 
GetMaxPlatformAddressBits() to CpubLib - 
https://edk2.groups.io/g/devel/message/105909.




+/*

+ * Determine and return the paging mode to be used in long mode, based on PCD

+ * configuration and CPU support for 1G leaf descriptors and 5 level paging.

+ *

+ * @return The paging mode

+ */

+STATIC

+PAGING_MODE

+GetPagingMode (

+  VOID

+  )

+{

+  BOOLEAN   Page5LevelSupport;

+  BOOLEAN   Page1GSupport;

+  UINT32RegEax;

+  UINT32RegEdx;

+  IA32_CR4  Cr4;

+

+  Cr4.UintN = AsmReadCr4 ();

+  Page5LevelSupport = (Cr4.Bits.LA57 != 0);

+  ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);

+

+  Page1GSupport = FALSE;

+  if (PcdGetBool (PcdUse1GPageTable)) {

+AsmCpuid (0x8000, &RegEax, NULL, NULL, NULL);

+if (RegEax >= 0x8001) {

+  AsmCpuid (0x8001, NULL, NULL, NULL, &RegEdx);

+  if ((RegEdx & BIT26) != 0) {

+Page1GSupport = TRUE;

+  }

+}

+  }

+

+  if (Page5LevelSupport && Page1GSupport) {

+return Paging5Level1GB;

+  } else if (Page5LevelSupport) {

+return Paging5Level;

+  } else if (Page1GSupport) {

+return Paging4Level1GB;

+  } else {

+return Paging4Level;

+  }

+}

+

  /**

-  Enable PAE Page Table.

+  Enable Page Table.

  


-  @retval   EFI_SUCCESS   The PAE Page Table was enabled successfully.

-  @retval   EFI_OUT_OF_RESOURCES  The PAE Page Table could not be enabled due 
to lack of available memory.

+  @paramLongMode  Whether the execution m

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM

I think Sean's point aligns more closely with traditional PI boot phase 
separation goals. Btw, in the project we discussed, this issue was meant 
to capture the DXE memory protection init dependencies - 
https://github.com/tianocore/edk2/issues/4502 if someone would like to 
update that at some point.


Thanks,
Michael

On 6/8/2023 2:27 PM, Sean wrote:

On 6/8/2023 10:48 AM, Ard Biesheuvel wrote:

On Thu, 8 Jun 2023 at 19:39, Oliver Smith-Denny
 wrote:

On 6/8/2023 10:23 AM, Ard Biesheuvel wrote:

Currently, we rely on the logic in DXE IPL to create new page tables
from scratch when executing in X64 mode, which means that we run with
the initial page tables all throughout PEI, and never enable 
protections

such as the CPU stack guard, even though the logic is already in place
for IA32.

So let's enable the existing logic for X64 as well. This will permit us
to apply stricter memory permissions to code and data allocations, as
well as the stack, when executing in PEI. It also makes the DxeIpl 
logic

redundant, and should allow us to make the PcdDxeIplBuildPageTables
feature PCD limited to IA32 DxeIpl loading the x64 DXE core.

When running in long mode, use the same logic that DxeIpl uses to
determine the size of the address space, whether or not to use 1 GB 
leaf

entries and whether or not to use 5 level paging. Note that in long
mode, PEI is entered with paging enabled, and given that switching
between 4 and 5 levels of paging is not currently supported without
dropping out of 64-bit mode temporarily, all we can do is carry on
without changing the number of levels.


I certainly agree with extending the ability to have memory protections
in PEI (and trying to unify across x86 and ARM (and beyond :)).

A few things I am trying to understand:

Does ARM today rebuild the page table in DxeIpl? Or is it using an
earlier built page table?


No. Most platforms run without any page tables until the permanent
memory is installed, at which point it essentially maps what the
platform describes as device memory and as normal memory.



If I understand your proposal correctly, with the addition of this
patch, you are suggesting we can drop creating new page tables in DxeIpl
and use only one page table throughout.

Yes.


Again, I like the idea of having
mapped memory protections that continue through, but do you have
concerns that we may end up with garbage from PEI in DXE in the page
table? For OEMs, they may not control PEI and therefore be at the whim
of another's PEI page table. Would you envision the GCD gets built from
the existing page table or that the GCD gets built according to resource
descriptor HOBs and DxeCore ensures that the page table reflects what
the HOBs indicated?


If there is a reason to start with a clean slate when DxeIpl hands
over to DXE core, I'd prefer that to be a conscious decision rather
than a consequence of the X64 vs IA32 legacy.

I think you can make a case for priming the GCD map based on resource
descriptors rather than current mappings, with the exception of DXE
core itself and the DXE mode stack. But I'd like to understand better
what we think might be wrong with the page tables as PEI leaves them.



On many platforms there are different "owners" for these different parts 
of firmware code.  The PEI phase is a place where the Silicon vendor and 
Platform teams must work together.  The Dxe Phase may have a different 
set of partners.  Industry trends definitely show more silicon vendor 
driven diversity in the PEI phase of the boot process and with this 
diversity it is much harder to make solid assumptions about the 
execution environment.   We have also discussed in the past meeting that 
PEI should be configurable using different solutions given it isn't a 
place where unknown 3rd party compatibility is critical.  This means 
that PEI might have different requirements than DXE and thus the 
configuration inherited from PEI may not be compliant. Additionally, the 
code and driver mappings from PEI phase should not be relevant in DXE. 
Even with the same architecture being used these are different execution 
phases with different constructs.  Keeping the PEI code mapped will only 
lead to additional security and correctness challenges.  Finally, as an 
overarching theme of this project we have suggested we should not be 
coupling the various phases, their requirements, and their assumptions 
together.  You could just as easily apply this to DXE and SMM/MM.  These 
are all independent execution environments and the more we can provide 
simplification and consistency the better our chances are of getting 
correct implementations across the ecosystem.
















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

Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg/CpuMpPei: Print correct buffer size used for page table


Reviewed-by: Michael Kubacki 

A couple comments below.

On 6/8/2023 1:23 PM, Ard Biesheuvel wrote:

The DEBUG print that outputs the base and size of the page table
allocation always prints 0x0 for the size, given that BufferSize will be
updated by PageTableMap () and contain the unused allocation on return.

So move the DEBUG print right after the allocation.

Signed-off-by: Ard Biesheuvel 
---
  UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index b7ddb0005b6fbcac..175e47ccd737a0c1 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -396,6 +396,13 @@ EnablePaePageTable (
  return EFI_OUT_OF_RESOURCES;

}

  


+  DEBUG ((

+DEBUG_INFO,

+"EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",

+PageTable,

+BufferSize

+));

+


In the past, a point was made to improve portability between 32-bit and 
64-bit architectures by casting UINTN values to UINT64 and then printing 
them %Lx. If this is a DEBUG only change that might be worth adding as well.


In any case, can you please prefix the print specifier for BufferSize 
with "0x"?




Status = PageTableMap (&PageTable, PagingPae, Buffer, &BufferSize, 0, SIZE_4GB, 
&MapAttribute, &MapMask, NULL);

ASSERT_EFI_ERROR (Status);

if (EFI_ERROR (Status) || (PageTable == 0)) {

@@ -417,13 +424,6 @@ EnablePaePageTable (
//

AsmWriteCr0 (AsmReadCr0 () | BIT31);

  


-  DEBUG ((

-DEBUG_INFO,

-"EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",

-PageTable,

-BufferSize

-));

-

return Status;

  }

  




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

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM

2023-06-08 Thread Sean


On 6/8/2023 10:48 AM, Ard Biesheuvel wrote:

On Thu, 8 Jun 2023 at 19:39, Oliver Smith-Denny
 wrote:

On 6/8/2023 10:23 AM, Ard Biesheuvel wrote:

Currently, we rely on the logic in DXE IPL to create new page tables
from scratch when executing in X64 mode, which means that we run with
the initial page tables all throughout PEI, and never enable protections
such as the CPU stack guard, even though the logic is already in place
for IA32.

So let's enable the existing logic for X64 as well. This will permit us
to apply stricter memory permissions to code and data allocations, as
well as the stack, when executing in PEI. It also makes the DxeIpl logic
redundant, and should allow us to make the PcdDxeIplBuildPageTables
feature PCD limited to IA32 DxeIpl loading the x64 DXE core.

When running in long mode, use the same logic that DxeIpl uses to
determine the size of the address space, whether or not to use 1 GB leaf
entries and whether or not to use 5 level paging. Note that in long
mode, PEI is entered with paging enabled, and given that switching
between 4 and 5 levels of paging is not currently supported without
dropping out of 64-bit mode temporarily, all we can do is carry on
without changing the number of levels.


I certainly agree with extending the ability to have memory protections
in PEI (and trying to unify across x86 and ARM (and beyond :)).

A few things I am trying to understand:

Does ARM today rebuild the page table in DxeIpl? Or is it using an
earlier built page table?


No. Most platforms run without any page tables until the permanent
memory is installed, at which point it essentially maps what the
platform describes as device memory and as normal memory.



If I understand your proposal correctly, with the addition of this
patch, you are suggesting we can drop creating new page tables in DxeIpl
and use only one page table throughout.

Yes.


Again, I like the idea of having
mapped memory protections that continue through, but do you have
concerns that we may end up with garbage from PEI in DXE in the page
table? For OEMs, they may not control PEI and therefore be at the whim
of another's PEI page table. Would you envision the GCD gets built from
the existing page table or that the GCD gets built according to resource
descriptor HOBs and DxeCore ensures that the page table reflects what
the HOBs indicated?


If there is a reason to start with a clean slate when DxeIpl hands
over to DXE core, I'd prefer that to be a conscious decision rather
than a consequence of the X64 vs IA32 legacy.

I think you can make a case for priming the GCD map based on resource
descriptors rather than current mappings, with the exception of DXE
core itself and the DXE mode stack. But I'd like to understand better
what we think might be wrong with the page tables as PEI leaves them.



On many platforms there are different "owners" for these different parts 
of firmware code.  The PEI phase is a place where the Silicon vendor and 
Platform teams must work together.  The Dxe Phase may have a different 
set of partners.  Industry trends definitely show more silicon vendor 
driven diversity in the PEI phase of the boot process and with this 
diversity it is much harder to make solid assumptions about the 
execution environment.   We have also discussed in the past meeting that 
PEI should be configurable using different solutions given it isn't a 
place where unknown 3rd party compatibility is critical.  This means 
that PEI might have different requirements than DXE and thus the 
configuration inherited from PEI may not be compliant. Additionally, the 
code and driver mappings from PEI phase should not be relevant in DXE.  
Even with the same architecture being used these are different execution 
phases with different constructs.  Keeping the PEI code mapped will only 
lead to additional security and correctness challenges.  Finally, as an 
overarching theme of this project we have suggested we should not be 
coupling the various phases, their requirements, and their assumptions 
together.  You could just as easily apply this to DXE and SMM/MM.  These 
are all independent execution environments and the more we can provide 
simplification and consistency the better our chances are of getting 
correct implementations across the ecosystem.











-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105950): https://edk2.groups.io/g/devel/message/105950
Mute This Topic: https://groups.io/mt/99411875/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/3] SecurityPkg: SubClassTpm: Updated default value

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

This change updated the default value of TPM device subclass PCD to
`0x010E` in order to match the definition of EFI_PERIPHERAL_TPM
from PI specification v1.8.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
Reviewed-by: Liming Gao 
---

Notes:
v2:
- Added reviewed-by tag [Jiewen]
- Added reviewed-by tag [Liming]

 SecurityPkg/SecurityPkg.dec | 6 +++---
 SecurityPkg/SecurityPkg.uni | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 0a8042d63fe1..53aa7ec43557 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -308,10 +308,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmPlatformClass|0|UINT8|0x0006
 
   ## Progress Code for TPM device subclass definitions.
-  #  EFI_PERIPHERAL_TPM  = (EFI_PERIPHERAL | 0x000D) = 0x010D
+  #  EFI_PERIPHERAL_TPM  = (EFI_PERIPHERAL | 0x000E) = 0x010E
   # @Prompt Status Code for TPM device definitions
-  # @ValidList  0x8003 | 0x010D
-  
gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeSubClassTpmDevice|0x010D|UINT32|0x0007
+  # @ValidList  0x8003 | 0x010E
+  
gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeSubClassTpmDevice|0x010E|UINT32|0x0007
 
   ## Defines the IO port used to trigger a software System Management 
Interrupt (SMI).
   #  Used as the SMI Command IO port by security functionality that triggers a 
software SMI such
diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
index 68587304d779..6c28b8021333 100644
--- a/SecurityPkg/SecurityPkg.uni
+++ b/SecurityPkg/SecurityPkg.uni
@@ -169,7 +169,7 @@
 #string 
STR_gEfiSecurityPkgTokenSpaceGuid_PcdStatusCodeSubClassTpmDevice_PROMPT  
#language en-US "Status Code for TPM device definitions"
 
 #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdStatusCodeSubClassTpmDevice_HELP  
#language en-US "Progress Code for TPM device subclass definitions.\n"
-   
"EFI_PERIPHERAL_TPM  = (EFI_PERIPHERAL | 0x000D) = 
0x010D"
+   
"EFI_PERIPHERAL_TPM  = (EFI_PERIPHERAL | 0x000E) = 
0x010E"
 
 #string 
STR_gEfiSecurityPkgTokenSpaceGuid_PcdRsa2048Sha256PublicKeyBuffer_PROMPT  
#language en-US "One or more SHA 256 Hashes of RSA 2048 bit public keys used to 
verify Recovery and Capsule Update images"
 
-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105949): https://edk2.groups.io/g/devel/message/105949
Mute This Topic: https://groups.io/mt/99413159/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/3] MdePkg: PiStatusCode: Add new Host Software class Error Code to MdePkg

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

This change introduces a new error code definitions under Host Software
class according to PI specification v1.8.

The new error code definition will cover system reboot events under the
conditions of inconsistent memory map from one boot to another.

These error codes could provide helpful datapoints to OEMs to investigate
and prevent system failures in general.

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

Signed-off-by: Kun Qin 
Reviewed-by: Liming Gao 
---

Notes:
v2:
- Added reviewed-by tag [Liming]

 MdePkg/Include/Pi/PiStatusCode.h | 41 ++--
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/MdePkg/Include/Pi/PiStatusCode.h b/MdePkg/Include/Pi/PiStatusCode.h
index ef2aea7364bc..3222a082c550 100644
--- a/MdePkg/Include/Pi/PiStatusCode.h
+++ b/MdePkg/Include/Pi/PiStatusCode.h
@@ -965,26 +965,27 @@ typedef struct {
 /// These are shared by all subclasses.
 ///
 ///@{
-#define EFI_SW_EC_NON_SPECIFIC0x
-#define EFI_SW_EC_LOAD_ERROR  0x0001
-#define EFI_SW_EC_INVALID_PARAMETER   0x0002
-#define EFI_SW_EC_UNSUPPORTED 0x0003
-#define EFI_SW_EC_INVALID_BUFFER  0x0004
-#define EFI_SW_EC_OUT_OF_RESOURCES0x0005
-#define EFI_SW_EC_ABORTED 0x0006
-#define EFI_SW_EC_ILLEGAL_SOFTWARE_STATE  0x0007
-#define EFI_SW_EC_ILLEGAL_HARDWARE_STATE  0x0008
-#define EFI_SW_EC_START_ERROR 0x0009
-#define EFI_SW_EC_BAD_DATE_TIME   0x000A
-#define EFI_SW_EC_CFG_INVALID 0x000B
-#define EFI_SW_EC_CFG_CLR_REQUEST 0x000C
-#define EFI_SW_EC_CFG_DEFAULT 0x000D
-#define EFI_SW_EC_PWD_INVALID 0x000E
-#define EFI_SW_EC_PWD_CLR_REQUEST 0x000F
-#define EFI_SW_EC_PWD_CLEARED 0x0010
-#define EFI_SW_EC_EVENT_LOG_FULL  0x0011
-#define EFI_SW_EC_WRITE_PROTECTED 0x0012
-#define EFI_SW_EC_FV_CORRUPTED0x0013
+#define EFI_SW_EC_NON_SPECIFIC 0x
+#define EFI_SW_EC_LOAD_ERROR   0x0001
+#define EFI_SW_EC_INVALID_PARAMETER0x0002
+#define EFI_SW_EC_UNSUPPORTED  0x0003
+#define EFI_SW_EC_INVALID_BUFFER   0x0004
+#define EFI_SW_EC_OUT_OF_RESOURCES 0x0005
+#define EFI_SW_EC_ABORTED  0x0006
+#define EFI_SW_EC_ILLEGAL_SOFTWARE_STATE   0x0007
+#define EFI_SW_EC_ILLEGAL_HARDWARE_STATE   0x0008
+#define EFI_SW_EC_START_ERROR  0x0009
+#define EFI_SW_EC_BAD_DATE_TIME0x000A
+#define EFI_SW_EC_CFG_INVALID  0x000B
+#define EFI_SW_EC_CFG_CLR_REQUEST  0x000C
+#define EFI_SW_EC_CFG_DEFAULT  0x000D
+#define EFI_SW_EC_PWD_INVALID  0x000E
+#define EFI_SW_EC_PWD_CLR_REQUEST  0x000F
+#define EFI_SW_EC_PWD_CLEARED  0x0010
+#define EFI_SW_EC_EVENT_LOG_FULL   0x0011
+#define EFI_SW_EC_WRITE_PROTECTED  0x0012
+#define EFI_SW_EC_FV_CORRUPTED 0x0013
+#define EFI_SW_EC_INCONSISTENT_MEMORY_MAP  0x0014
 ///@}
 
 //
-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105947): https://edk2.groups.io/g/devel/message/105947
Mute This Topic: https://groups.io/mt/99413157/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/3] MdePkg: PiStatusCode: Add TPM subclass definition to MdePkg

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

This change introduces a new peripheral subclass definition from PI
specification v1.8.

The new subclass definition will cover system reboot events under the
status reports from Trusted Platform Modules (TPMs).

These definition could provide helpful datapoints to OEMs to analyze
system security state and healthiness, as well as avoid definition
collision with other existing peripheral subclass definitions.

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

Signed-off-by: Kun Qin 
Reviewed-by: Liming Gao 
---

Notes:
v2:
- Added reviewed-by tag [Liming]

 MdePkg/Include/Pi/PiStatusCode.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdePkg/Include/Pi/PiStatusCode.h b/MdePkg/Include/Pi/PiStatusCode.h
index 3222a082c550..6675431611cb 100644
--- a/MdePkg/Include/Pi/PiStatusCode.h
+++ b/MdePkg/Include/Pi/PiStatusCode.h
@@ -363,6 +363,7 @@ typedef struct {
 #define EFI_PERIPHERAL_LCD_DEVICE   (EFI_PERIPHERAL | 0x000B)
 #define EFI_PERIPHERAL_NETWORK  (EFI_PERIPHERAL | 0x000C)
 #define EFI_PERIPHERAL_DOCKING  (EFI_PERIPHERAL | 0x000D)
+#define EFI_PERIPHERAL_TPM  (EFI_PERIPHERAL | 0x000E)
 ///@}
 
 ///
-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105948): https://edk2.groups.io/g/devel/message/105948
Mute This Topic: https://groups.io/mt/99413158/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/3] Adding Status Code Definitions from PI Spec v1.8

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

This v2 series is a follow up of previously submitted patches:
https://edk2.groups.io/g/devel/message/105599

The v2 patches include `Reviewed-by` tags collected during the v1
round and no other changes.

Patch v2 branch: https://github.com/kuqin12/edk2/tree/pi_v1p8_v2

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 

Kun Qin (3):
  MdePkg: PiStatusCode: Add new Host Software class Error Code to MdePkg
  MdePkg: PiStatusCode: Add TPM subclass definition to MdePkg
  SecurityPkg: SubClassTpm: Updated default value

 MdePkg/Include/Pi/PiStatusCode.h | 42 ++--
 SecurityPkg/SecurityPkg.dec  |  6 +--
 SecurityPkg/SecurityPkg.uni  |  2 +-
 3 files changed, 26 insertions(+), 24 deletions(-)

-- 
2.40.1.windows.1



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

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM

On Thu, 8 Jun 2023 at 19:39, Oliver Smith-Denny
 wrote:
>
> On 6/8/2023 10:23 AM, Ard Biesheuvel wrote:
> > Currently, we rely on the logic in DXE IPL to create new page tables
> > from scratch when executing in X64 mode, which means that we run with
> > the initial page tables all throughout PEI, and never enable protections
> > such as the CPU stack guard, even though the logic is already in place
> > for IA32.
> >
> > So let's enable the existing logic for X64 as well. This will permit us
> > to apply stricter memory permissions to code and data allocations, as
> > well as the stack, when executing in PEI. It also makes the DxeIpl logic
> > redundant, and should allow us to make the PcdDxeIplBuildPageTables
> > feature PCD limited to IA32 DxeIpl loading the x64 DXE core.
> >
> > When running in long mode, use the same logic that DxeIpl uses to
> > determine the size of the address space, whether or not to use 1 GB leaf
> > entries and whether or not to use 5 level paging. Note that in long
> > mode, PEI is entered with paging enabled, and given that switching
> > between 4 and 5 levels of paging is not currently supported without
> > dropping out of 64-bit mode temporarily, all we can do is carry on
> > without changing the number of levels.
> >
>
> I certainly agree with extending the ability to have memory protections
> in PEI (and trying to unify across x86 and ARM (and beyond :)).
>
> A few things I am trying to understand:
>
> Does ARM today rebuild the page table in DxeIpl? Or is it using an
> earlier built page table?
>

No. Most platforms run without any page tables until the permanent
memory is installed, at which point it essentially maps what the
platform describes as device memory and as normal memory.


> If I understand your proposal correctly, with the addition of this
> patch, you are suggesting we can drop creating new page tables in DxeIpl
> and use only one page table throughout.

Yes.

> Again, I like the idea of having
> mapped memory protections that continue through, but do you have
> concerns that we may end up with garbage from PEI in DXE in the page
> table? For OEMs, they may not control PEI and therefore be at the whim
> of another's PEI page table. Would you envision the GCD gets built from
> the existing page table or that the GCD gets built according to resource
> descriptor HOBs and DxeCore ensures that the page table reflects what
> the HOBs indicated?
>

If there is a reason to start with a clean slate when DxeIpl hands
over to DXE core, I'd prefer that to be a conscious decision rather
than a consequence of the X64 vs IA32 legacy.

I think you can make a case for priming the GCD map based on resource
descriptors rather than current mappings, with the exception of DXE
core itself and the DXE mode stack. But I'd like to understand better
what we think might be wrong with the page tables as PEI leaves them.


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

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM

2023-06-08 Thread Oliver Smith-Denny


On 6/8/2023 10:23 AM, Ard Biesheuvel wrote:

Currently, we rely on the logic in DXE IPL to create new page tables
from scratch when executing in X64 mode, which means that we run with
the initial page tables all throughout PEI, and never enable protections
such as the CPU stack guard, even though the logic is already in place
for IA32.

So let's enable the existing logic for X64 as well. This will permit us
to apply stricter memory permissions to code and data allocations, as
well as the stack, when executing in PEI. It also makes the DxeIpl logic
redundant, and should allow us to make the PcdDxeIplBuildPageTables
feature PCD limited to IA32 DxeIpl loading the x64 DXE core.

When running in long mode, use the same logic that DxeIpl uses to
determine the size of the address space, whether or not to use 1 GB leaf
entries and whether or not to use 5 level paging. Note that in long
mode, PEI is entered with paging enabled, and given that switching
between 4 and 5 levels of paging is not currently supported without
dropping out of 64-bit mode temporarily, all we can do is carry on
without changing the number of levels.



I certainly agree with extending the ability to have memory protections
in PEI (and trying to unify across x86 and ARM (and beyond :)).

A few things I am trying to understand:

Does ARM today rebuild the page table in DxeIpl? Or is it using an
earlier built page table?

If I understand your proposal correctly, with the addition of this
patch, you are suggesting we can drop creating new page tables in DxeIpl
and use only one page table throughout. Again, I like the idea of having
mapped memory protections that continue through, but do you have
concerns that we may end up with garbage from PEI in DXE in the page
table? For OEMs, they may not control PEI and therefore be at the whim
of another's PEI page table. Would you envision the GCD gets built from
the existing page table or that the GCD gets built according to resource
descriptor HOBs and DxeCore ensures that the page table reflects what
the HOBs indicated?

Thanks,
Oliver


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

[edk2-devel] [PATCH 1/2] UefiCpuPkg/CpuMpPei: Print correct buffer size used for page table

The DEBUG print that outputs the base and size of the page table
allocation always prints 0x0 for the size, given that BufferSize will be
updated by PageTableMap () and contain the unused allocation on return.

So move the DEBUG print right after the allocation.

Signed-off-by: Ard Biesheuvel 
---
 UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index b7ddb0005b6fbcac..175e47ccd737a0c1 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -396,6 +396,13 @@ EnablePaePageTable (
 return EFI_OUT_OF_RESOURCES;
   }
 
+  DEBUG ((
+DEBUG_INFO,
+"EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",
+PageTable,
+BufferSize
+));
+
   Status = PageTableMap (&PageTable, PagingPae, Buffer, &BufferSize, 0, 
SIZE_4GB, &MapAttribute, &MapMask, NULL);
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status) || (PageTable == 0)) {
@@ -417,13 +424,6 @@ EnablePaePageTable (
   //
   AsmWriteCr0 (AsmReadCr0 () | BIT31);
 
-  DEBUG ((
-DEBUG_INFO,
-"EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",
-PageTable,
-BufferSize
-));
-
   return Status;
 }
 
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105942): https://edk2.groups.io/g/devel/message/105942
Mute This Topic: https://groups.io/mt/99411874/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] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM

Currently, we rely on the logic in DXE IPL to create new page tables
from scratch when executing in X64 mode, which means that we run with
the initial page tables all throughout PEI, and never enable protections
such as the CPU stack guard, even though the logic is already in place
for IA32.

So let's enable the existing logic for X64 as well. This will permit us
to apply stricter memory permissions to code and data allocations, as
well as the stack, when executing in PEI. It also makes the DxeIpl logic
redundant, and should allow us to make the PcdDxeIplBuildPageTables
feature PCD limited to IA32 DxeIpl loading the x64 DXE core.

When running in long mode, use the same logic that DxeIpl uses to
determine the size of the address space, whether or not to use 1 GB leaf
entries and whether or not to use 5 level paging. Note that in long
mode, PEI is entered with paging enabled, and given that switching
between 4 and 5 levels of paging is not currently supported without
dropping out of 64-bit mode temporarily, all we can do is carry on
without changing the number of levels.

Signed-off-by: Ard Biesheuvel 
---
 UefiCpuPkg/CpuMpPei/CpuMpPei.inf |   2 +
 UefiCpuPkg/CpuMpPei/CpuPaging.c  | 163 
 2 files changed, 139 insertions(+), 26 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
index 865be5627e8551ee..77eecaa0ea035b38 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
@@ -65,6 +65,8 @@ [Ppis]
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable  ## 
SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable  ## 
SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList  ## 
SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize## 
SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize   ## 
SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index 175e47ccd737a0c1..2a901e44253434c2 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -302,7 +302,7 @@ ConvertMemoryPageAttributes (
 return RETURN_INVALID_PARAMETER;
   }
 
-  MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_UINT32;
+  MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_ADDRESS;
   if ((BaseAddress > MaximumAddress) ||
   (Length > MaximumAddress) ||
   (BaseAddress > MaximumAddress - (Length - 1)))
@@ -350,16 +350,91 @@ ConvertMemoryPageAttributes (
   return RETURN_SUCCESS;
 }
 
+/*
+ * Get physical address bits supported.
+ *
+ * @return The number of supported physical address bits.
+ */
+STATIC
+UINT8
+GetPhysicalAddressBits (
+  VOID
+  )
+{
+  EFI_HOB_CPU  *Hob;
+  UINT32   RegEax;
+
+  Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
+  if (Hob != NULL) {
+return Hob->SizeOfMemorySpace;
+  }
+
+  AsmCpuid (0x8000, &RegEax, NULL, NULL, NULL);
+  if (RegEax >= 0x8008) {
+AsmCpuid (0x8008, &RegEax, NULL, NULL, NULL);
+return (UINT8)RegEax;
+  }
+
+  return 36;
+}
+
+/*
+ * Determine and return the paging mode to be used in long mode, based on PCD
+ * configuration and CPU support for 1G leaf descriptors and 5 level paging.
+ *
+ * @return The paging mode
+ */
+STATIC
+PAGING_MODE
+GetPagingMode (
+  VOID
+  )
+{
+  BOOLEAN   Page5LevelSupport;
+  BOOLEAN   Page1GSupport;
+  UINT32RegEax;
+  UINT32RegEdx;
+  IA32_CR4  Cr4;
+
+  Cr4.UintN = AsmReadCr4 ();
+  Page5LevelSupport = (Cr4.Bits.LA57 != 0);
+  ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);
+
+  Page1GSupport = FALSE;
+  if (PcdGetBool (PcdUse1GPageTable)) {
+AsmCpuid (0x8000, &RegEax, NULL, NULL, NULL);
+if (RegEax >= 0x8001) {
+  AsmCpuid (0x8001, NULL, NULL, NULL, &RegEdx);
+  if ((RegEdx & BIT26) != 0) {
+Page1GSupport = TRUE;
+  }
+}
+  }
+
+  if (Page5LevelSupport && Page1GSupport) {
+return Paging5Level1GB;
+  } else if (Page5LevelSupport) {
+return Paging5Level;
+  } else if (Page1GSupport) {
+return Paging4Level1GB;
+  } else {
+return Paging4Level;
+  }
+}
+
 /**
-  Enable PAE Page Table.
+  Enable Page Table.
 
-  @retval   EFI_SUCCESS   The PAE Page Table was enabled successfully.
-  @retval   EFI_OUT_OF_RESOURCES  The PAE Page Table could not be enabled due 
to lack of available memory.
+  @paramLongMode  Whether the execution mode is 64 bit
+
+  @retval   EFI_SUCCESS   The Page Table was enabled successfully.
+  @retval   EFI_OUT_OF_RESOURCES  The Page Table could not be enabled due to 
lack of available memory.
 
 **/
+STATIC
 EFI_STATUS
-EnablePaePageTable (
-  VOID
+EnablePageTable (
+  IN  BOOLEAN  LongMode
   )
 {
   EFI_STA

[edk2-devel] [PATCH 0/2] UefiCpuPkg/CpuMpPei X64: reallocate page tables in PEI

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

Take a step towards enabling a generic approach to manage memory
permissions in PEI, by wiring up the existing IA32 page table creation
logic in CpuMpPei for X64 as well. This will enable future work to expose
a PPI that is available throughout PEI to manage memory permissions in a
generic manner across architectures.

The DxeIpl that implements this logic today will be made redundant by
this, and we should be able to retire it once the replacement pieces are
all in place.

Cc: Ray Ni 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Taylor Beebe 
Cc: Oliver Smith-Denny 
Cc: Dandan Bi 
Cc: Dun Tan 
Cc: Liming Gao 
Cc: "Kinney, Michael D" 
Cc: Michael Kubacki 
Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Kun Qin 

Ard Biesheuvel (2):
  UefiCpuPkg/CpuMpPei: Print correct buffer size used for page table
  UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM

 UefiCpuPkg/CpuMpPei/CpuMpPei.inf |   2 +
 UefiCpuPkg/CpuMpPei/CpuPaging.c  | 187 
 2 files changed, 151 insertions(+), 38 deletions(-)

-- 
2.39.2



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

Re: [edk2-devel] Using Depex for Standalone MM drivers on ARM

Forgot to add applicable maintainers earlier..

It would be great if StandaloneMmPkg maintainers could shed some light here.

Thanks,
Kun

On 6/6/2023 5:57 PM, Kun Qin via groups.io wrote:

Hi all,

We found an issue a while back on ARM systems, where the Standalone MM
drivers with Depex specified will run into a hang when TFA hands off to
Standalone MM core. (https://bugzilla.tianocore.org/show_bug.cgi?id=3883)

After some debugging, the logic in the ticket still seems to be true
until today:

1. The UEFI Standalone MM partition needs to setup Standalone MM core to
be branched off that the beginning of this FV by manipulating the build
rules. Example:
https://github.com/tianocore/edk2-platforms/blob/93a71a67fd80bbc5baf0708ba75e73696b4a1c67/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf#LL88C1-L89C1

2. During build time, the MM partition was initialized to code pages
through `ARM_SP_IMAGE_MMAP` as part of `plat_arm_mmap` see here:
https://github.com/ARM-software/arm-trusted-firmware/blob/a0f3b552cfa45258099170c83f79619b2dbd7b9b/include/plat/arm/common/arm_spm_def.h#LL36C11-L36C11,

which was later used for initial setup.

3. During BL31 setup step, the EL1 jump point will be updated to
`sp_image_base` from `plat_get_secure_partition_boot_info`:
https://github.com/ARM-software/arm-trusted-firmware/blob/a0f3b552cfa45258099170c83f79619b2dbd7b9b/services/std_svc/spm/spm_mm/spm_mm_setup.c#LL45C29-L45C42

which was initialized to `ARM_SP_IMAGE_BASE`.

4. After demoting to Secure EL1, the FD will first branch off to
standalone core entrypoint:
https://github.com/tianocore/edk2/blob/ded0b489af09cde5afa05d74acdb12cd4b4f8394/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c#L319.

Standalone core entrypoint will execute in place of FV for the first few
lines. Then the core will locate itself in reported buffer SpImageBase
(which is also the FV image buffer location) and update the page
attribute accordingly
(https://github.com/tianocore/edk2/blob/ded0b489af09cde5afa05d74acdb12cd4b4f8394/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c#LL386C12-L386C48).

5. The FV image buffer location (SpImageBase from UEFI, sp_image_base
from tf-a) is then published as an FV hob for further dispatches
(https://github.com/tianocore/edk2/blob/ded0b489af09cde5afa05d74acdb12cd4b4f8394/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c#LL84C1-L84C75),

which was then used for Depex tracking/evaluation during driver discovery
and dispatching process in
https://github.com/tianocore/edk2/blob/ded0b489af09cde5afa05d74acdb12cd4b4f8394/StandaloneMmPkg/Core/StandaloneMmCore.c#L646.

6. Depex discovered from FV during this process is cached in the
DriverEntry structure as pointers and added to the mDiscoveredList in
https://github.com/tianocore/edk2/blob/ded0b489af09cde5afa05d74acdb12cd4b4f8394/StandaloneMmPkg/Core/Dispatcher.c#L864

7. Later during Standalone MM dispatching, this Depex pointer will be
updated in place:
https://github.com/tianocore/edk2/blob/ded0b489af09cde5afa05d74acdb12cd4b4f8394/StandaloneMmPkg/Core/Dependency.c#LL256C14-L256C14,

which causes memory violation as we are writing to the code pages.

Could you please let me know if there is anything I missed on this path?

To resolve this issue, I think any one of the following changes
could be considered:

1. Updating the incoming FV region to be Data pages, in TF-A;
2. Copying this incoming region to a separate data buffer;
3. Allocating designated copy pools for Depex sections during dispatching
(and free them once done).

We could work around this by compressing the FV, but I think we should
still fix the issue, correct?

Any thoughts or suggestions on this would be appreciated.

Regards,
Kun

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

Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/XhciDxe: Fix Broken Timeouts

2023-06-08 Thread Henz, Patrick

Hi Jess,

Thank you for reporting this, it's certainly a bug. I'm planning to implement a 
fix.

Thanks,
Patrick Henz

-Original Message-
From: Jessica Clarke  
Sent: Wednesday, June 7, 2023 4:38 PM
To: h...@mx0a-002e3701.pphosted.com; Henz, Patrick ; 
devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/XhciDxe: Fix Broken 
Timeouts

On Wed, Sep 23, 2020 at 08:36 PM, Henz, Patrick wrote:

>
> From: Patrick Henz 
> 
> REF:INVALID URI REMOVED
> g.cgi?id=2948__;!!NpxR!iZG1WFNYYsLpsDRVpII4yD3VcV3qTNWYYBKIkjOpmdsJq4e
> cXWzlnbRiJf9vTEZUUu73BuQmhJJwUHocWQ$
> 
> Timeouts in the XhciDxe driver are taking longer than expected due to 
> the timeout loops not accounting for code execution time. As en 
> example, 5 second timeouts have been observed to take around 36 
> seconds to complete.
> Use SetTimer and Create/CheckEvent from Boot Services to determine 
> when timeout occurred.

This strategy doesn't work during ExitBootServices, as called from 
XhcExitBootService via XhcHaltHC. The net result is that, if the XHCI 
controller fails to halt upon clearing R/S, EDK2 hangs in an infinite loop 
(something I just had the joy of debugging).

Jess


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

[edk2-devel] Now: TianoCore Community Meeting EMEA/NAMO - Thursday, June 8, 2023 #cal-notice

2023-06-08 Thread Group Notification

*TianoCore Community Meeting EMEA/NAMO*

*When:*
Thursday, June 8, 2023
8:00am to 9:00am
(UTC-07:00) America/Los Angeles

*Where:*
Microsoft Teams meeting Join on your computer or mobile app Click here to join 
the meeting Meeting ID: 226 323 011 029 Passcode: hMRCj6 Download Teams | Join 
on the web Join with a video conferencing device te...@conf.intel.com Video 
Conference ID: 112 716 814 3 Alternate VTC instructions Learn More | Meeting 
options

*Organizer:* Miki Demeter

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1890539 )

*Description:*



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNlZDk4NjY5%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

Meeting ID: 226 323 011 029
Passcode: hMRCj6

Download Teams ( https://www.microsoft.com/en-us/microsoft-teams/download-app ) 
| Join on the web ( https://www.microsoft.com/microsoft-teams/join-a-meeting )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 112 716 814 3

Alternate VTC instructions ( 
https://conf.intel.com/teams/?conf=1127168143&ivr=teams&d=conf.intel.com&test=test_call
 )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=6e4ce4c4-1242-431b-9a51-92cd01a5df3c&tenantId=46c98d88-e344-4ed4-8496-4ed7712e255d&threadId=19_meeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNlZDk4NjY5@thread.v2&messageId=0&language=en-US
 )




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

[edk2-devel] Event: TianoCore Community Meeting EMEA/NAMO - Thursday, June 8, 2023 #cal-reminder

2023-06-08 Thread Group Notification

*Reminder: TianoCore Community Meeting EMEA/NAMO*

*When:*
Thursday, June 8, 2023
8:00am to 9:00am
(UTC-07:00) America/Los Angeles

*Where:*
Microsoft Teams meeting Join on your computer or mobile app Click here to join 
the meeting Meeting ID: 226 323 011 029 Passcode: hMRCj6 Download Teams | Join 
on the web Join with a video conferencing device te...@conf.intel.com Video 
Conference ID: 112 716 814 3 Alternate VTC instructions Learn More | Meeting 
options

*Organizer:* Miki Demeter

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1890539 )

*Description:*



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNlZDk4NjY5%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

Meeting ID: 226 323 011 029
Passcode: hMRCj6

Download Teams ( https://www.microsoft.com/en-us/microsoft-teams/download-app ) 
| Join on the web ( https://www.microsoft.com/microsoft-teams/join-a-meeting )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 112 716 814 3

Alternate VTC instructions ( 
https://conf.intel.com/teams/?conf=1127168143&ivr=teams&d=conf.intel.com&test=test_call
 )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=6e4ce4c4-1242-431b-9a51-92cd01a5df3c&tenantId=46c98d88-e344-4ed4-8496-4ed7712e255d&threadId=19_meeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNlZDk4NjY5@thread.v2&messageId=0&language=en-US
 )




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

Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity issue

2023-06-08 Thread Ranbir Singh

I mentioned similar approach in
https://bugzilla.tianocore.org/show_bug.cgi?id=4204#c1

Let me know if I should update the patch as Hao proposed -

if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "Set drive parameters Fail, Status = %r\n",
Status));
continue;
  }

On Thu, Jun 8, 2023 at 12:25 PM Wu, Hao A  wrote:

> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Ard
> > Biesheuvel
> > Sent: Monday, June 5, 2023 4:32 PM
> > To: Ranbir Singh 
> > Cc: devel@edk2.groups.io; pedro.falc...@gmail.com; Wu, Hao A
> > ; Ni, Ray 
> > Subject: Re: [edk2-devel] [PATCH 2/2]
> > MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity
> > issue
> >
> > On Mon, 5 Jun 2023 at 10:10, Ranbir Singh 
> > wrote:
> > >
> > > I counted myself as not the right person to decide what all to do if
> Status is
> > not successful. Adding the DEBUG statement from the Coverity aspect
> > doesn't count as a fix as RELEASE mode behavior remains the same.
> > >
> > > In the comments / description, I already mentioned - Assuming, this
> non-
> > usage is deliberate, so as such I did not intend to hide anything - and
> left it in
> > the status quo.
> > >
> > > The patch proposed may not be appropriate, but should now give a
> > thinking point to active module developers / owners / maintainers if they
> > indeed feel that there is a bug here and it needs to be fixed.
> > >
> >
> > Thanks - I agree that it is a good thing that people are aware of this
> now.
>
>
> Judging from the context in DetectAndConfigIdeDevice(), I think a fix can
> be:
>
>   Status = SetDriveParameters (Instance, IdeChannel, IdeDevice,
> &DriveParameters, NULL);
>   if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "Set drive parameters Fail, Status = %r\n",
> Status));
> continue;
>   }
>
> But I do not have the device and platform environment to verify the change.
> Really sorry for this. I am not sure on the potential risk of making this
> change without at least some kind of 'no-break' tests.
>
> Best Regards,
> Hao Wu
>
>
> >
> > > On Mon, Jun 5, 2023 at 4:33 AM Ard Biesheuvel  wrote:
> > >>
> > >> On Sat, 3 Jun 2023 at 18:04, Pedro Falcato 
> > wrote:
> > >> >
> > >> > On Fri, Jun 2, 2023 at 8:42 PM Ranbir Singh <
> rsi...@ventanamicro.com>
> > wrote:
> > >> > >
> > >> > > From: Ranbir Singh 
> > >> > >
> > >> > > The return value stored in Status after call to SetDriveParameters
> > >> > > is not made of any use thereafter and hence it remains as UNUSED.
> > >> > > Assuming, this non-usage is deliberate, the storage in Status can
> be
> > >> > > done away with.
> > >> > >
> > >> > > Cc: Hao A Wu 
> > >> > > Cc: Ray Ni 
> > >> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
> > >> > > Signed-off-by: Ranbir Singh 
> > >> > > ---
> > >> > >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 2 +-
> > >> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >> > >
> > >> > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> > >> > > index 75403886e44a..c6d637afa989 100644
> > >> > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> > >> > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> > >> > > @@ -2555,7 +2555,7 @@ DetectAndConfigIdeDevice (
> > >> > >DriveParameters.Heads  =
> (UINT8)(((ATA5_IDENTIFY_DATA
> > *)(&Buffer.AtaData))->heads - 1);
> > >> > >DriveParameters.MultipleSector =
> (UINT8)((ATA5_IDENTIFY_DATA
> > *)(&Buffer.AtaData))->multi_sector_cmd_max_sct_cnt;
> > >> > >
> > >> > > -  Status = SetDriveParameters (Instance, IdeChannel,
> IdeDevice,
> > &DriveParameters, NULL);
> > >> > > +  SetDriveParameters (Instance, IdeChannel, IdeDevice,
> > &DriveParameters, NULL);
> > >> >
> > >> > I'm /fairly/ sure this is wrong and that you need to use Status.
> > >> >
> > >>
> > >> Yeah, removing the assignment fixes the coverity warning, but now you
> > >> are hiding a bug instead of fixing it.
> > >>
> > >> SetDriveParameters () can apparently fail, and this is being ignored.
> > >> At the very least, we should emit a diagnostic here in DEBUG mode to
> > >> log this. E.g.,
> > >>
> > >> DEBUG ((DEBUG_WARN, "%a: SetDriveParameters () failed - %r\n",
> > >> __func__, Status));
> >
> >
> > 
> >
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105934): https://edk2.groups.io/g/devel/message/105934
Mute This Topic: https://groups.io/mt/99293623/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] MdeModulePkg/Bus/Ata/AtaBusDxe: Fix SIGN_EXTENSION Coverity issue

2023-06-08 Thread Ranbir Singh

Yes, I have already noted to update this patch on the same lines as
discussed in context of https://edk2.groups.io/g/devel/topic/99293622

On Thu, Jun 8, 2023 at 12:48 PM Wu, Hao A  wrote:

> > -Original Message-
> > From: Ranbir Singh 
> > Sent: Saturday, June 3, 2023 12:09 AM
> > To: devel@edk2.groups.io; rsi...@ventanamicro.com
> > Cc: Wu, Hao A ; Ni, Ray 
> > Subject: [PATCH 1/1] MdeModulePkg/Bus/Ata/AtaBusDxe: Fix
> > SIGN_EXTENSION Coverity issue
> >
> > From: Ranbir Singh 
> >
> > Line number 365 does contain a typecast with UINT32, but it is after
> > all the operations (16-bit left shift followed by OR'ing) are over.
> > To avoid any SIGN_EXTENSION, typecast the intermediate result after
> > 16-bit left shift operation immediately with UINT32.
> >
> > Cc: Hao A Wu 
> > Cc: Ray Ni 
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4209
> > Signed-off-by: Ranbir Singh 
> > ---
> >  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> > b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> > index a77852bae054..ccd4c5f05b59 100644
> > --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> > +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> > @@ -362,7 +362,7 @@ IdentifyAtaDevice (
> >  // Check logical block size
> >
> >  //
> >
> >  if ((PhyLogicSectorSupport & BIT12) != 0) {
> >
> > -  BlockMedia->BlockSize =
> (UINT32)(((IdentifyData->logic_sector_size_hi
> > << 16) | IdentifyData->logic_sector_size_lo) * sizeof (UINT16));
> >
> > +  BlockMedia->BlockSize =
> (((UINT32)(IdentifyData->logic_sector_size_hi
> > << 16) | IdentifyData->logic_sector_size_lo) * sizeof (UINT16));
> >
> >  }
>
>
> This patch seems to have the same issue with the concern raised in
> https://edk2.groups.io/g/devel/topic/99293622.
>
> Best Regards,
> Hao Wu
>
>
> >
> >
> >
> >  AtaDevice->BlockIo.Revision = EFI_BLOCK_IO_PROTOCOL_REVISION2;
> >
> > --
> > 2.34.1
>
>


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

[edk2-devel] [PATCH v2] StandaloneMmPkg: Add StandaloneMmIplPei driver.

2023-06-08 Thread Zhang, Hongbin1

Add StandaloneMmIplPei IA32/X64 driver at PEI stage.
FSP will use this driver to load Standalone MM code
to dispatch other Standalone MM drivers.
And this is the 1st patch to implement the entrypoint
to find the correct SMRAM range and dump it

Signed-off-by: Hongbin1 Zhang 
Cc: Jiewen Yao 
Cc: Ray Ni 
Cc: Star Zeng 
Cc: Jiaxin Wu 
Cc: Sami Mujawar 
Cc: Ard Biesheuvel 
Cc: Supreeth Venkatesh 
---
 StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.c   | 264 

 StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.inf |  54 
 StandaloneMmPkg/StandaloneMmPkg.dsc   |  15 +-
 3 files changed, 331 insertions(+), 2 deletions(-)

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.c 
b/StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.c
new file mode 100644
index 00..f988521c12
--- /dev/null
+++ b/StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.c
@@ -0,0 +1,264 @@
+/** @file
+  SMM IPL that load the SMM Core into SMRAM at PEI stage
+
+  Copyright (c) 2023, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+//
+// MM Core Private Data structure that contains the data shared between
+// the SMM IPL and the Standalone MM Core.
+//
+MM_CORE_PRIVATE_DATA  mMmCorePrivateData = {
+  MM_CORE_PRIVATE_DATA_SIGNATURE, // Signature
+  0,  // MmramRangeCount
+  0,  // MmramRanges
+  0,  // MmEntryPoint
+  FALSE,  // MmEntryPointRegistered
+  FALSE,  // InMm
+  0,  // Mmst
+  0,  // CommunicationBuffer
+  0,  // BufferSize
+  EFI_SUCCESS,// ReturnStatus
+  0,  // MmCoreImageBase
+  0,  // MmCoreImageSize
+  0,  // MmCoreEntryPoint
+  0,  // StandaloneBfvAddress
+};
+
+//
+// Global pointer used to access mMmCorePrivateData from outside and inside SMM
+//
+MM_CORE_PRIVATE_DATA  *gMmCorePrivate;
+
+//
+// SMM IPL global variables
+//
+PEI_SMM_ACCESS_PPI*mSmmAccess;
+EFI_SMRAM_DESCRIPTOR  *mCurrentSmramRange;
+EFI_PHYSICAL_ADDRESS  mSmramCacheBase;
+UINT64mSmramCacheSize;
+
+/**
+  Find the maximum SMRAM cache range that covers the range specified by 
SmramRange.
+
+  This function searches and joins all adjacent ranges of SmramRange into a 
range to be cached.
+
+  @param   SmramRange   The SMRAM range to search from.
+  @param   SmramCacheBase   The returned cache range base.
+  @param   SmramCacheSize   The returned cache range size.
+**/
+VOID
+GetSmramCacheRange (
+  IN  EFI_SMRAM_DESCRIPTOR  *SmramRange,
+  OUT EFI_PHYSICAL_ADDRESS  *SmramCacheBase,
+  OUT UINT64*SmramCacheSize
+  )
+{
+  UINTN Index;
+  EFI_PHYSICAL_ADDRESS  RangeCpuStart;
+  UINT64RangePhysicalSize;
+  BOOLEAN   FoundAdjacentRange;
+  EFI_SMRAM_DESCRIPTOR  *SmramRanges;
+
+  *SmramCacheBase = SmramRange->CpuStart;
+  *SmramCacheSize = SmramRange->PhysicalSize;
+
+  SmramRanges = (EFI_SMRAM_DESCRIPTOR *)(UINTN)gMmCorePrivate->MmramRanges;
+  do {
+FoundAdjacentRange = FALSE;
+for (Index = 0; Index < gMmCorePrivate->MmramRangeCount; Index++) {
+  RangeCpuStart = SmramRanges[Index].CpuStart;
+  RangePhysicalSize = SmramRanges[Index].PhysicalSize;
+  if ((RangeCpuStart < *SmramCacheBase) && (*SmramCacheBase == 
(RangeCpuStart + RangePhysicalSize))) {
+*SmramCacheBase= RangeCpuStart;
+*SmramCacheSize   += RangePhysicalSize;
+FoundAdjacentRange = TRUE;
+  } else if (((*SmramCacheBase + *SmramCacheSize) == RangeCpuStart) && 
(RangePhysicalSize > 0)) {
+*SmramCacheSize   += RangePhysicalSize;
+FoundAdjacentRange = TRUE;
+  }
+}
+  } while (FoundAdjacentRange);
+}
+
+/**
+  Get full SMRAM ranges.
+
+  It will get SMRAM ranges from SmmAccess PPI. It will also reserve one entry
+  for SMM core.
+
+  @param[in]  PeiServices   Describes the list of possible PEI 
Services.
+  @param[out] FullSmramRangeCount   Output pointer to full SMRAM range count.
+
+  @return Pointer to full SMRAM ranges.
+
+**/
+EFI_SMRAM_DESCRIPTOR *
+GetFullSmramRanges (
+  IN  CONST EFI_PEI_SERVICES  **PeiServices,
+  OUT   UINTN *FullSmramRangeCount
+  )
+{
+  EFI_STATUSStatus;
+  UINTN Size;
+  EFI_SMRAM_DESCRIPTOR  *FullSmramRanges;
+  UINTN AdditionSmramRangeCount;
+  UINTN SmramRangeCount;
+
+  //
+  // Get SMRAM information.
+  //
+  Size   = 0;
+  Status = mSmmAccess

Re: [edk2-devel] [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page

On Thu, 8 Jun 2023 at 04:28, duntan  wrote:
>
> Remove RO and NX protection when unset guard page.
> When UnsetGuardPage(), remove all the memory attribute protection
> for guarded page.
>

Why is it acceptable to remove NX protections here?


> Signed-off-by: Dun Tan 
> Cc: Liming Gao 
> Cc: Ray Ni 
> Cc: Jian J Wang 
> ---
>  MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c 
> b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index 8f3bab6fee..7daeeccf13 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -553,7 +553,7 @@ UnsetGuardPage (
>   mSmmMemoryAttribute,
>   BaseAddress,
>   EFI_PAGE_SIZE,
> - EFI_MEMORY_RP
> + 
> EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
>   );
>  ASSERT_EFI_ERROR (Status);
>  mOnGuarding = FALSE;
> --
> 2.31.1.windows.1
>
>
>
> 
>
>


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

Re: [edk2-devel] [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, June 8, 2023 10:27 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Yao, Jiewen
> ; Justen, Jordan L ; Gerd
> Hoffmann ; Tom Lendacky
> ; Ni, Ray 
> Subject: [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask
> to non-leaf entry
> 
> Remove code that apply AddressEncMask to non-leaf entry when split
> smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it
> calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask
> bit in page table for a specific range. In AMD SEV feature, this
> AddressEncMask bit in page table is used to indicate if the memory
> is guest private memory or shared memory. But all memory used by
> page table are treated as encrypted regardless of encryption bit.
> So remove the EncMask bit for smm non-leaf page table entry
> doesn't impact AMD SEV feature.
> If page split happens in the AddressEncMask bit clear process,
> there will be some new non-leaf entries with AddressEncMask
> applied in smm page table. When ReadyToLock, code in PiSmmCpuDxe
> module will use CpuPageTableLib to modify smm page table. So
> remove code to apply AddressEncMask for new non-leaf entries
> since CpuPageTableLib doesn't consume the EncMask PCD.
> 
> Signed-off-by: Dun Tan 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Gerd Hoffmann 
> Cc: Tom Lendacky 
> Cc: Ray Ni 
> ---
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 6
> +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> index cf2441b551..aba2e8c081 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -233,7 +233,7 @@ Split2MPageTo4K (
>// Fill in 2M page entry.
>//
>*PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 |
> -  IA32_PG_P | IA32_PG_RW | AddressEncMask);
> +  IA32_PG_P | IA32_PG_RW);
>  }
> 
>  /**
> @@ -352,7 +352,7 @@ SetPageTablePoolReadOnly (
>  PhysicalAddress += LevelSize[Level - 1];
>}
> 
> -  PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask |
> +  PageTable[Index] = (UINT64)(UINTN)NewPageTable |
>   IA32_PG_P | IA32_PG_RW;
>PageTable = NewPageTable;
>  }
> @@ -440,7 +440,7 @@ Split1GPageTo2M (
>// Fill in 1G page entry.
>//
>*PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry |
> -  IA32_PG_P | IA32_PG_RW | AddressEncMask);
> +  IA32_PG_P | IA32_PG_RW);
> 
>PhysicalAddress2M = PhysicalAddress;
>for (IndexOfPageDirectoryEntries = 0;
> --
> 2.31.1.windows.1



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

Re: [edk2-devel] [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case
> when clear RP
> 
> In ConvertMemoryPageAttributes() function, when clear RP for a
> specific range [BaseAddress, BaseAddress + Length], it means to
> set the present bit to 1 and assign default value for other
> attributes in page table. The default attributes for the input
> specific range are NX disabled and ReadOnly. If there is existing
> present range in [BaseAddress, BaseAddress + Length] and the
> attributes are not NX disabled or not ReadOnly, then output the
> DEBUG message to indicate that the NX and ReadOnly attributes of
> the existing present range are modified in the function.
> 
> Signed-off-by: Dun Tan 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 48
> 
>  1 file changed, 48 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 12723e5750..862b3e9720 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -349,6 +349,8 @@ ConvertMemoryPageAttributes (
>IA32_MAP_ENTRY*Map;
>UINTN Count;
>UINTN Index;
> +  UINT64OverlappedRangeBase;
> +  UINT64OverlappedRangeLimit;
> 
>ASSERT (Attributes != 0);
>ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
> @@ -430,6 +432,52 @@ ConvertMemoryPageAttributes (
>// By default memory is Ring 3 accessble.
>//
>PagingAttribute.Bits.UserSupervisor = 1;
> +
> +  DEBUG_CODE_BEGIN ();
> +  if (((Attributes & EFI_MEMORY_RO) == 0) || (((Attributes &
> EFI_MEMORY_XP) == 0) && (mXdSupported))) {
> +//
> +// When mapping a range to present and EFI_MEMORY_RO or
> EFI_MEMORY_XP is not specificed,
> +// check if [BaseAddress, BaseAddress + Length] contains present 
> range.
> +// Existing Present range in [BaseAddress, BaseAddress + Length] is 
> set to
> NX disable or ReadOnly.
> +//
> +Count  = 0;
> +Map= NULL;
> +Status = PageTableParse (PageTableBase, mPagingMode, NULL, &Count);
> +
> +while (Status == RETURN_BUFFER_TOO_SMALL) {
> +  if (Map != NULL) {
> +FreePool (Map);
> +  }
> +
> +  Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
> +  ASSERT (Map != NULL);
> +  Status = PageTableParse (PageTableBase, mPagingMode, Map, &Count);
> +}
> +
> +ASSERT_RETURN_ERROR (Status);
> +for (Index = 0; Index < Count; Index++) {
> +  if (Map[Index].LinearAddress >= BaseAddress + Length) {
> +break;
> +  }
> +
> +  if ((BaseAddress < Map[Index].LinearAddress + Map[Index].Length) &&
> (BaseAddress + Length > Map[Index].LinearAddress)) {
> +OverlappedRangeBase  = MAX (BaseAddress,
> Map[Index].LinearAddress);
> +OverlappedRangeLimit = MIN (BaseAddress + Length,
> Map[Index].LinearAddress + Map[Index].Length);
> +
> +if (((Attributes & EFI_MEMORY_RO) == 0) &&
> (Map[Index].Attribute.Bits.ReadWrite == 1)) {
> +  DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes:
> [0x%lx, 0x%lx] is set from ReadWrite to ReadOnly\n", OverlappedRangeBase,
> OverlappedRangeLimit));
> +}
> +
> +if (((Attributes & EFI_MEMORY_XP) == 0) && (mXdSupported) &&
> (Map[Index].Attribute.Bits.Nx == 1)) {
> +  DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes:
> [0x%lx, 0x%lx] is set from NX enabled to NX disabled\n",
> OverlappedRangeBase, OverlappedRangeLimit));
> +}
> +  }
> +}
> +
> +FreePool (Map);
> +  }
> +
> +  DEBUG_CODE_END ();
>  }
>}
> 
> --
> 2.31.1.windows.1



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

Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/XhciDxe: Fix Broken Timeouts

2023-06-08 Thread Leif Lindholm


+maintainers

On 2023-06-07 22:37, Jessica Clarke wrote:

On Wed, Sep 23, 2020 at 08:36 PM, Henz, Patrick wrote:



From: Patrick Henz 

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

Timeouts in the XhciDxe driver are taking longer than
expected due to the timeout loops not accounting for
code execution time. As en example, 5 second timeouts
have been observed to take around 36 seconds to complete.
Use SetTimer and Create/CheckEvent from Boot Services to
determine when timeout occurred.


This strategy doesn't work during ExitBootServices, as called from
XhcExitBootService via XhcHaltHC. The net result is that, if the XHCI
controller fails to halt upon clearing R/S, EDK2 hangs in an infinite
loop (something I just had the joy of debugging).

Jess









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

Re: [edk2-devel] [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX

Reviewed-by: Ray Ni 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of duntan
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [edk2-devel] [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm:
> Avoid setting non-present range to RO/NX
> 
> In PiSmmCpuDxeSmm code, SetMemMapAttributes() marks memory ranges
> in SmmMemoryAttributesTable to RO/NX. There may exist non-present
> range in these memory ranges. Set other attributes for a non-present
> range is not permitted in CpuPageTableMapLib. So add code to handle
> this case. Only map the present ranges in SmmMemoryAttributesTable
> to RO or NX.
> 
> Signed-off-by: Dun Tan 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 129
> ++
> +---
> ---
>  1 file changed, 107 insertions(+), 22 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 862b3e9720..3c79927c7b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -918,6 +918,70 @@ PatchGdtIdtMap (
>  );
>  }
> 
> +/**
> +  This function set [Base, Limit] to the input MemoryAttribute.
> +
> +  @param  BaseStart address of range.
> +  @param  Limit   Limit address of range.
> +  @param  Attribute   The bit mask of attributes to modify for the memory
> region.
> +  @param  Map Pointer to the array of Cr3 IA32_MAP_ENTRY.
> +  @param  Count   Count of IA32_MAP_ENTRY in Map.
> +**/
> +VOID
> +SetMemMapWithNonPresentRange (
> +  UINT64  Base,
> +  UINT64  Limit,
> +  UINT64  Attribute,
> +  IA32_MAP_ENTRY  *Map,
> +  UINTN   Count
> +  )
> +{
> +  UINTN   Index;
> +  UINT64  NonPresentRangeStart;
> +
> +  NonPresentRangeStart = 0;
> +  for (Index = 0; Index < Count; Index++) {
> +if ((Map[Index].LinearAddress > NonPresentRangeStart) &&
> +(Base < Map[Index].LinearAddress) && (Limit > NonPresentRangeStart))
> +{
> +  //
> +  // We should NOT set attributes for non-present ragne.
> +  //
> +  //
> +  // There is a non-present ( [NonPresentStart,
> Map[Index].LinearAddress] ) range before current Map[Index]
> +  // and it is overlapped with [Base, Limit].
> +  //
> +  if (Base < NonPresentRangeStart) {
> +SmmSetMemoryAttributes (
> +  Base,
> +  NonPresentRangeStart - Base,
> +  Attribute
> +  );
> +  }
> +
> +  Base = Map[Index].LinearAddress;
> +}
> +
> +NonPresentRangeStart = Map[Index].LinearAddress + Map[Index].Length;
> +if (NonPresentRangeStart >= Limit) {
> +  break;
> +}
> +  }
> +
> +  Limit = MIN (NonPresentRangeStart, Limit);
> +
> +  if (Base < Limit) {
> +//
> +// There is no non-present range in current [Base, Limit] anymore.
> +//
> +SmmSetMemoryAttributes (
> +  Base,
> +  Limit - Base,
> +  Attribute
> +  );
> +  }
> +}
> +
>  /**
>This function sets memory attribute according to MemoryAttributesTable.
>  **/
> @@ -932,6 +996,11 @@ SetMemMapAttributes (
>UINTN DescriptorSize;
>UINTN Index;
>EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE  *MemoryAttributesTable;
> +  UINTN PageTable;
> +  EFI_STATUSStatus;
> +  IA32_MAP_ENTRY*Map;
> +  UINTN Count;
> +  UINT64MemoryAttribute;
> 
>SmmGetSystemConfigurationTable
> (&gEdkiiPiSmmMemoryAttributesTableGuid, (VOID
> **)&MemoryAttributesTable);
>if (MemoryAttributesTable == NULL) {
> @@ -958,36 +1027,52 @@ SetMemMapAttributes (
>  MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap,
> DescriptorSize);
>}
> 
> +  Count = 0;
> +  Map   = NULL;
> +  PageTable = AsmReadCr3 ();
> +  Status= PageTableParse (PageTable, mPagingMode, NULL, &Count);
> +  while (Status == RETURN_BUFFER_TOO_SMALL) {
> +if (Map != NULL) {
> +  FreePool (Map);
> +}
> +
> +Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
> +ASSERT (Map != NULL);
> +Status = PageTableParse (PageTable, mPagingMode, Map, &Count);
> +  }
> +
> +  ASSERT_RETURN_ERROR (Status);
> +
>MemoryMap = MemoryMapStart;
>for (Index = 0; Index < MemoryMapEntryCount; Index++) {
>  DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n",
> MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
> -switch (MemoryMap->Type) {
> -  case EfiRuntimeServicesCode:
> -SmmSetMemoryAttributes (
> -  MemoryMap->PhysicalStart,
> -  EFI_PAGES_TO_SIZE (

Re: [edk2-devel] [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute.

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM
> paging attribute.
> 
> Simplify the ConvertMemoryPageAttributes API to convert paging
> attribute by CpuPageTableLib. In the new API, it calls
> PageTableMap() to update the page attributes of a memory range.
> With the PageTableMap() API in CpuPageTableLib, we can remove
> the complicated page table manipulating code.
> 
> Signed-off-by: Dun Tan 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   3 ++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  28
> +---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   1 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 409
> ++
> ++---
> --
> --
> --
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c|   7 ++-
>  5 files changed, 122 insertions(+), 326 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index 34bf6e1a25..9c8107080a 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> @@ -1,7 +1,7 @@
>  /** @file
>  Page table manipulation functions for IA-32 processors
> 
> -Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
> +Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -31,6 +31,7 @@ SmmInitPageTable (
>InitializeSpinLock (mPFLock);
> 
>mPhysicalAddressBits = 32;
> +  mPagingMode  = PagingPae;
> 
>if (FeaturePcdGet (PcdCpuSmmProfileEnable) ||
>HEAP_GUARD_NONSTOP_MODE ||
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index a5c2bdd971..ba341cadc6 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -50,6 +50,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
>  #include 
> @@ -260,6 +261,7 @@ extern UINTN mNumberOfCpus;
>  extern EFI_SMM_CPU_PROTOCOL  mSmmCpu;
>  extern EFI_MM_MP_PROTOCOLmSmmMp;
>  extern BOOLEAN   m5LevelPagingNeeded;
> +extern PAGING_MODE   mPagingMode;
> 
>  ///
>  /// The mode of the CPU at the time an SMI occurs
> @@ -1008,11 +1010,10 @@ SetPageTableAttributes (
>Length from their current attributes to the attributes specified by 
> Attributes.
> 
>@param[in]   PageTableBaseThe page table base.
> -  @param[in]   EnablePML5Paging If PML5 paging is enabled.
> +  @param[in]   PagingMode   The paging mode.
>@param[in]   BaseAddress  The physical address that is the start 
> address of
> a memory region.
>@param[in]   Length   The size in bytes of the memory region.
>@param[in]   Attributes   The bit mask of attributes to set for the 
> memory
> region.
> -  @param[out]  IsSplitted   TRUE means page table splitted. FALSE means
> page table not splitted.
> 
>@retval EFI_SUCCESS   The attributes were set for the memory 
> region.
>@retval EFI_ACCESS_DENIED The attributes for the memory resource
> range specified by
> @@ -1030,12 +1031,11 @@ SetPageTableAttributes (
>  **/
>  EFI_STATUS
>  SmmSetMemoryAttributesEx (
> -  IN  UINTN PageTableBase,
> -  IN  BOOLEAN   EnablePML5Paging,
> -  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
> -  IN  UINT64Length,
> -  IN  UINT64Attributes,
> -  OUT BOOLEAN   *IsSplitted  OPTIONAL
> +  IN  UINTN PageTableBase,
> +  IN  PAGING_MODE   PagingMode,
> +  IN  PHYSICAL_ADDRESS  BaseAddress,
> +  IN  UINT64Length,
> +  IN  UINT64Attributes
>);
> 
>  /**
> @@ -1043,34 +1043,32 @@ SmmSetMemoryAttributesEx (
>Length from their current attributes to the attributes specified by 
> Attributes.
> 
>@param[in]   PageTableBaseThe page table base.
> -  @param[in]   EnablePML5Paging If PML5 paging is enabled.
> +  @param[in]   PagingMode   The paging mode.
>@param[in]   BaseAddress  The physical address that is the start 
> address of
> a memory region.
>@param[in]   Length   The size in bytes of the

Re: [edk2-devel] [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in
> PiSmmCpuDxeSmm.h
> 
> Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h and remove
> extern for mSmmShadowStackSize in c files to simplify code.
> 
> Signed-off-by: Dun Tan 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 3 +--
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 --
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h| 1 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c   | 2 --
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c  | 3 +--
>  5 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> index 6c48a53f67..636dc8d92f 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> @@ -1,7 +1,7 @@
>  /** @file
>SMM CPU misc functions for Ia32 arch specific.
> 
> -Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -14,7 +14,6 @@ EFI_PHYSICAL_ADDRESS  mGdtBuffer;
>  UINTN mGdtBufferSize;
> 
>  extern BOOLEAN  mCetSupported;
> -extern UINTNmSmmShadowStackSize;
> 
>  X86_ASSEMBLY_PATCH_LABEL  mPatchCetPl0Ssp;
>  X86_ASSEMBLY_PATCH_LABEL  mPatchCetInterruptSsp;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index baf827cf9d..1878252eac 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -29,8 +29,6 @@ MM_COMPLETIONmSmmStartupThisApToken;
>  //
>  UINT32  *mPackageFirstThreadIndex = NULL;
> 
> -extern UINTN  mSmmShadowStackSize;
> -
>  /**
>Performs an atomic compare exchange operation to get semaphore.
>The compare exchange operation must be performed using
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index e0c4ca76dc..a7da9673a5 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -262,6 +262,7 @@ extern EFI_SMM_CPU_PROTOCOL  mSmmCpu;
>  extern EFI_MM_MP_PROTOCOLmSmmMp;
>  extern BOOLEAN   m5LevelPagingNeeded;
>  extern PAGING_MODE   mPagingMode;
> +extern UINTN mSmmShadowStackSize;
> 
>  ///
>  /// The mode of the CPU at the time an SMI occurs
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 0bed857cae..46d8ff5d4e 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -13,8 +13,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define PAGE_TABLE_PAGES  8
>  #define ACC_MAX_BIT   BIT3
> 
> -extern UINTN  mSmmShadowStackSize;
> -
>  LIST_ENTRYmPagePool   = INITIALIZE_LIST_HEAD_VARIABLE
> (mPagePool);
>  BOOLEAN   m1GPageTableSupport = FALSE;
>  BOOLEAN   mCpuSmmRestrictedMemoryAccess;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> index 00a284c369..c4f21e2155 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> @@ -1,7 +1,7 @@
>  /** @file
>SMM CPU misc functions for x64 arch specific.
> 
> -Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -12,7 +12,6 @@ EFI_PHYSICAL_ADDRESS  mGdtBuffer;
>  UINTN mGdtBufferSize;
> 
>  extern BOOLEAN  mCetSupported;
> -extern UINTNmSmmShadowStackSize;
> 
>  X86_ASSEMBLY_PATCH_LABEL  mPatchCetPl0Ssp;
>  X86_ASSEMBLY_PATCH_LABEL  mPatchCetInterruptSsp;
> --
> 2.31.1.windows.1



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

Re: [edk2-devel] [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime InitPaging() code

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime
> InitPaging() code
> 
> This commit is code refinement to current smm runtime InitPaging()
> page table update code. In InitPaging(), if PcdCpuSmmProfileEnable
> is TRUE, use ConvertMemoryPageAttributes() API to map the range in
> mProtectionMemRange to the attrbute recorded in the attribute field
> of mProtectionMemRange, map the range outside mProtectionMemRange
> as non-present. If PcdCpuSmmProfileEnable is FALSE, only need to
> set the ranges not in mSmmCpuSmramRanges as NX.
> 
> Signed-off-by: Dun Tan 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  37
> +
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 293
> ++
> ++
> --
> ---
>  2 files changed, 101 insertions(+), 229 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 5399659bc0..12ad86028e 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -725,6 +725,43 @@ SmmBlockingStartupThisAp (
>IN OUT  VOID  *ProcArguments OPTIONAL
>);
> 
> +/**
> +  This function modifies the page attributes for the memory region specified
> by BaseAddress and
> +  Length from their current attributes to the attributes specified by 
> Attributes.
> +
> +  Caller should make sure BaseAddress and Length is at page boundary.
> +
> +  @param[in]   PageTableBaseThe page table base.
> +  @param[in]   BaseAddress  The physical address that is the start 
> address
> of a memory region.
> +  @param[in]   Length   The size in bytes of the memory region.
> +  @param[in]   Attributes   The bit mask of attributes to modify for the
> memory region.
> +  @param[in]   IsSetTRUE means to set attributes. FALSE means to 
> clear
> attributes.
> +  @param[out]  IsModified   TRUE means page table modified. FALSE means
> page table not modified.
> +
> +  @retval RETURN_SUCCESS   The attributes were modified for the
> memory region.
> +  @retval RETURN_ACCESS_DENIED The attributes for the memory resource
> range specified by
> +   BaseAddress and Length cannot be modified.
> +  @retval RETURN_INVALID_PARAMETER Length is zero.
> +   Attributes specified an illegal 
> combination of attributes that
> +   cannot be set together.
> +  @retval RETURN_OUT_OF_RESOURCES  There are not enough system
> resources to modify the attributes of
> +   the memory resource range.
> +  @retval RETURN_UNSUPPORTED   The processor does not support one or
> more bytes of the memory
> +   resource range specified by BaseAddress 
> and Length.
> +   The bit mask of attributes is not support 
> for the memory
> resource
> +   range specified by BaseAddress and Length.
> +**/
> +RETURN_STATUS
> +ConvertMemoryPageAttributes (
> +  IN  UINTN PageTableBase,
> +  IN  PAGING_MODE   PagingMode,
> +  IN  PHYSICAL_ADDRESS  BaseAddress,
> +  IN  UINT64Length,
> +  IN  UINT64Attributes,
> +  IN  BOOLEAN   IsSet,
> +  OUT BOOLEAN   *IsModified   OPTIONAL
> +  );
> +
>  /**
>This function sets the attributes for the memory region specified by
> BaseAddress and
>Length from their current attributes to the attributes specified by 
> Attributes.
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index 2a1f132b29..ab6b79ddf4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -586,254 +586,89 @@ InitPaging (
>VOID
>)
>  {
> -  UINT64Pml5Entry;
> -  UINT64Pml4Entry;
> -  UINT64*Pml5;
> -  UINT64*Pml4;
> -  UINT64*Pdpt;
> -  UINT64*Pd;
> -  UINT64*Pt;
> -  UINTN Address;
> -  UINTN Pml5Index;
> -  UINTN Pml4Index;
> -  UINTN PdptIndex;
> -  UINTN PdIndex;
> -  UINTN PtIndex;
> -  UINTN NumberOfPdptEntries;
> -  UINTN NumberOfPml4Entries;
> -  UINTN NumberOfPml5Entries;
> -  UINTN SizeOfMemorySpace;
> -  BOOLEAN   Nx;
> -  IA32_CR4  Cr4;
> -  BOOLEAN   Enable5LevelPaging;
> -  BOOLEAN   WpEnabled;
> -  BOOLEAN   CetEnabled;

Re: [edk2-devel] [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when
> ReadyToLock
> 
> Sort mProtectionMemRange in InitProtectedMemRange() when
> ReadyToLock.
> 
> Signed-off-by: Dun Tan 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 32
> 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index 5625ba0cac..2a1f132b29 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -375,6 +375,32 @@ IsAddressSplit (
>return FALSE;
>  }
> 
> +/**
> +  Function to compare 2 MEMORY_PROTECTION_RANGE based on range
> base.
> +
> +  @param[in] Buffer1pointer to Device Path poiner to compare
> +  @param[in] Buffer2pointer to second DevicePath pointer to 
> compare
> +
> +  @retval 0 Buffer1 equal to Buffer2
> +  @retval <0Buffer1 is less than Buffer2
> +  @retval >0Buffer1 is greater than Buffer2
> +**/
> +INTN
> +EFIAPI
> +ProtectionRangeCompare (
> +  IN  CONST VOID  *Buffer1,
> +  IN  CONST VOID  *Buffer2
> +  )
> +{
> +  if (((MEMORY_PROTECTION_RANGE *)Buffer1)->Range.Base >
> ((MEMORY_PROTECTION_RANGE *)Buffer2)->Range.Base) {
> +return 1;
> +  } else if (((MEMORY_PROTECTION_RANGE *)Buffer1)->Range.Base <
> ((MEMORY_PROTECTION_RANGE *)Buffer2)->Range.Base) {
> +return -1;
> +  }
> +
> +  return 0;
> +}
> +
>  /**
>Initialize the protected memory ranges and the 4KB-page mapped memory
> ranges.
> 
> @@ -397,6 +423,7 @@ InitProtectedMemRange (
>EFI_PHYSICAL_ADDRESS Base2MBAlignedAddress;
>UINT64   High4KBPageSize;
>UINT64   Low4KBPageSize;
> +  MEMORY_PROTECTION_RANGE  MemProtectionRange;
> 
>NumberOfDescriptors  = 0;
>NumberOfAddedDescriptors = mSmmCpuSmramRangeCount;
> @@ -533,6 +560,11 @@ InitProtectedMemRange (
> 
>mSplitMemRangeCount = NumberOfSpliteRange;
> 
> +  //
> +  // Sort the mProtectionMemRange
> +  //
> +  QuickSort (mProtectionMemRange, mProtectionMemRangeCount, sizeof
> (MEMORY_PROTECTION_RANGE),
> (BASE_SORT_COMPARE)ProtectionRangeCompare, &MemProtectionRange);
> +
>DEBUG ((DEBUG_INFO, "SMM Profile Memory Ranges:\n"));
>for (Index = 0; Index < mProtectionMemRangeCount; Index++) {
>  DEBUG ((DEBUG_INFO, "mProtectionMemRange[%d].Base = %lx\n", Index,
> mProtectionMemRange[Index].Range.Base));
> --
> 2.31.1.windows.1



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

Re: [edk2-devel] [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in
> FindSmramInfo
> 
> Sort mSmmCpuSmramRanges after get the SMRAM info in
> FindSmramInfo() function.
> 
> Signed-off-by: Dun Tan 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 32
> 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 2144d6ade8..2568ffd677 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -1197,6 +1197,32 @@ PiCpuSmmEntry (
>return EFI_SUCCESS;
>  }
> 
> +/**
> +  Function to compare 2 EFI_SMRAM_DESCRIPTOR based on CpuStart.
> +
> +  @param[in] Buffer1pointer to Device Path poiner to compare
> +  @param[in] Buffer2pointer to second DevicePath pointer to 
> compare
> +
> +  @retval 0 Buffer1 equal to Buffer2
> +  @retval <0Buffer1 is less than Buffer2
> +  @retval >0Buffer1 is greater than Buffer2
> +**/
> +INTN
> +EFIAPI
> +CpuSmramRangeCompare (
> +  IN  CONST VOID  *Buffer1,
> +  IN  CONST VOID  *Buffer2
> +  )
> +{
> +  if (((EFI_SMRAM_DESCRIPTOR *)Buffer1)->CpuStart >
> ((EFI_SMRAM_DESCRIPTOR *)Buffer2)->CpuStart) {
> +return 1;
> +  } else if (((EFI_SMRAM_DESCRIPTOR *)Buffer1)->CpuStart <
> ((EFI_SMRAM_DESCRIPTOR *)Buffer2)->CpuStart) {
> +return -1;
> +  }
> +
> +  return 0;
> +}
> +
>  /**
> 
>Find out SMRAM information including SMRR base and SMRR size.
> @@ -1218,6 +1244,7 @@ FindSmramInfo (
>UINTN Index;
>UINT64MaxSize;
>BOOLEAN   Found;
> +  EFI_SMRAM_DESCRIPTOR  SmramDescriptor;
> 
>//
>// Get SMM Access Protocol
> @@ -1240,6 +1267,11 @@ FindSmramInfo (
> 
>mSmmCpuSmramRangeCount = Size / sizeof (EFI_SMRAM_DESCRIPTOR);
> 
> +  //
> +  // Sort the mSmmCpuSmramRanges
> +  //
> +  QuickSort (mSmmCpuSmramRanges, mSmmCpuSmramRangeCount, sizeof
> (EFI_SMRAM_DESCRIPTOR),
> (BASE_SORT_COMPARE)CpuSmramRangeCompare, &SmramDescriptor);
> +
>//
>// Find the largest SMRAM range between 1MB and 4GB that is at least 256K
> - 4K in size
>//
> --
> 2.31.1.windows.1



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

Re: [edk2-devel] [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create smm page table

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create
> smm page table
> 
> This commit is code refinement to current smm pagetable generation
> code. Add a new GenSmmPageTable() API to create smm page table
> based on the PageTableMap() API in CpuPageTableLib. Caller only
> needs to specify the paging mode and the PhysicalAddressBits to map.
> This function can be used to create both IA32 pae paging and X64
> 5level, 4level paging.
> 
> Signed-off-by: Dun Tan 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   2 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  15
> +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  65
> ++
> +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c| 220
> ++
> --
> 
>  4 files changed, 107 insertions(+), 195 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index 9c8107080a..b11264ce4a 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> @@ -63,7 +63,7 @@ SmmInitPageTable (
>  InitializeIDTSmmStackGuard ();
>}
> 
> -  return Gen4GPageTable (TRUE);
> +  return GenSmmPageTable (PagingPae, mPhysicalAddressBits);
>  }
> 
>  /**
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index a7da9673a5..5399659bc0 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -553,6 +553,21 @@ Gen4GPageTable (
>IN  BOOLEAN  Is32BitPageTable
>);
> 
> +/**
> +  Create page table based on input PagingMode and PhysicalAddressBits in
> smm.
> +
> +  @param[in]  PagingMode   The paging mode.
> +  @param[in]  PhysicalAddressBits  The bits of physical address to map.
> +
> +  @retval PageTable Address
> +
> +**/
> +UINTN
> +GenSmmPageTable (
> +  IN PAGING_MODE  PagingMode,
> +  IN UINT8PhysicalAddressBits
> +  );
> +
>  /**
>Initialize global data for MP synchronization.
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 4ee99d06d7..4e93d26eb4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -1640,6 +1640,71 @@ EdkiiSmmClearMemoryAttributes (
>return SmmClearMemoryAttributes (BaseAddress, Length, Attributes);
>  }
> 
> +/**
> +  Create page table based on input PagingMode and PhysicalAddressBits in
> smm.
> +
> +  @param[in]  PagingMode   The paging mode.
> +  @param[in]  PhysicalAddressBits  The bits of physical address to map.
> +
> +  @retval PageTable Address
> +
> +**/
> +UINTN
> +GenSmmPageTable (
> +  IN PAGING_MODE  PagingMode,
> +  IN UINT8PhysicalAddressBits
> +  )
> +{
> +  UINTN   PageTableBufferSize;
> +  UINTN   PageTable;
> +  VOID*PageTableBuffer;
> +  IA32_MAP_ATTRIBUTE  MapAttribute;
> +  IA32_MAP_ATTRIBUTE  MapMask;
> +  RETURN_STATUS   Status;
> +  UINTN   GuardPage;
> +  UINTN   Index;
> +  UINT64  Length;
> +
> +  Length   = LShiftU64 (1, PhysicalAddressBits);
> +  PageTable= 0;
> +  PageTableBufferSize  = 0;
> +  MapMask.Uint64   = MAX_UINT64;
> +  MapAttribute.Uint64  = mAddressEncMask;
> +  MapAttribute.Bits.Present= 1;
> +  MapAttribute.Bits.ReadWrite  = 1;
> +  MapAttribute.Bits.UserSupervisor = 1;
> +  MapAttribute.Bits.Accessed   = 1;
> +  MapAttribute.Bits.Dirty  = 1;
> +
> +  Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
> +  ASSERT (Status == RETURN_BUFFER_TOO_SMALL);
> +  DEBUG ((DEBUG_INFO, "GenSMMPageTable: 0x%x bytes needed for initial
> SMM page table\n", PageTableBufferSize));
> +  PageTableBuffer = AllocatePageTableMemory (EFI_SIZE_TO_PAGES
> (PageTableBufferSize));
> +  ASSERT (PageTableBuffer != NULL);
> +  Status = PageTableMap (&PageTable, PagingMode, PageTableBuffer,
> &PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
> +  ASSERT (Status == RETURN_SUCCESS);
> +  ASSERT (PageTableBufferSize == 0);
> +
> +  if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> +//
> +// Mark the 4KB guard page between

Re: [edk2-devel] [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page