* [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes
@ 2023-06-02 15:17 Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 1/7] MdeModulePkg: Define memory attribute PPI Ard Biesheuvel
` (8 more replies)
0 siblings, 9 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-02 15:17 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4468
This is a followup to the RFC that I sent to the edk2-devel list on the
25th of May.
In an attempt to make some incremental progress, this v2 only covers the
NX remapping of the DXE stack in DxeIpl, using the newly introduced
memory attributes PPI.
Other use cases are deferred until we can converge on an approach that
works across architectures and platforms. In particular, this means the
following use cases:
- mapping the DXE core code and data regions RO and XP, respectively;
- mapping shadowed PEIMs read-only (including the PEI core itself);
- managing memory permissions after temporary RAM migration;
- reorganizing the X64 PEI flow with respect to page table allocation;
- managing the dispatch order of the PEIM producing the PPI in relation
to its consumers.
The current series specifies the PPI in patch #1, and wires it up into
DxeIpl to remap the DXE stack non-executable in a generic manner
(patches #2 and #3)
Patches #4 and #5 implement the PPI for ARM and AArch64.
Patch #6 switches ARM and AArch64 over to the generic DxeIpl.
Patch #7 cleans up the ARM implementation of the UEFI memory attributes
protocol, based on the improvements made in patch #4.
Changes since RFC (in addition to the above):
- update PPI protype to use attributes+mask instead of setmask+clearmask
- drop OVMF patch for RISC-V that has been applied in the meantime
Cc: Ray Ni <ray.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Taylor Beebe <t@taylorbeebe.com>
Cc: Oliver Smith-Denny <osd@smith-denny.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Dun Tan <dun.tan@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Ard Biesheuvel (7):
MdeModulePkg: Define memory attribute PPI
MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack NX
ArmPkg/ArmMmuLib: Extend API to manage memory permissions better
ArmPkg/CpuPei: Implement the memory attributes PPI
MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code
ArmPkg/CpuDxe: Simplify memory attributes protocol implementation
ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
ArmPkg/Drivers/CpuDxe/MemoryAttribute.c | 50 +----------
ArmPkg/Drivers/CpuPei/CpuPei.c | 76 +++++++++++++++++
ArmPkg/Drivers/CpuPei/CpuPei.inf | 4 +
ArmPkg/Include/Library/ArmMmuLib.h | 36 +++++++-
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 52 +++++++++++-
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 88 +++++++++++++++++---
ArmPkg/Library/OpteeLib/Optee.c | 2 +-
MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 71 ----------------
MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} | 31 ++++++-
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 24 ++----
MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c | 63 --------------
MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c | 75 -----------------
MdeModulePkg/Include/Ppi/MemoryAttribute.h | 83 ++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +
15 files changed, 366 insertions(+), 294 deletions(-)
delete mode 100644 MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
rename MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} (62%)
delete mode 100644 MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
delete mode 100644 MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
create mode 100644 MdeModulePkg/Include/Ppi/MemoryAttribute.h
--
2.39.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/7] MdeModulePkg: Define memory attribute PPI
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
@ 2023-06-02 15:17 ` Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 2/7] MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code Ard Biesheuvel
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-02 15:17 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
Define a PPI interface that may be used by the PEI core or other PEIMs
to manage permissions on memory ranges. This is primarily intended for
restricting permissions to what is actually needed for correct execution
by the code in question, and for limiting the use of memory mappings
that are both writable and executable at the same time.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
MdeModulePkg/Include/Ppi/MemoryAttribute.h | 83 ++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +
2 files changed, 86 insertions(+)
diff --git a/MdeModulePkg/Include/Ppi/MemoryAttribute.h b/MdeModulePkg/Include/Ppi/MemoryAttribute.h
new file mode 100644
index 0000000000000000..83bcc33a76719712
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/MemoryAttribute.h
@@ -0,0 +1,83 @@
+/** @file
+
+Copyright (c) 2023, Google LLC. All rights reserved.<BR>
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef EDKII_MEMORY_ATTRIBUTE_PPI_H_
+#define EDKII_MEMORY_ATTRIBUTE_PPI_H_
+
+#include <Uefi/UefiSpec.h>
+
+///
+/// Global ID for the EDKII_MEMORY_ATTRIBUTE_PPI.
+///
+#define EDKII_MEMORY_ATTRIBUTE_PPI_GUID \
+ { \
+ 0x1be840de, 0x2d92, 0x41ec, { 0xb6, 0xd3, 0x19, 0x64, 0x13, 0x50, 0x51, 0xfb } \
+ }
+
+///
+/// Forward declaration for the EDKII_MEMORY_ATTRIBUTE_PPI.
+///
+typedef struct _EDKII_MEMORY_ATTRIBUTE_PPI EDKII_MEMORY_ATTRIBUTE_PPI;
+
+/**
+ Set the requested memory permission attributes on a region of memory.
+
+ BaseAddress and Length must be aligned to EFI_PAGE_SIZE.
+
+ Attributes must contain a combination of EFI_MEMORY_RP, EFI_MEMORY_RO and
+ EFI_MEMORY_XP, and specifies the attributes that must be set for the
+ region in question. Attributes that are omitted will be cleared from the
+ region only if they are set in AttributeMask.
+
+ AttributeMask must contain a combination of EFI_MEMORY_RP, EFI_MEMORY_RO and
+ EFI_MEMORY_XP, and specifies the attributes that the call will operate on.
+ AttributeMask must not be 0x0, and must contain at least the bits set in
+ Attributes.
+
+ @param[in] This The protocol instance pointer.
+ @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 Memory attributes to set or clear.
+ @param[in] AttributeMask Mask of memory attributes to operate on.
+
+ @retval EFI_SUCCESS The attributes were set for the memory region.
+ @retval EFI_INVALID_PARAMETER Length is zero.
+ AttributeMask is zero.
+ AttributeMask lacks bits set in Attributes.
+ BaseAddress or Length is not suitably aligned.
+ @retval EFI_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 supported for
+ the memory resource range specified by
+ BaseAddress and Length.
+ @retval EFI_OUT_OF_RESOURCES Requested attributes cannot be applied due to
+ lack of system resources.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_MEMORY_ATTRIBUTE_SET_PERMISSIONS)(
+ IN EDKII_MEMORY_ATTRIBUTE_PPI *This,
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN UINT64 AttributeMask
+ );
+
+///
+/// This PPI contains a set of services to manage memory permission attributes.
+///
+struct _EDKII_MEMORY_ATTRIBUTE_PPI {
+ EDKII_MEMORY_ATTRIBUTE_SET_PERMISSIONS SetPermissions;
+};
+
+extern EFI_GUID gEdkiiMemoryAttributePpiGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 95dd077e19b3a901..d65dae18aa81e569 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -528,6 +528,9 @@ [Ppis]
gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d, { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7, 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
+ ## Include/Ppi/MemoryAttribute.h
+ gEdkiiMemoryAttributePpiGuid = { 0x1be840de, 0x2d92, 0x41ec, { 0xb6, 0xd3, 0x19, 0x64, 0x13, 0x50, 0x51, 0xfb } }
+
[Protocols]
## Load File protocol provides capability to load and unload EFI image into memory and execute it.
# Include/Protocol/LoadPe32Image.h
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/7] MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 1/7] MdeModulePkg: Define memory attribute PPI Ard Biesheuvel
@ 2023-06-02 15:17 ` Ard Biesheuvel
2023-06-06 18:48 ` [edk2-devel] " Michael Kubacki
2023-06-02 15:17 ` [PATCH v2 3/7] MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack NX Ard Biesheuvel
` (6 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-02 15:17 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
The Risc-V and LoongArch specific versions of the DXE core handoff code
in DxeIpl are essentially copies of the EBC version (modulo the
copyright in the header and some debug prints in the code).
In preparation for introducing a generic PPI based method to implement
the non-executable stack, let's merge these versions, so we only need to
add this logic once.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} | 2 +-
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 10 +--
MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c | 63 ----------------
MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c | 75 --------------------
4 files changed, 3 insertions(+), 147 deletions(-)
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ebc/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
similarity index 92%
rename from MdeModulePkg/Core/DxeIplPeim/Ebc/DxeLoadFunc.c
rename to MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
index c1a16b602452218e..a0f85ebea56e6cba 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ebc/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
@@ -1,5 +1,5 @@
/** @file
- EBC-specific functionality for DxeLoad.
+ Generic version of arch-specific functionality for DxeLoad.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index 052ea0ec1a6f2771..60c998be6c1bad01 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -45,17 +45,11 @@ [Sources.X64]
X64/VirtualMemory.c
X64/DxeLoadFunc.c
-[Sources.EBC]
- Ebc/DxeLoadFunc.c
-
[Sources.ARM, Sources.AARCH64]
Arm/DxeLoadFunc.c
-[Sources.RISCV64]
- RiscV64/DxeLoadFunc.c
-
-[Sources.LOONGARCH64]
- LoongArch64/DxeLoadFunc.c
+[Sources.LOONGARCH64,Sources.RISCV64,Sources.EBC]
+ DxeHandoff.c
[Packages]
MdePkg/MdePkg.dec
diff --git a/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
deleted file mode 100644
index 95d3af19ea4c9f00..0000000000000000
--- a/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
+++ /dev/null
@@ -1,63 +0,0 @@
-/** @file
- LoongArch specifc functionality for DxeLoad.
-
- Copyright (c) 2022, Loongson Technology Corporation Limited. All rights reserved.<BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include "DxeIpl.h"
-
-/**
- Transfers control to DxeCore.
-
- This function performs a CPU architecture specific operations to execute
- the entry point of DxeCore with the parameters of HobList.
- It also installs EFI_END_OF_PEI_PPI to signal the end of PEI phase.
-
- @param[in] DxeCoreEntryPoint The entry point of DxeCore.
- @param[in] HobList The start of HobList passed to DxeCore.
-
-**/
-VOID
-HandOffToDxeCore (
- IN EFI_PHYSICAL_ADDRESS DxeCoreEntryPoint,
- IN EFI_PEI_HOB_POINTERS HobList
- )
-{
- VOID *BaseOfStack;
- VOID *TopOfStack;
- EFI_STATUS Status;
-
- //
- // Allocate 128KB for the Stack
- //
- BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
- ASSERT (BaseOfStack != NULL);
-
- //
- // Compute the top of the stack we were allocated. Pre-allocate a UINTN
- // for safety.
- //
- TopOfStack = (VOID *)((UINTN)BaseOfStack + EFI_SIZE_TO_PAGES (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);
- TopOfStack = ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
-
- //
- // End of PEI phase signal
- //
- Status = PeiServicesInstallPpi (&gEndOfPeiSignalPpi);
- ASSERT_EFI_ERROR (Status);
-
- //
- // Update the contents of BSP stack HOB to reflect the real stack info passed to DxeCore.
- //
- UpdateStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)BaseOfStack, STACK_SIZE);
-
- SwitchStack (
- (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeCoreEntryPoint,
- HobList.Raw,
- NULL,
- TopOfStack
- );
-}
diff --git a/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
deleted file mode 100644
index b3567d88f73467e7..0000000000000000
--- a/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
+++ /dev/null
@@ -1,75 +0,0 @@
-/** @file
- RISC-V specific functionality for DxeLoad.
-
- Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include "DxeIpl.h"
-
-/**
- Transfers control to DxeCore.
-
- This function performs a CPU architecture specific operations to execute
- the entry point of DxeCore with the parameters of HobList.
- It also installs EFI_END_OF_PEI_PPI to signal the end of PEI phase.
-
- @param DxeCoreEntryPoint The entry point of DxeCore.
- @param HobList The start of HobList passed to DxeCore.
-
-**/
-VOID
-HandOffToDxeCore (
- IN EFI_PHYSICAL_ADDRESS DxeCoreEntryPoint,
- IN EFI_PEI_HOB_POINTERS HobList
- )
-{
- VOID *BaseOfStack;
- VOID *TopOfStack;
- EFI_STATUS Status;
-
- //
- //
- // Allocate 128KB for the Stack
- //
- BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
- if (BaseOfStack == NULL) {
- DEBUG ((DEBUG_ERROR, "%a: Can't allocate memory for stack.", __func__));
- ASSERT (FALSE);
- }
-
- //
- // Compute the top of the stack we were allocated. Pre-allocate a UINTN
- // for safety.
- //
- TopOfStack = (VOID *)((UINTN)BaseOfStack + EFI_SIZE_TO_PAGES (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);
- TopOfStack = ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
-
- //
- // End of PEI phase signal
- //
- Status = PeiServicesInstallPpi (&gEndOfPeiSignalPpi);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "%a: Fail to signal End of PEI event.", __func__));
- ASSERT (FALSE);
- }
-
- //
- // Update the contents of BSP stack HOB to reflect the real stack info passed to DxeCore.
- //
- UpdateStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)BaseOfStack, STACK_SIZE);
-
- DEBUG ((DEBUG_INFO, "DXE Core new stack at %x, stack pointer at %x\n", BaseOfStack, TopOfStack));
-
- //
- // Transfer the control to the entry point of DxeCore.
- //
- SwitchStack (
- (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeCoreEntryPoint,
- HobList.Raw,
- NULL,
- TopOfStack
- );
-}
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/7] MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack NX
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 1/7] MdeModulePkg: Define memory attribute PPI Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 2/7] MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code Ard Biesheuvel
@ 2023-06-02 15:17 ` Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 4/7] ArmPkg/ArmMmuLib: Extend API to manage memory permissions better Ard Biesheuvel
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-02 15:17 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
If the associated PCD is set to TRUE, use the memory attribute PPI to
remap the stack non-executable. This provides a generic method for doing
so, which will be used by ARM and AArch64 as well once they move to the
generic DxeIpl handoff implementation.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c | 29 ++++++++++++++++++--
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 5 +++-
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c b/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
index a0f85ebea56e6cba..60400da3521a8272 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
@@ -2,12 +2,15 @@
Generic version of arch-specific functionality for DxeLoad.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2023, Google, LLC. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include "DxeIpl.h"
+#include <Ppi/MemoryAttribute.h>
+
/**
Transfers control to DxeCore.
@@ -25,9 +28,10 @@ HandOffToDxeCore (
IN EFI_PEI_HOB_POINTERS HobList
)
{
- VOID *BaseOfStack;
- VOID *TopOfStack;
- EFI_STATUS Status;
+ VOID *BaseOfStack;
+ VOID *TopOfStack;
+ EFI_STATUS Status;
+ EDKII_MEMORY_ATTRIBUTE_PPI *MemoryPpi;
//
// Allocate 128KB for the Stack
@@ -35,6 +39,25 @@ HandOffToDxeCore (
BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
ASSERT (BaseOfStack != NULL);
+ if (PcdGetBool (PcdSetNxForStack)) {
+ Status = PeiServicesLocatePpi (
+ &gEdkiiMemoryAttributePpiGuid,
+ 0,
+ NULL,
+ (VOID **)&MemoryPpi
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = MemoryPpi->SetPermissions (
+ MemoryPpi,
+ (UINTN)BaseOfStack,
+ STACK_SIZE,
+ EFI_MEMORY_XP,
+ EFI_MEMORY_XP
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
+
//
// Compute the top of the stack we were allocated. Pre-allocate a UINTN
// for safety.
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index 60c998be6c1bad01..7126a96d8378d1f8 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -91,6 +91,7 @@ [Ppis]
gEfiPeiMemoryDiscoveredPpiGuid ## SOMETIMES_CONSUMES
gEdkiiPeiBootInCapsuleOnDiskModePpiGuid ## SOMETIMES_CONSUMES
gEdkiiPeiCapsuleOnDiskPpiGuid ## SOMETIMES_CONSUMES # Consumed on firmware update boot path
+ gEdkiiMemoryAttributePpiGuid ## SOMETIMES_CONSUMES
[Guids]
## SOMETIMES_CONSUMES ## Variable:L"MemoryTypeInformation"
@@ -117,10 +118,12 @@ [Pcd.IA32,Pcd.X64]
gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize ## CONSUMES
[Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
- gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## SOMETIMES_CONSUMES
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES
+
[Depex]
gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 4/7] ArmPkg/ArmMmuLib: Extend API to manage memory permissions better
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
` (2 preceding siblings ...)
2023-06-02 15:17 ` [PATCH v2 3/7] MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack NX Ard Biesheuvel
@ 2023-06-02 15:17 ` Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 5/7] ArmPkg/CpuPei: Implement the memory attributes PPI Ard Biesheuvel
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-02 15:17 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
Currently, ArmSetMemoryAttributes () takes a combination of
EFI_MEMORY_xx constants describing the memory type and permission
attributes that should be set on a region of memory. In cases where the
memory type is omitted, we assume that the memory permissions being set
are final, and that existing memory permissions can be discarded.
This is problematic, because we aim to map memory non-executable
(EFI_MEMORY_XP) by default, and only relax this requirement for code
regions that are mapped read-only (EFI_MEMORY_RO). Currently, setting
one permission clears the other, and so code managing these permissions
has to be aware of the existing permissions in order to be able to
preserve them, and this is not always tractable (e.g., the UEFI memory
attribute protocol implements an abstraction that promises to preserve
memory permissions that it is not operating on explicitly).
So let's add an AttributeMask parameter to ArmSetMemoryAttributes(),
which is permitted to be non-zero if no memory type is being provided,
in which case only memory permission attributes covered in the mask will
be affected by the update.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
ArmPkg/Include/Library/ArmMmuLib.h | 36 +++++++-
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 52 +++++++++++-
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 88 +++++++++++++++++---
ArmPkg/Library/OpteeLib/Optee.c | 2 +-
5 files changed, 165 insertions(+), 15 deletions(-)
diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
index 2e73719dce04ceb5..2d60c7d24dc05ee9 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
@@ -217,7 +217,7 @@ CpuSetMemoryAttributes (
if (EFI_ERROR (Status) || (RegionArmAttributes != ArmAttributes) ||
((BaseAddress + Length) > (RegionBaseAddress + RegionLength)))
{
- return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes);
+ return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
} else {
return EFI_SUCCESS;
}
diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/ArmMmuLib.h
index 4cf59a1e376b123c..91d112314fdf4859 100644
--- a/ArmPkg/Include/Library/ArmMmuLib.h
+++ b/ArmPkg/Include/Library/ArmMmuLib.h
@@ -92,11 +92,45 @@ ArmReplaceLiveTranslationEntry (
IN BOOLEAN DisableMmu
);
+/**
+ Set the requested memory permission attributes on a region of memory.
+
+ BaseAddress and Length must be aligned to EFI_PAGE_SIZE.
+
+ If Attributes contains a memory type attribute (EFI_MEMORY_UC/WC/WT/WB), the
+ region is mapped according to this memory type, and additional memory
+ permission attributes (EFI_MEMORY_RP/RO/XP) are taken into account as well,
+ discarding any permission attributes that are currently set for the region.
+ AttributeMask is ignored in this case, and must be set to 0x0.
+
+ If Attributes contains only a combination of memory permission attributes
+ (EFI_MEMORY_RP/RO/XP), each page in the region will retain its existing
+ memory type, even if it is not uniformly set across the region. In this case,
+ AttributesMask may be set to a mask of permission attributes, and memory
+ permissions omitted from this mask will not be updated for any page in the
+ region. All attributes appearing in Attributes must appear in AttributeMask
+ as well. (Attributes & ~AttributeMask must produce 0x0)
+
+ @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 Mask of memory attributes to set.
+ @param[in] AttributeMask Mask of memory attributes to take into account.
+
+ @retval EFI_SUCCESS The attributes were set for the memory region.
+ @retval EFI_INVALID_PARAMETER BaseAddress or Length is not suitably aligned.
+ Invalid combination of Attributes and
+ AttributeMask.
+ @retval EFI_OUT_OF_RESOURCES Requested attributes cannot be applied due to
+ lack of system resources.
+
+**/
EFI_STATUS
ArmSetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
- IN UINT64 Attributes
+ IN UINT64 Attributes,
+ IN UINT64 AttributeMask
);
#endif // ARM_MMU_LIB_H_
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 7ed758fbbc699732..22623572b9cb931c 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -469,11 +469,45 @@ GcdAttributeToPageAttribute (
return PageAttributes;
}
+/**
+ Set the requested memory permission attributes on a region of memory.
+
+ BaseAddress and Length must be aligned to EFI_PAGE_SIZE.
+
+ If Attributes contains a memory type attribute (EFI_MEMORY_UC/WC/WT/WB), the
+ region is mapped according to this memory type, and additional memory
+ permission attributes (EFI_MEMORY_RP/RO/XP) are taken into account as well,
+ discarding any permission attributes that are currently set for the region.
+ AttributeMask is ignored in this case, and must be set to 0x0.
+
+ If Attributes contains only a combination of memory permission attributes
+ (EFI_MEMORY_RP/RO/XP), each page in the region will retain its existing
+ memory type, even if it is not uniformly set across the region. In this case,
+ AttributesMask may be set to a mask of permission attributes, and memory
+ permissions omitted from this mask will not be updated for any page in the
+ region. All attributes appearing in Attributes must appear in AttributeMask
+ as well. (Attributes & ~AttributeMask must produce 0x0)
+
+ @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 Mask of memory attributes to set.
+ @param[in] AttributeMask Mask of memory attributes to take into account.
+
+ @retval EFI_SUCCESS The attributes were set for the memory region.
+ @retval EFI_INVALID_PARAMETER BaseAddress or Length is not suitably aligned.
+ Invalid combination of Attributes and
+ AttributeMask.
+ @retval EFI_OUT_OF_RESOURCES Requested attributes cannot be applied due to
+ lack of system resources.
+
+**/
EFI_STATUS
ArmSetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
- IN UINT64 Attributes
+ IN UINT64 Attributes,
+ IN UINT64 AttributeMask
)
{
UINT64 PageAttributes;
@@ -490,6 +524,22 @@ ArmSetMemoryAttributes (
PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK | TT_AF;
PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK |
TT_PXN_MASK | TT_XN_MASK | TT_AF);
+ if (AttributeMask != 0) {
+ if (((AttributeMask & ~(UINT64)(EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP)) != 0) ||
+ ((Attributes & ~AttributeMask) != 0))
+ {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // Add attributes omitted from AttributeMask to the set of attributes to preserve
+ PageAttributeMask |= GcdAttributeToPageAttribute (~AttributeMask) &
+ (TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK | TT_AF);
+ }
+ } else {
+ ASSERT (AttributeMask == 0);
+ if (AttributeMask != 0) {
+ return EFI_INVALID_PARAMETER;
+ }
}
return UpdateRegionMapping (
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
index 299d38ad07e85059..61405965a73eaeb8 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
@@ -10,6 +10,7 @@
#include <Uefi.h>
#include <Library/ArmLib.h>
+#include <Library/ArmMmuLib.h>
#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
@@ -451,31 +452,96 @@ SetMemoryAttributes (
}
/**
- Update the permission or memory type attributes on a range of memory.
+ Set the requested memory permission attributes on a region of memory.
- @param BaseAddress The start of the region.
- @param Length The size of the region.
- @param Attributes A mask of EFI_MEMORY_xx constants.
+ BaseAddress and Length must be aligned to EFI_PAGE_SIZE.
- @retval EFI_SUCCESS The attributes were set successfully.
- @retval EFI_OUT_OF_RESOURCES The operation failed due to insufficient memory.
+ If Attributes contains a memory type attribute (EFI_MEMORY_UC/WC/WT/WB), the
+ region is mapped according to this memory type, and additional memory
+ permission attributes (EFI_MEMORY_RP/RO/XP) are taken into account as well,
+ discarding any permission attributes that are currently set for the region.
+ AttributeMask is ignored in this case, and must be set to 0x0.
+
+ If Attributes contains only a combination of memory permission attributes
+ (EFI_MEMORY_RP/RO/XP), each page in the region will retain its existing
+ memory type, even if it is not uniformly set across the region. In this case,
+ AttributesMask may be set to a mask of permission attributes, and memory
+ permissions omitted from this mask will not be updated for any page in the
+ region. All attributes appearing in Attributes must appear in AttributeMask
+ as well. (Attributes & ~AttributeMask must produce 0x0)
+
+ @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 Mask of memory attributes to set.
+ @param[in] AttributeMask Mask of memory attributes to take into account.
+
+ @retval EFI_SUCCESS The attributes were set for the memory region.
+ @retval EFI_INVALID_PARAMETER BaseAddress or Length is not suitably aligned.
+ Invalid combination of Attributes and
+ AttributeMask.
+ @retval EFI_OUT_OF_RESOURCES Requested attributes cannot be applied due to
+ lack of system resources.
**/
EFI_STATUS
ArmSetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
- IN UINT64 Attributes
+ IN UINT64 Attributes,
+ IN UINT64 AttributeMask
)
{
+ UINT32 TtEntryMask;
+
+ if (((BaseAddress | Length) & EFI_PAGE_MASK) != 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if ((Attributes & EFI_MEMORY_CACHETYPE_MASK) == 0) {
+ //
+ // No memory type was set in Attributes, so we are going to update the
+ // permissions only.
+ //
+ if (AttributeMask != 0) {
+ if (((AttributeMask & ~(UINT64)(EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP)) != 0) ||
+ ((Attributes & ~AttributeMask) != 0))
+ {
+ return EFI_INVALID_PARAMETER;
+ }
+ } else {
+ AttributeMask = EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP;
+ }
+
+ TtEntryMask = 0;
+ if ((AttributeMask & EFI_MEMORY_RP) != 0) {
+ TtEntryMask |= TT_DESCRIPTOR_SECTION_AF;
+ }
+
+ if ((AttributeMask & EFI_MEMORY_RO) != 0) {
+ TtEntryMask |= TT_DESCRIPTOR_SECTION_AP_MASK;
+ }
+
+ if ((AttributeMask & EFI_MEMORY_XP) != 0) {
+ TtEntryMask |= TT_DESCRIPTOR_SECTION_XN_MASK;
+ }
+ } else {
+ ASSERT (AttributeMask == 0);
+ if (AttributeMask != 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ TtEntryMask = TT_DESCRIPTOR_SECTION_TYPE_MASK |
+ TT_DESCRIPTOR_SECTION_XN_MASK |
+ TT_DESCRIPTOR_SECTION_AP_MASK |
+ TT_DESCRIPTOR_SECTION_AF;
+ }
+
return SetMemoryAttributes (
BaseAddress,
Length,
Attributes,
- TT_DESCRIPTOR_SECTION_TYPE_MASK |
- TT_DESCRIPTOR_SECTION_XN_MASK |
- TT_DESCRIPTOR_SECTION_AP_MASK |
- TT_DESCRIPTOR_SECTION_AF
+ TtEntryMask
);
}
diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Optee.c
index 48e33cb3d5ee4ab6..46464f17ef06653e 100644
--- a/ArmPkg/Library/OpteeLib/Optee.c
+++ b/ArmPkg/Library/OpteeLib/Optee.c
@@ -86,7 +86,7 @@ OpteeSharedMemoryRemap (
return EFI_BUFFER_TOO_SMALL;
}
- Status = ArmSetMemoryAttributes (PhysicalAddress, Size, EFI_MEMORY_WB);
+ Status = ArmSetMemoryAttributes (PhysicalAddress, Size, EFI_MEMORY_WB, 0);
if (EFI_ERROR (Status)) {
return Status;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 5/7] ArmPkg/CpuPei: Implement the memory attributes PPI
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
` (3 preceding siblings ...)
2023-06-02 15:17 ` [PATCH v2 4/7] ArmPkg/ArmMmuLib: Extend API to manage memory permissions better Ard Biesheuvel
@ 2023-06-02 15:17 ` Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 6/7] MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code Ard Biesheuvel
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-02 15:17 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
Implement the newly defined PPI that permits the PEI core and DXE IPL to
manage memory permissions on ranges of DRAM, for doing things like
mapping the stack non-executable, or granting executable permissions to
shadowed PEIMs.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
ArmPkg/Drivers/CpuPei/CpuPei.c | 76 ++++++++++++++++++++
ArmPkg/Drivers/CpuPei/CpuPei.inf | 4 ++
2 files changed, 80 insertions(+)
diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.c b/ArmPkg/Drivers/CpuPei/CpuPei.c
index 85ef5ec07b9fdafa..1c2b53100f6a424e 100644
--- a/ArmPkg/Drivers/CpuPei/CpuPei.c
+++ b/ArmPkg/Drivers/CpuPei/CpuPei.c
@@ -3,6 +3,7 @@
Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2011 Hewlett Packard Corporation. All rights reserved.<BR>
Copyright (c) 2011-2013, ARM Limited. All rights reserved.<BR>
+Copyright (c) 2023, Google, LLC. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -24,6 +25,7 @@ Module Name:
// The protocols, PPI and GUID definitions for this module
//
#include <Ppi/ArmMpCoreInfo.h>
+#include <Ppi/MemoryAttribute.h>
//
// The Library classes this module consumes
@@ -34,6 +36,77 @@ Module Name:
#include <Library/PcdLib.h>
#include <Library/HobLib.h>
#include <Library/ArmLib.h>
+#include <Library/ArmMmuLib.h>
+
+/**
+ Set the requested memory permission attributes on a region of memory.
+
+ BaseAddress and Length must be aligned to EFI_PAGE_SIZE.
+
+ Attributes must contain a combination of EFI_MEMORY_RP, EFI_MEMORY_RO and
+ EFI_MEMORY_XP, and specifies the attributes that must be set for the
+ region in question. Attributes that are omitted will be cleared from the
+ region only if they are set in AttributeMask.
+
+ AttributeMask must contain a combination of EFI_MEMORY_RP, EFI_MEMORY_RO and
+ EFI_MEMORY_XP, and specifies the attributes that the call will operate on.
+ AttributeMask must not be 0x0, and must contain at least the bits set in
+ Attributes.
+
+ @param[in] This The protocol instance pointer.
+ @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 Memory attributes to set or clear.
+ @param[in] AttributeMask Mask of memory attributes to operate on.
+
+ @retval EFI_SUCCESS The attributes were set for the memory region.
+ @retval EFI_INVALID_PARAMETER Length is zero.
+ AttributeMask is zero.
+ AttributeMask lacks bits set in Attributes.
+ BaseAddress or Length is not suitably aligned.
+ @retval EFI_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 supported for
+ the memory resource range specified by
+ BaseAddress and Length.
+ @retval EFI_OUT_OF_RESOURCES Requested attributes cannot be applied due to
+ lack of system resources.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+SetMemoryPermissions (
+ IN EDKII_MEMORY_ATTRIBUTE_PPI *This,
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN UINT64 AttributeMask
+ )
+{
+ if ((Length == 0) ||
+ (AttributeMask == 0) ||
+ ((AttributeMask & (EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0) ||
+ ((Attributes & ~AttributeMask) != 0) ||
+ (((BaseAddress | Length) & EFI_PAGE_MASK) != 0))
+ {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ return ArmSetMemoryAttributes (BaseAddress, Length, Attributes, AttributeMask);
+}
+
+STATIC CONST EDKII_MEMORY_ATTRIBUTE_PPI mMemoryAttributePpi = {
+ SetMemoryPermissions
+};
+
+STATIC CONST EFI_PEI_PPI_DESCRIPTOR mMemoryAttributePpiDesc = {
+ (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+ &gEdkiiMemoryAttributePpiGuid,
+ (VOID *)&mMemoryAttributePpi
+};
/*++
@@ -79,5 +152,8 @@ InitializeCpuPeim (
}
}
+ Status = PeiServicesInstallPpi (&mMemoryAttributePpiDesc);
+ ASSERT_EFI_ERROR (Status);
+
return EFI_SUCCESS;
}
diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.inf b/ArmPkg/Drivers/CpuPei/CpuPei.inf
index a9f85cbc68b1c52e..49b67077ec6166f1 100644
--- a/ArmPkg/Drivers/CpuPei/CpuPei.inf
+++ b/ArmPkg/Drivers/CpuPei/CpuPei.inf
@@ -3,6 +3,7 @@
#
# This module provides platform specific function to detect boot mode.
# Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2023, Google, LLC. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -28,6 +29,7 @@ [Sources]
CpuPei.c
[Packages]
+ MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
EmbeddedPkg/EmbeddedPkg.dec
ArmPkg/ArmPkg.dec
@@ -37,9 +39,11 @@ [LibraryClasses]
DebugLib
HobLib
ArmLib
+ ArmMmuLib
[Ppis]
gArmMpCoreInfoPpiGuid
+ gEdkiiMemoryAttributePpiGuid
[Guids]
gArmMpCoreInfoGuid
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 6/7] MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
` (4 preceding siblings ...)
2023-06-02 15:17 ` [PATCH v2 5/7] ArmPkg/CpuPei: Implement the memory attributes PPI Ard Biesheuvel
@ 2023-06-02 15:17 ` Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 7/7] ArmPkg/CpuDxe: Simplify memory attributes protocol implementation Ard Biesheuvel
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-02 15:17 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
Now that we have a generic method to manage memory permissions using a
PPI, we can switch to the generic version of the DXE handoff code in
DxeIpl, and drop the ARM specific version.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 71 --------------------
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 11 +--
2 files changed, 1 insertion(+), 81 deletions(-)
diff --git a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
deleted file mode 100644
index f62b6dcb38a702d7..0000000000000000
--- a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
+++ /dev/null
@@ -1,71 +0,0 @@
-/** @file
- ARM specifc functionality for DxeLoad.
-
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
-Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
-
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include "DxeIpl.h"
-
-#include <Library/ArmMmuLib.h>
-
-/**
- Transfers control to DxeCore.
-
- This function performs a CPU architecture specific operations to execute
- the entry point of DxeCore with the parameters of HobList.
- It also installs EFI_END_OF_PEI_PPI to signal the end of PEI phase.
-
- @param DxeCoreEntryPoint The entry point of DxeCore.
- @param HobList The start of HobList passed to DxeCore.
-
-**/
-VOID
-HandOffToDxeCore (
- IN EFI_PHYSICAL_ADDRESS DxeCoreEntryPoint,
- IN EFI_PEI_HOB_POINTERS HobList
- )
-{
- VOID *BaseOfStack;
- VOID *TopOfStack;
- EFI_STATUS Status;
-
- //
- // Allocate 128KB for the Stack
- //
- BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
- ASSERT (BaseOfStack != NULL);
-
- if (PcdGetBool (PcdSetNxForStack)) {
- Status = ArmSetMemoryRegionNoExec ((UINTN)BaseOfStack, STACK_SIZE);
- ASSERT_EFI_ERROR (Status);
- }
-
- //
- // Compute the top of the stack we were allocated. Pre-allocate a UINTN
- // for safety.
- //
- TopOfStack = (VOID *)((UINTN)BaseOfStack + EFI_SIZE_TO_PAGES (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);
- TopOfStack = ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
-
- //
- // End of PEI phase singal
- //
- Status = PeiServicesInstallPpi (&gEndOfPeiSignalPpi);
- ASSERT_EFI_ERROR (Status);
-
- //
- // Update the contents of BSP stack HOB to reflect the real stack info passed to DxeCore.
- //
- UpdateStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)BaseOfStack, STACK_SIZE);
-
- SwitchStack (
- (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeCoreEntryPoint,
- HobList.Raw,
- NULL,
- TopOfStack
- );
-}
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index 7126a96d8378d1f8..f1990eac77607854 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -45,19 +45,13 @@ [Sources.X64]
X64/VirtualMemory.c
X64/DxeLoadFunc.c
-[Sources.ARM, Sources.AARCH64]
- Arm/DxeLoadFunc.c
-
-[Sources.LOONGARCH64,Sources.RISCV64,Sources.EBC]
+[Sources.LOONGARCH64,Sources.RISCV64,Sources.EBC,Sources.ARM,Sources.AARCH64]
DxeHandoff.c
[Packages]
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
-[Packages.ARM, Packages.AARCH64]
- ArmPkg/ArmPkg.dec
-
[LibraryClasses]
PcdLib
MemoryAllocationLib
@@ -74,9 +68,6 @@ [LibraryClasses]
PeiServicesTablePointerLib
PerformanceLib
-[LibraryClasses.ARM, LibraryClasses.AARCH64]
- ArmMmuLib
-
[Ppis]
gEfiDxeIplPpiGuid ## PRODUCES
gEfiPeiDecompressPpiGuid ## PRODUCES
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 7/7] ArmPkg/CpuDxe: Simplify memory attributes protocol implementation
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
` (5 preceding siblings ...)
2023-06-02 15:17 ` [PATCH v2 6/7] MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code Ard Biesheuvel
@ 2023-06-02 15:17 ` Ard Biesheuvel
2023-06-06 16:42 ` [edk2-devel] [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Oliver Smith-Denny
2023-06-06 23:12 ` Michael Kubacki
8 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-02 15:17 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
Now that ArmSetMemoryAttributes() permits a mask to be provided, we can
simplify the implementation the UEFI memory attribute protocol
substantially, and just pass on the requested mask to be set or cleared
directly.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
ArmPkg/Drivers/CpuDxe/MemoryAttribute.c | 50 +-------------------
1 file changed, 2 insertions(+), 48 deletions(-)
diff --git a/ArmPkg/Drivers/CpuDxe/MemoryAttribute.c b/ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
index 61ba8fbbae4ee795..16cc4ef474f9772b 100644
--- a/ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
+++ b/ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
@@ -183,8 +183,6 @@ SetMemoryAttributes (
IN UINT64 Attributes
)
{
- EFI_STATUS Status;
-
DEBUG ((
DEBUG_INFO,
"%a: BaseAddress == 0x%lx, Length == 0x%lx, Attributes == 0x%lx\n",
@@ -204,28 +202,7 @@ SetMemoryAttributes (
return EFI_UNSUPPORTED;
}
- if ((Attributes & EFI_MEMORY_RP) != 0) {
- Status = ArmSetMemoryRegionNoAccess (BaseAddress, Length);
- if (EFI_ERROR (Status)) {
- return EFI_UNSUPPORTED;
- }
- }
-
- if ((Attributes & EFI_MEMORY_RO) != 0) {
- Status = ArmSetMemoryRegionReadOnly (BaseAddress, Length);
- if (EFI_ERROR (Status)) {
- return EFI_UNSUPPORTED;
- }
- }
-
- if ((Attributes & EFI_MEMORY_XP) != 0) {
- Status = ArmSetMemoryRegionNoExec (BaseAddress, Length);
- if (EFI_ERROR (Status)) {
- return EFI_UNSUPPORTED;
- }
- }
-
- return EFI_SUCCESS;
+ return ArmSetMemoryAttributes (BaseAddress, Length, Attributes, Attributes);
}
/**
@@ -267,8 +244,6 @@ ClearMemoryAttributes (
IN UINT64 Attributes
)
{
- EFI_STATUS Status;
-
DEBUG ((
DEBUG_INFO,
"%a: BaseAddress == 0x%lx, Length == 0x%lx, Attributes == 0x%lx\n",
@@ -288,28 +263,7 @@ ClearMemoryAttributes (
return EFI_UNSUPPORTED;
}
- if ((Attributes & EFI_MEMORY_RP) != 0) {
- Status = ArmClearMemoryRegionNoAccess (BaseAddress, Length);
- if (EFI_ERROR (Status)) {
- return EFI_UNSUPPORTED;
- }
- }
-
- if ((Attributes & EFI_MEMORY_RO) != 0) {
- Status = ArmClearMemoryRegionReadOnly (BaseAddress, Length);
- if (EFI_ERROR (Status)) {
- return EFI_UNSUPPORTED;
- }
- }
-
- if ((Attributes & EFI_MEMORY_XP) != 0) {
- Status = ArmClearMemoryRegionNoExec (BaseAddress, Length);
- if (EFI_ERROR (Status)) {
- return EFI_UNSUPPORTED;
- }
- }
-
- return EFI_SUCCESS;
+ return ArmSetMemoryAttributes (BaseAddress, Length, 0, Attributes);
}
EFI_MEMORY_ATTRIBUTE_PROTOCOL mMemoryAttribute = {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
` (6 preceding siblings ...)
2023-06-02 15:17 ` [PATCH v2 7/7] ArmPkg/CpuDxe: Simplify memory attributes protocol implementation Ard Biesheuvel
@ 2023-06-06 16:42 ` Oliver Smith-Denny
2023-06-06 23:12 ` Michael Kubacki
8 siblings, 0 replies; 15+ messages in thread
From: Oliver Smith-Denny @ 2023-06-06 16:42 UTC (permalink / raw)
To: devel, ardb
Cc: Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm, Michael Kubacki
Hi Ard,
This makes sense to me to get in as we discuss other elements
of the memory protection story.
For the patchset:
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Thanks!
On 6/2/2023 8:17 AM, Ard Biesheuvel wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4468
>
>
>
> This is a followup to the RFC that I sent to the edk2-devel list on the
>
> 25th of May.
>
>
>
> In an attempt to make some incremental progress, this v2 only covers the
>
> NX remapping of the DXE stack in DxeIpl, using the newly introduced
>
> memory attributes PPI.
>
>
>
> Other use cases are deferred until we can converge on an approach that
>
> works across architectures and platforms. In particular, this means the
>
> following use cases:
>
> - mapping the DXE core code and data regions RO and XP, respectively;
>
> - mapping shadowed PEIMs read-only (including the PEI core itself);
>
> - managing memory permissions after temporary RAM migration;
>
> - reorganizing the X64 PEI flow with respect to page table allocation;
>
> - managing the dispatch order of the PEIM producing the PPI in relation
>
> to its consumers.
>
>
>
> The current series specifies the PPI in patch #1, and wires it up into
>
> DxeIpl to remap the DXE stack non-executable in a generic manner
>
> (patches #2 and #3)
>
>
>
> Patches #4 and #5 implement the PPI for ARM and AArch64.
>
>
>
> Patch #6 switches ARM and AArch64 over to the generic DxeIpl.
>
>
>
> Patch #7 cleans up the ARM implementation of the UEFI memory attributes
>
> protocol, based on the improvements made in patch #4.
>
>
>
> Changes since RFC (in addition to the above):
>
> - update PPI protype to use attributes+mask instead of setmask+clearmask
>
> - drop OVMF patch for RISC-V that has been applied in the meantime
>
>
>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
>
> Cc: Taylor Beebe <t@taylorbeebe.com>
>
> Cc: Oliver Smith-Denny <osd@smith-denny.com>
>
> Cc: Dandan Bi <dandan.bi@intel.com>
>
> Cc: Dun Tan <dun.tan@intel.com>
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>
> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>
>
>
> Ard Biesheuvel (7):
>
> MdeModulePkg: Define memory attribute PPI
>
> MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
>
> MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack NX
>
> ArmPkg/ArmMmuLib: Extend API to manage memory permissions better
>
> ArmPkg/CpuPei: Implement the memory attributes PPI
>
> MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code
>
> ArmPkg/CpuDxe: Simplify memory attributes protocol implementation
>
>
>
> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
>
> ArmPkg/Drivers/CpuDxe/MemoryAttribute.c | 50 +----------
>
> ArmPkg/Drivers/CpuPei/CpuPei.c | 76 +++++++++++++++++
>
> ArmPkg/Drivers/CpuPei/CpuPei.inf | 4 +
>
> ArmPkg/Include/Library/ArmMmuLib.h | 36 +++++++-
>
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 52 +++++++++++-
>
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 88 +++++++++++++++++---
>
> ArmPkg/Library/OpteeLib/Optee.c | 2 +-
>
> MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 71 ----------------
>
> MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} | 31 ++++++-
>
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 24 ++----
>
> MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c | 63 --------------
>
> MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c | 75 -----------------
>
> MdeModulePkg/Include/Ppi/MemoryAttribute.h | 83 ++++++++++++++++++
>
> MdeModulePkg/MdeModulePkg.dec | 3 +
>
> 15 files changed, 366 insertions(+), 294 deletions(-)
>
> delete mode 100644 MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
>
> rename MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} (62%)
>
> delete mode 100644 MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
>
> delete mode 100644 MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
>
> create mode 100644 MdeModulePkg/Include/Ppi/MemoryAttribute.h
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/7] MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
2023-06-02 15:17 ` [PATCH v2 2/7] MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code Ard Biesheuvel
@ 2023-06-06 18:48 ` Michael Kubacki
2023-06-06 22:17 ` Ard Biesheuvel
0 siblings, 1 reply; 15+ messages in thread
From: Michael Kubacki @ 2023-06-06 18:48 UTC (permalink / raw)
To: devel, ardb
Cc: Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm
On 6/2/2023 11:17 AM, Ard Biesheuvel wrote:
> The Risc-V and LoongArch specific versions of the DXE core handoff code
> in DxeIpl are essentially copies of the EBC version (modulo the
> copyright in the header and some debug prints in the code).
>
> In preparation for introducing a generic PPI based method to implement
> the non-executable stack, let's merge these versions, so we only need to
> add this logic once.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} | 2 +-
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 10 +--
> MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c | 63 ----------------
> MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c | 75 --------------------
> 4 files changed, 3 insertions(+), 147 deletions(-)
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/Ebc/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
> similarity index 92%
> rename from MdeModulePkg/Core/DxeIplPeim/Ebc/DxeLoadFunc.c
> rename to MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
> index c1a16b602452218e..a0f85ebea56e6cba 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/Ebc/DxeLoadFunc.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
> @@ -1,5 +1,5 @@
> /** @file
>
> - EBC-specific functionality for DxeLoad.
>
> + Generic version of arch-specific functionality for DxeLoad.
>
>
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 052ea0ec1a6f2771..60c998be6c1bad01 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -45,17 +45,11 @@ [Sources.X64]
> X64/VirtualMemory.c
>
> X64/DxeLoadFunc.c
>
>
>
> -[Sources.EBC]
>
> - Ebc/DxeLoadFunc.c
>
> -
>
> [Sources.ARM, Sources.AARCH64]
>
> Arm/DxeLoadFunc.c
>
>
>
> -[Sources.RISCV64]
>
> - RiscV64/DxeLoadFunc.c
>
> -
>
> -[Sources.LOONGARCH64]
>
> - LoongArch64/DxeLoadFunc.c
>
> +[Sources.LOONGARCH64,Sources.RISCV64,Sources.EBC]
>
> + DxeHandoff.c
>
>
>
> [Packages]
>
> MdePkg/MdePkg.dec
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
> deleted file mode 100644
> index 95d3af19ea4c9f00..0000000000000000
> --- a/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
> +++ /dev/null
> @@ -1,63 +0,0 @@
> -/** @file
>
> - LoongArch specifc functionality for DxeLoad.
>
> -
>
> - Copyright (c) 2022, Loongson Technology Corporation Limited. All rights reserved.<BR>
>
> -
>
> - SPDX-License-Identifier: BSD-2-Clause-Patent
>
> -
>
> -**/
>
> -
>
> -#include "DxeIpl.h"
>
> -
>
> -/**
>
> - Transfers control to DxeCore.
>
> -
>
> - This function performs a CPU architecture specific operations to execute
>
> - the entry point of DxeCore with the parameters of HobList.
>
> - It also installs EFI_END_OF_PEI_PPI to signal the end of PEI phase.
>
> -
>
> - @param[in] DxeCoreEntryPoint The entry point of DxeCore.
>
> - @param[in] HobList The start of HobList passed to DxeCore.
>
> -
>
> -**/
>
> -VOID
>
> -HandOffToDxeCore (
>
> - IN EFI_PHYSICAL_ADDRESS DxeCoreEntryPoint,
>
> - IN EFI_PEI_HOB_POINTERS HobList
>
> - )
>
> -{
>
> - VOID *BaseOfStack;
>
> - VOID *TopOfStack;
>
> - EFI_STATUS Status;
>
> -
>
> - //
>
> - // Allocate 128KB for the Stack
>
> - //
>
> - BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
>
> - ASSERT (BaseOfStack != NULL);
>
> -
I know this code path is critical to continue boot, but dereferencing a
null pointer if asserts are not active is not a great alternative.
Perhaps a status error code could be reported to allow platforms to
potentially hook onto? Kind of like what is done if the DXE IPL PPI
cannot be found
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#LL518-L522.
>
> - //
>
> - // Compute the top of the stack we were allocated. Pre-allocate a UINTN
>
> - // for safety.
>
> - //
>
> - TopOfStack = (VOID *)((UINTN)BaseOfStack + EFI_SIZE_TO_PAGES (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);
>
> - TopOfStack = ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
>
> -
>
> - //
>
> - // End of PEI phase signal
>
> - //
>
> - Status = PeiServicesInstallPpi (&gEndOfPeiSignalPpi);
>
> - ASSERT_EFI_ERROR (Status);
>
> -
>
> - //
>
> - // Update the contents of BSP stack HOB to reflect the real stack info passed to DxeCore.
>
> - //
>
> - UpdateStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)BaseOfStack, STACK_SIZE);
>
> -
>
> - SwitchStack (
>
> - (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeCoreEntryPoint,
>
> - HobList.Raw,
>
> - NULL,
>
> - TopOfStack
>
> - );
>
> -}
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> deleted file mode 100644
> index b3567d88f73467e7..0000000000000000
> --- a/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> +++ /dev/null
> @@ -1,75 +0,0 @@
> -/** @file
>
> - RISC-V specific functionality for DxeLoad.
>
> -
>
> - Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>
> -
>
> - SPDX-License-Identifier: BSD-2-Clause-Patent
>
> -
>
> -**/
>
> -
>
> -#include "DxeIpl.h"
>
> -
>
> -/**
>
> - Transfers control to DxeCore.
>
> -
>
> - This function performs a CPU architecture specific operations to execute
>
> - the entry point of DxeCore with the parameters of HobList.
>
> - It also installs EFI_END_OF_PEI_PPI to signal the end of PEI phase.
>
> -
>
> - @param DxeCoreEntryPoint The entry point of DxeCore.
>
> - @param HobList The start of HobList passed to DxeCore.
>
> -
>
> -**/
>
> -VOID
>
> -HandOffToDxeCore (
>
> - IN EFI_PHYSICAL_ADDRESS DxeCoreEntryPoint,
>
> - IN EFI_PEI_HOB_POINTERS HobList
>
> - )
>
> -{
>
> - VOID *BaseOfStack;
>
> - VOID *TopOfStack;
>
> - EFI_STATUS Status;
>
> -
>
> - //
>
> - //
>
> - // Allocate 128KB for the Stack
>
> - //
>
> - BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
>
> - if (BaseOfStack == NULL) {
>
> - DEBUG ((DEBUG_ERROR, "%a: Can't allocate memory for stack.", __func__));
>
> - ASSERT (FALSE);
>
> - }
>
> -
>
> - //
>
> - // Compute the top of the stack we were allocated. Pre-allocate a UINTN
>
> - // for safety.
>
> - //
>
> - TopOfStack = (VOID *)((UINTN)BaseOfStack + EFI_SIZE_TO_PAGES (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);
>
> - TopOfStack = ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
>
> -
>
> - //
>
> - // End of PEI phase signal
>
> - //
>
> - Status = PeiServicesInstallPpi (&gEndOfPeiSignalPpi);
>
> - if (EFI_ERROR (Status)) {
>
> - DEBUG ((DEBUG_ERROR, "%a: Fail to signal End of PEI event.", __func__));
>
> - ASSERT (FALSE);
>
> - }
>
> -
>
> - //
>
> - // Update the contents of BSP stack HOB to reflect the real stack info passed to DxeCore.
>
> - //
>
> - UpdateStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)BaseOfStack, STACK_SIZE);
>
> -
>
> - DEBUG ((DEBUG_INFO, "DXE Core new stack at %x, stack pointer at %x\n", BaseOfStack, TopOfStack));
>
> -
>
> - //
>
> - // Transfer the control to the entry point of DxeCore.
>
> - //
>
> - SwitchStack (
>
> - (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeCoreEntryPoint,
>
> - HobList.Raw,
>
> - NULL,
>
> - TopOfStack
>
> - );
>
> -}
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/7] MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
2023-06-06 18:48 ` [edk2-devel] " Michael Kubacki
@ 2023-06-06 22:17 ` Ard Biesheuvel
2023-06-06 23:07 ` Michael Kubacki
0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-06 22:17 UTC (permalink / raw)
To: Michael Kubacki
Cc: devel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm
On Tue, 6 Jun 2023 at 20:48, Michael Kubacki
<mikuback@linux.microsoft.com> wrote:
>
> On 6/2/2023 11:17 AM, Ard Biesheuvel wrote:
> > The Risc-V and LoongArch specific versions of the DXE core handoff code
> > in DxeIpl are essentially copies of the EBC version (modulo the
> > copyright in the header and some debug prints in the code).
> >
> > In preparation for introducing a generic PPI based method to implement
> > the non-executable stack, let's merge these versions, so we only need to
> > add this logic once.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} | 2 +-
> > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 10 +--
> > MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c | 63 ----------------
> > MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c | 75 --------------------
> > 4 files changed, 3 insertions(+), 147 deletions(-)
> >
...
> > diff --git a/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
> > deleted file mode 100644
> > index 95d3af19ea4c9f00..0000000000000000
> > --- a/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
> > +++ /dev/null
> > @@ -1,63 +0,0 @@
> > -/** @file
> >
> > - LoongArch specifc functionality for DxeLoad.
> >
> > -
> >
> > - Copyright (c) 2022, Loongson Technology Corporation Limited. All rights reserved.<BR>
> >
> > -
> >
> > - SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > -
> >
> > -**/
> >
> > -
> >
> > -#include "DxeIpl.h"
> >
> > -
> >
> > -/**
> >
> > - Transfers control to DxeCore.
> >
> > -
> >
> > - This function performs a CPU architecture specific operations to execute
> >
> > - the entry point of DxeCore with the parameters of HobList.
> >
> > - It also installs EFI_END_OF_PEI_PPI to signal the end of PEI phase.
> >
> > -
> >
> > - @param[in] DxeCoreEntryPoint The entry point of DxeCore.
> >
> > - @param[in] HobList The start of HobList passed to DxeCore.
> >
> > -
> >
> > -**/
> >
> > -VOID
> >
> > -HandOffToDxeCore (
> >
> > - IN EFI_PHYSICAL_ADDRESS DxeCoreEntryPoint,
> >
> > - IN EFI_PEI_HOB_POINTERS HobList
> >
> > - )
> >
> > -{
> >
> > - VOID *BaseOfStack;
> >
> > - VOID *TopOfStack;
> >
> > - EFI_STATUS Status;
> >
> > -
> >
> > - //
> >
> > - // Allocate 128KB for the Stack
> >
> > - //
> >
> > - BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
> >
> > - ASSERT (BaseOfStack != NULL);
> >
> > -
>
> I know this code path is critical to continue boot, but dereferencing a
> null pointer if asserts are not active is not a great alternative.
>
> Perhaps a status error code could be reported to allow platforms to
> potentially hook onto? Kind of like what is done if the DXE IPL PPI
> cannot be found
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#LL518-L522.
>
I understand your point, but you are responding to a patch that
removes these lines.
All architectures currently implement basically the same logic here,
with the exception of IA32, which does
Status = PeiServicesAllocatePages (EfiBootServicesData,
EFI_SIZE_TO_PAGES (STACK_SIZE), &BaseOfStack);
ASSERT_EFI_ERROR (Status);
and so the only difference is that it dereferences a bogus pointer
instead of a NULL pointer on failure. (RISC-V doesn't ASSERT() so it
will happily carry on even in DEBUG mode)
So I propose we log this as a todo item for after we've unified all
these implementations across architectures.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/7] MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
2023-06-06 22:17 ` Ard Biesheuvel
@ 2023-06-06 23:07 ` Michael Kubacki
0 siblings, 0 replies; 15+ messages in thread
From: Michael Kubacki @ 2023-06-06 23:07 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm
On 6/6/2023 6:17 PM, Ard Biesheuvel wrote:
> On Tue, 6 Jun 2023 at 20:48, Michael Kubacki
> <mikuback@linux.microsoft.com> wrote:
>>
>> On 6/2/2023 11:17 AM, Ard Biesheuvel wrote:
>>> The Risc-V and LoongArch specific versions of the DXE core handoff code
>>> in DxeIpl are essentially copies of the EBC version (modulo the
>>> copyright in the header and some debug prints in the code).
>>>
>>> In preparation for introducing a generic PPI based method to implement
>>> the non-executable stack, let's merge these versions, so we only need to
>>> add this logic once.
>>>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>> MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} | 2 +-
>>> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 10 +--
>>> MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c | 63 ----------------
>>> MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c | 75 --------------------
>>> 4 files changed, 3 insertions(+), 147 deletions(-)
>>>
> ...
>>> diff --git a/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
>>> deleted file mode 100644
>>> index 95d3af19ea4c9f00..0000000000000000
>>> --- a/MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
>>> +++ /dev/null
>>> @@ -1,63 +0,0 @@
>>> -/** @file
>>>
>>> - LoongArch specifc functionality for DxeLoad.
>>>
>>> -
>>>
>>> - Copyright (c) 2022, Loongson Technology Corporation Limited. All rights reserved.<BR>
>>>
>>> -
>>>
>>> - SPDX-License-Identifier: BSD-2-Clause-Patent
>>>
>>> -
>>>
>>> -**/
>>>
>>> -
>>>
>>> -#include "DxeIpl.h"
>>>
>>> -
>>>
>>> -/**
>>>
>>> - Transfers control to DxeCore.
>>>
>>> -
>>>
>>> - This function performs a CPU architecture specific operations to execute
>>>
>>> - the entry point of DxeCore with the parameters of HobList.
>>>
>>> - It also installs EFI_END_OF_PEI_PPI to signal the end of PEI phase.
>>>
>>> -
>>>
>>> - @param[in] DxeCoreEntryPoint The entry point of DxeCore.
>>>
>>> - @param[in] HobList The start of HobList passed to DxeCore.
>>>
>>> -
>>>
>>> -**/
>>>
>>> -VOID
>>>
>>> -HandOffToDxeCore (
>>>
>>> - IN EFI_PHYSICAL_ADDRESS DxeCoreEntryPoint,
>>>
>>> - IN EFI_PEI_HOB_POINTERS HobList
>>>
>>> - )
>>>
>>> -{
>>>
>>> - VOID *BaseOfStack;
>>>
>>> - VOID *TopOfStack;
>>>
>>> - EFI_STATUS Status;
>>>
>>> -
>>>
>>> - //
>>>
>>> - // Allocate 128KB for the Stack
>>>
>>> - //
>>>
>>> - BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
>>>
>>> - ASSERT (BaseOfStack != NULL);
>>>
>>> -
>>
>> I know this code path is critical to continue boot, but dereferencing a
>> null pointer if asserts are not active is not a great alternative.
>>
>> Perhaps a status error code could be reported to allow platforms to
>> potentially hook onto? Kind of like what is done if the DXE IPL PPI
>> cannot be found
>> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#LL518-L522.
>>
>
> I understand your point, but you are responding to a patch that
> removes these lines.
>
> All architectures currently implement basically the same logic here,
> with the exception of IA32, which does
>
> Status = PeiServicesAllocatePages (EfiBootServicesData,
> EFI_SIZE_TO_PAGES (STACK_SIZE), &BaseOfStack);
> ASSERT_EFI_ERROR (Status);
>
> and so the only difference is that it dereferences a bogus pointer
> instead of a NULL pointer on failure. (RISC-V doesn't ASSERT() so it
> will happily carry on even in DEBUG mode)
>
> So I propose we log this as a todo item for after we've unified all
> these implementations across architectures.
Sorry, I got this mixed up with another patch I was looking at on
GitHub. That sounds good.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
` (7 preceding siblings ...)
2023-06-06 16:42 ` [edk2-devel] [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Oliver Smith-Denny
@ 2023-06-06 23:12 ` Michael Kubacki
2023-06-25 2:58 ` 回复: [edk2-devel] " gaoliming
8 siblings, 1 reply; 15+ messages in thread
From: Michael Kubacki @ 2023-06-06 23:12 UTC (permalink / raw)
To: Ard Biesheuvel, devel
Cc: Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Liming Gao,
Kinney, Michael D, Leif Lindholm
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
On 6/2/2023 11:17 AM, Ard Biesheuvel wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4468
>
>
>
> This is a followup to the RFC that I sent to the edk2-devel list on the
>
> 25th of May.
>
>
>
> In an attempt to make some incremental progress, this v2 only covers the
>
> NX remapping of the DXE stack in DxeIpl, using the newly introduced
>
> memory attributes PPI.
>
>
>
> Other use cases are deferred until we can converge on an approach that
>
> works across architectures and platforms. In particular, this means the
>
> following use cases:
>
> - mapping the DXE core code and data regions RO and XP, respectively;
>
> - mapping shadowed PEIMs read-only (including the PEI core itself);
>
> - managing memory permissions after temporary RAM migration;
>
> - reorganizing the X64 PEI flow with respect to page table allocation;
>
> - managing the dispatch order of the PEIM producing the PPI in relation
>
> to its consumers.
>
>
>
> The current series specifies the PPI in patch #1, and wires it up into
>
> DxeIpl to remap the DXE stack non-executable in a generic manner
>
> (patches #2 and #3)
>
>
>
> Patches #4 and #5 implement the PPI for ARM and AArch64.
>
>
>
> Patch #6 switches ARM and AArch64 over to the generic DxeIpl.
>
>
>
> Patch #7 cleans up the ARM implementation of the UEFI memory attributes
>
> protocol, based on the improvements made in patch #4.
>
>
>
> Changes since RFC (in addition to the above):
>
> - update PPI protype to use attributes+mask instead of setmask+clearmask
>
> - drop OVMF patch for RISC-V that has been applied in the meantime
>
>
>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
>
> Cc: Taylor Beebe <t@taylorbeebe.com>
>
> Cc: Oliver Smith-Denny <osd@smith-denny.com>
>
> Cc: Dandan Bi <dandan.bi@intel.com>
>
> Cc: Dun Tan <dun.tan@intel.com>
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>
> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>
>
>
> Ard Biesheuvel (7):
>
> MdeModulePkg: Define memory attribute PPI
>
> MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
>
> MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack NX
>
> ArmPkg/ArmMmuLib: Extend API to manage memory permissions better
>
> ArmPkg/CpuPei: Implement the memory attributes PPI
>
> MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code
>
> ArmPkg/CpuDxe: Simplify memory attributes protocol implementation
>
>
>
> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
>
> ArmPkg/Drivers/CpuDxe/MemoryAttribute.c | 50 +----------
>
> ArmPkg/Drivers/CpuPei/CpuPei.c | 76 +++++++++++++++++
>
> ArmPkg/Drivers/CpuPei/CpuPei.inf | 4 +
>
> ArmPkg/Include/Library/ArmMmuLib.h | 36 +++++++-
>
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 52 +++++++++++-
>
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 88 +++++++++++++++++---
>
> ArmPkg/Library/OpteeLib/Optee.c | 2 +-
>
> MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 71 ----------------
>
> MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} | 31 ++++++-
>
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 24 ++----
>
> MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c | 63 --------------
>
> MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c | 75 -----------------
>
> MdeModulePkg/Include/Ppi/MemoryAttribute.h | 83 ++++++++++++++++++
>
> MdeModulePkg/MdeModulePkg.dec | 3 +
>
> 15 files changed, 366 insertions(+), 294 deletions(-)
>
> delete mode 100644 MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
>
> rename MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c} (62%)
>
> delete mode 100644 MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
>
> delete mode 100644 MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
>
> create mode 100644 MdeModulePkg/Include/Ppi/MemoryAttribute.h
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* 回复: [edk2-devel] [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes
2023-06-06 23:12 ` Michael Kubacki
@ 2023-06-25 2:58 ` gaoliming
2023-06-26 9:16 ` Ard Biesheuvel
0 siblings, 1 reply; 15+ messages in thread
From: gaoliming @ 2023-06-25 2:58 UTC (permalink / raw)
To: devel, mikuback, 'Ard Biesheuvel'
Cc: 'Ray Ni', 'Jiewen Yao', 'Gerd Hoffmann',
'Taylor Beebe', 'Oliver Smith-Denny',
'Dandan Bi', 'Dun Tan',
'Kinney, Michael D', 'Leif Lindholm'
For the changes in MdeModulePkg, Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael
> Kubacki
> 发送时间: 2023年6月7日 7:13
> 收件人: Ard Biesheuvel <ardb@kernel.org>; devel@edk2.groups.io
> 抄送: Ray Ni <ray.ni@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Taylor Beebe <t@taylorbeebe.com>; Oliver
> Smith-Denny <osd@smith-denny.com>; Dandan Bi <dandan.bi@intel.com>;
> Dun Tan <dun.tan@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Leif Lindholm
> <quic_llindhol@quicinc.com>
> 主题: Re: [edk2-devel] [PATCH v2 0/7] Add PPI to manage PEI phase memory
> attributes
>
> Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
>
> On 6/2/2023 11:17 AM, Ard Biesheuvel wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4468
> >
> >
> >
> > This is a followup to the RFC that I sent to the edk2-devel list on the
> >
> > 25th of May.
> >
> >
> >
> > In an attempt to make some incremental progress, this v2 only covers the
> >
> > NX remapping of the DXE stack in DxeIpl, using the newly introduced
> >
> > memory attributes PPI.
> >
> >
> >
> > Other use cases are deferred until we can converge on an approach that
> >
> > works across architectures and platforms. In particular, this means the
> >
> > following use cases:
> >
> > - mapping the DXE core code and data regions RO and XP, respectively;
> >
> > - mapping shadowed PEIMs read-only (including the PEI core itself);
> >
> > - managing memory permissions after temporary RAM migration;
> >
> > - reorganizing the X64 PEI flow with respect to page table allocation;
> >
> > - managing the dispatch order of the PEIM producing the PPI in relation
> >
> > to its consumers.
> >
> >
> >
> > The current series specifies the PPI in patch #1, and wires it up into
> >
> > DxeIpl to remap the DXE stack non-executable in a generic manner
> >
> > (patches #2 and #3)
> >
> >
> >
> > Patches #4 and #5 implement the PPI for ARM and AArch64.
> >
> >
> >
> > Patch #6 switches ARM and AArch64 over to the generic DxeIpl.
> >
> >
> >
> > Patch #7 cleans up the ARM implementation of the UEFI memory attributes
> >
> > protocol, based on the improvements made in patch #4.
> >
> >
> >
> > Changes since RFC (in addition to the above):
> >
> > - update PPI protype to use attributes+mask instead of setmask+clearmask
> >
> > - drop OVMF patch for RISC-V that has been applied in the meantime
> >
> >
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> >
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Cc: Taylor Beebe <t@taylorbeebe.com>
> >
> > Cc: Oliver Smith-Denny <osd@smith-denny.com>
> >
> > Cc: Dandan Bi <dandan.bi@intel.com>
> >
> > Cc: Dun Tan <dun.tan@intel.com>
> >
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
> >
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> >
> > Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> >
> >
> >
> > Ard Biesheuvel (7):
> >
> > MdeModulePkg: Define memory attribute PPI
> >
> > MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
> >
> > MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack
> NX
> >
> > ArmPkg/ArmMmuLib: Extend API to manage memory permissions
> better
> >
> > ArmPkg/CpuPei: Implement the memory attributes PPI
> >
> > MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code
> >
> > ArmPkg/CpuDxe: Simplify memory attributes protocol implementation
> >
> >
> >
> > ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> | 2 +-
> >
> > ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
> | 50 +----------
> >
> > ArmPkg/Drivers/CpuPei/CpuPei.c
> | 76 +++++++++++++++++
> >
> > ArmPkg/Drivers/CpuPei/CpuPei.inf
> | 4 +
> >
> > ArmPkg/Include/Library/ArmMmuLib.h
> | 36 +++++++-
> >
> > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> | 52 +++++++++++-
> >
> > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> | 88 +++++++++++++++++---
> >
> > ArmPkg/Library/OpteeLib/Optee.c
> | 2 +-
> >
> > MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
> | 71 ----------------
> >
> > MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c}
> | 31 ++++++-
> >
> > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> | 24 ++----
> >
> > MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
> | 63 --------------
> >
> > MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> | 75 -----------------
> >
> > MdeModulePkg/Include/Ppi/MemoryAttribute.h
> | 83 ++++++++++++++++++
> >
> > MdeModulePkg/MdeModulePkg.dec
> | 3 +
> >
> > 15 files changed, 366 insertions(+), 294 deletions(-)
> >
> > delete mode 100644
> MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
> >
> > rename MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c =>
> DxeHandoff.c} (62%)
> >
> > delete mode 100644
> MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
> >
> > delete mode 100644
> MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> >
> > create mode 100644 MdeModulePkg/Include/Ppi/MemoryAttribute.h
> >
> >
> >
>
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes
2023-06-25 2:58 ` 回复: [edk2-devel] " gaoliming
@ 2023-06-26 9:16 ` Ard Biesheuvel
0 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-06-26 9:16 UTC (permalink / raw)
To: devel, gaoliming
Cc: mikuback, Ray Ni, Jiewen Yao, Gerd Hoffmann, Taylor Beebe,
Oliver Smith-Denny, Dandan Bi, Dun Tan, Kinney, Michael D,
Leif Lindholm
On Sun, 25 Jun 2023 at 04:59, gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> For the changes in MdeModulePkg, Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>
Merged as #4573
Thanks all.
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael
> > Kubacki
> > 发送时间: 2023年6月7日 7:13
> > 收件人: Ard Biesheuvel <ardb@kernel.org>; devel@edk2.groups.io
> > 抄送: Ray Ni <ray.ni@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Gerd
> > Hoffmann <kraxel@redhat.com>; Taylor Beebe <t@taylorbeebe.com>; Oliver
> > Smith-Denny <osd@smith-denny.com>; Dandan Bi <dandan.bi@intel.com>;
> > Dun Tan <dun.tan@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> > Kinney, Michael D <michael.d.kinney@intel.com>; Leif Lindholm
> > <quic_llindhol@quicinc.com>
> > 主题: Re: [edk2-devel] [PATCH v2 0/7] Add PPI to manage PEI phase memory
> > attributes
> >
> > Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > On 6/2/2023 11:17 AM, Ard Biesheuvel wrote:
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4468
> > >
> > >
> > >
> > > This is a followup to the RFC that I sent to the edk2-devel list on the
> > >
> > > 25th of May.
> > >
> > >
> > >
> > > In an attempt to make some incremental progress, this v2 only covers the
> > >
> > > NX remapping of the DXE stack in DxeIpl, using the newly introduced
> > >
> > > memory attributes PPI.
> > >
> > >
> > >
> > > Other use cases are deferred until we can converge on an approach that
> > >
> > > works across architectures and platforms. In particular, this means the
> > >
> > > following use cases:
> > >
> > > - mapping the DXE core code and data regions RO and XP, respectively;
> > >
> > > - mapping shadowed PEIMs read-only (including the PEI core itself);
> > >
> > > - managing memory permissions after temporary RAM migration;
> > >
> > > - reorganizing the X64 PEI flow with respect to page table allocation;
> > >
> > > - managing the dispatch order of the PEIM producing the PPI in relation
> > >
> > > to its consumers.
> > >
> > >
> > >
> > > The current series specifies the PPI in patch #1, and wires it up into
> > >
> > > DxeIpl to remap the DXE stack non-executable in a generic manner
> > >
> > > (patches #2 and #3)
> > >
> > >
> > >
> > > Patches #4 and #5 implement the PPI for ARM and AArch64.
> > >
> > >
> > >
> > > Patch #6 switches ARM and AArch64 over to the generic DxeIpl.
> > >
> > >
> > >
> > > Patch #7 cleans up the ARM implementation of the UEFI memory attributes
> > >
> > > protocol, based on the improvements made in patch #4.
> > >
> > >
> > >
> > > Changes since RFC (in addition to the above):
> > >
> > > - update PPI protype to use attributes+mask instead of setmask+clearmask
> > >
> > > - drop OVMF patch for RISC-V that has been applied in the meantime
> > >
> > >
> > >
> > > Cc: Ray Ni <ray.ni@intel.com>
> > >
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > >
> > > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > >
> > > Cc: Taylor Beebe <t@taylorbeebe.com>
> > >
> > > Cc: Oliver Smith-Denny <osd@smith-denny.com>
> > >
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > >
> > > Cc: Dun Tan <dun.tan@intel.com>
> > >
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > >
> > > Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
> > >
> > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > >
> > > Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> > >
> > >
> > >
> > > Ard Biesheuvel (7):
> > >
> > > MdeModulePkg: Define memory attribute PPI
> > >
> > > MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code
> > >
> > > MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack
> > NX
> > >
> > > ArmPkg/ArmMmuLib: Extend API to manage memory permissions
> > better
> > >
> > > ArmPkg/CpuPei: Implement the memory attributes PPI
> > >
> > > MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code
> > >
> > > ArmPkg/CpuDxe: Simplify memory attributes protocol implementation
> > >
> > >
> > >
> > > ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> > | 2 +-
> > >
> > > ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
> > | 50 +----------
> > >
> > > ArmPkg/Drivers/CpuPei/CpuPei.c
> > | 76 +++++++++++++++++
> > >
> > > ArmPkg/Drivers/CpuPei/CpuPei.inf
> > | 4 +
> > >
> > > ArmPkg/Include/Library/ArmMmuLib.h
> > | 36 +++++++-
> > >
> > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > | 52 +++++++++++-
> > >
> > > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> > | 88 +++++++++++++++++---
> > >
> > > ArmPkg/Library/OpteeLib/Optee.c
> > | 2 +-
> > >
> > > MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
> > | 71 ----------------
> > >
> > > MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c => DxeHandoff.c}
> > | 31 ++++++-
> > >
> > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> > | 24 ++----
> > >
> > > MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
> > | 63 --------------
> > >
> > > MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> > | 75 -----------------
> > >
> > > MdeModulePkg/Include/Ppi/MemoryAttribute.h
> > | 83 ++++++++++++++++++
> > >
> > > MdeModulePkg/MdeModulePkg.dec
> > | 3 +
> > >
> > > 15 files changed, 366 insertions(+), 294 deletions(-)
> > >
> > > delete mode 100644
> > MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
> > >
> > > rename MdeModulePkg/Core/DxeIplPeim/{Ebc/DxeLoadFunc.c =>
> > DxeHandoff.c} (62%)
> > >
> > > delete mode 100644
> > MdeModulePkg/Core/DxeIplPeim/LoongArch64/DxeLoadFunc.c
> > >
> > > delete mode 100644
> > MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> > >
> > > create mode 100644 MdeModulePkg/Include/Ppi/MemoryAttribute.h
> > >
> > >
> > >
> >
> >
> >
> >
>
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-06-26 9:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-02 15:17 [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 1/7] MdeModulePkg: Define memory attribute PPI Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 2/7] MdeModulePkg/DxeIpl: Merge EBC, RISCV64 and LOONGARCH code Ard Biesheuvel
2023-06-06 18:48 ` [edk2-devel] " Michael Kubacki
2023-06-06 22:17 ` Ard Biesheuvel
2023-06-06 23:07 ` Michael Kubacki
2023-06-02 15:17 ` [PATCH v2 3/7] MdeModulePkg/DxeIpl: Use memory attribute PPI to remap the stack NX Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 4/7] ArmPkg/ArmMmuLib: Extend API to manage memory permissions better Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 5/7] ArmPkg/CpuPei: Implement the memory attributes PPI Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 6/7] MdeModulePkg/DxeIpl ARM AARCH64: Switch to generic handoff code Ard Biesheuvel
2023-06-02 15:17 ` [PATCH v2 7/7] ArmPkg/CpuDxe: Simplify memory attributes protocol implementation Ard Biesheuvel
2023-06-06 16:42 ` [edk2-devel] [PATCH v2 0/7] Add PPI to manage PEI phase memory attributes Oliver Smith-Denny
2023-06-06 23:12 ` Michael Kubacki
2023-06-25 2:58 ` 回复: [edk2-devel] " gaoliming
2023-06-26 9:16 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox