* [PATCH v3 1/5] MdePkg: Add Memory Attribute Protocol definition
2023-02-06 22:20 [PATCH v3 0/5] ArmPkg: implement EFI memory attributes protocol Ard Biesheuvel
@ 2023-02-06 22:20 ` Ard Biesheuvel
2023-02-06 22:20 ` [PATCH v3 2/5] ArmPkg/CpuDxe ARM: Fix page-to-section attribute conversion Ard Biesheuvel
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2023-02-06 22:20 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
Sami Mujawar, Taylor Beebe
Add the Memory Attribute Protocol definition, which was adopted and
included in version 2.10 of the UEFI specification.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=3519
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
MdePkg/Include/Protocol/MemoryAttribute.h | 142 ++++++++++++++++++++
MdePkg/MdePkg.dec | 3 +
2 files changed, 145 insertions(+)
diff --git a/MdePkg/Include/Protocol/MemoryAttribute.h b/MdePkg/Include/Protocol/MemoryAttribute.h
new file mode 100644
index 000000000000..ff930fb21aa6
--- /dev/null
+++ b/MdePkg/Include/Protocol/MemoryAttribute.h
@@ -0,0 +1,142 @@
+/** @file
+
+ EFI Memory Attribute Protocol provides retrieval and update service
+ for memory attributes in EFI environment.
+
+ Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2023, Google LLC. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __EFI_MEMORY_ATTRIBUTE_H__
+#define __EFI_MEMORY_ATTRIBUTE_H__
+
+#define EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID \
+ { \
+ 0xf4560cf6, 0x40ec, 0x4b4a, { 0xa1, 0x92, 0xbf, 0x1d, 0x57, 0xd0, 0xb1, 0x89 } \
+ }
+
+typedef struct _EFI_MEMORY_ATTRIBUTE_PROTOCOL EFI_MEMORY_ATTRIBUTE_PROTOCOL;
+
+/**
+ This function set given attributes of the memory region specified by
+ BaseAddress and Length.
+
+ The valid Attributes is EFI_MEMORY_RP, EFI_MEMORY_XP, and EFI_MEMORY_RO.
+
+ @param This The EFI_MEMORY_ATTRIBUTE_PROTOCOL instance.
+ @param BaseAddress The physical address that is the start address of
+ a memory region.
+ @param Length The size in bytes of the memory region.
+ @param Attributes The bit mask of attributes to set for the memory
+ region.
+
+ @retval EFI_SUCCESS The attributes were set for the memory region.
+ @retval EFI_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of
+ attributes that cannot be set together.
+ @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.
+ @retval EFI_ACCESS_DENIED Attributes for the requested memory region are
+ controlled by system firmware and cannot be
+ updated via the protocol.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_SET_MEMORY_ATTRIBUTES)(
+ IN EFI_MEMORY_ATTRIBUTE_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes
+ );
+
+/**
+ This function clears given attributes of the memory region specified by
+ BaseAddress and Length.
+
+ The valid Attributes is EFI_MEMORY_RP, EFI_MEMORY_XP, and EFI_MEMORY_RO.
+
+ @param This The EFI_MEMORY_ATTRIBUTE_PROTOCOL instance.
+ @param BaseAddress The physical address that is the start address of
+ a memory region.
+ @param Length The size in bytes of the memory region.
+ @param Attributes The bit mask of attributes to clear for the memory
+ region.
+
+ @retval EFI_SUCCESS The attributes were cleared for the memory region.
+ @retval EFI_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of
+ attributes that cannot be cleared together.
+ @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.
+ @retval EFI_ACCESS_DENIED Attributes for the requested memory region are
+ controlled by system firmware and cannot be
+ updated via the protocol.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_CLEAR_MEMORY_ATTRIBUTES)(
+ IN EFI_MEMORY_ATTRIBUTE_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes
+ );
+
+/**
+ This function retrieves the attributes of the memory region specified by
+ BaseAddress and Length. If different attributes are got from different part
+ of the memory region, EFI_NO_MAPPING will be returned.
+
+ @param This The EFI_MEMORY_ATTRIBUTE_PROTOCOL instance.
+ @param BaseAddress The physical address that is the start address of
+ a memory region.
+ @param Length The size in bytes of the memory region.
+ @param Attributes Pointer to attributes returned.
+
+ @retval EFI_SUCCESS The attributes got for the memory region.
+ @retval EFI_INVALID_PARAMETER Length is zero.
+ Attributes is NULL.
+ @retval EFI_NO_MAPPING Attributes are not consistent cross the memory
+ region.
+ @retval EFI_UNSUPPORTED The processor does not support one or more
+ bytes of the memory resource range specified
+ by BaseAddress and Length.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_GET_MEMORY_ATTRIBUTES)(
+ IN EFI_MEMORY_ATTRIBUTE_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ OUT UINT64 *Attributes
+ );
+
+///
+/// EFI Memory Attribute Protocol provides services to retrieve or update
+/// attribute of memory in the EFI environment.
+///
+struct _EFI_MEMORY_ATTRIBUTE_PROTOCOL {
+ EFI_GET_MEMORY_ATTRIBUTES GetMemoryAttributes;
+ EFI_SET_MEMORY_ATTRIBUTES SetMemoryAttributes;
+ EFI_CLEAR_MEMORY_ATTRIBUTES ClearMemoryAttributes;
+};
+
+extern EFI_GUID gEfiMemoryAttributeProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 3d08f20d15b0..a8658403c8fd 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1915,6 +1915,9 @@ [Protocols]
## Include/Protocol/RedfishDiscover.h
gEfiRedfishDiscoverProtocolGuid = { 0x5db12509, 0x4550, 0x4347, { 0x96, 0xb3, 0x73, 0xc0, 0xff, 0x6e, 0x86, 0x9f }}
+ ## Include/Protocol/MemoryAttribute.h
+ gEfiMemoryAttributeProtocolGuid = { 0xf4560cf6, 0x40ec, 0x4b4a, { 0xa1, 0x92, 0xbf, 0x1d, 0x57, 0xd0, 0xb1, 0x89 }}
+
#
# Protocols defined in Shell2.0
#
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/5] ArmPkg/CpuDxe ARM: Fix page-to-section attribute conversion
2023-02-06 22:20 [PATCH v3 0/5] ArmPkg: implement EFI memory attributes protocol Ard Biesheuvel
2023-02-06 22:20 ` [PATCH v3 1/5] MdePkg: Add Memory Attribute Protocol definition Ard Biesheuvel
@ 2023-02-06 22:20 ` Ard Biesheuvel
2023-02-06 22:20 ` [PATCH v3 3/5] ArmPkg/CpuDxe: Expose unified region-to-EFI " Ard Biesheuvel
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2023-02-06 22:20 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
Sami Mujawar, Taylor Beebe
The section-to-page attribute conversion takes the shareability and
execute-never attributes into account, whereas the page-to-section
counterpart does not. The result is that GetMemoryRegionPage () -which
takes a section attribute argument (via *RegionAttributes) that is
ostensibly based on the first page in the range, but differs from the
actual page attributes when converted back- may return with a
RegionLength of zero. This is incorrect, and confuses code that scans a
region by calling GetMemoryRegion () in sequence.
So fix the conversion, and ASSERT () on a non-zero region length.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 3 +++
ArmPkg/Include/Chipset/ArmV7Mmu.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index 2daf47ba6fe5..e7acd84b8af9 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -480,6 +480,8 @@ GetMemoryRegion (
PageAttributes = PageTable[PageTableIndex] & TT_DESCRIPTOR_PAGE_ATTRIBUTE_MASK;
*RegionAttributes = TT_DESCRIPTOR_CONVERT_TO_SECTION_CACHE_POLICY (PageAttributes, 0) |
+ TT_DESCRIPTOR_CONVERT_TO_SECTION_S (PageAttributes) |
+ TT_DESCRIPTOR_CONVERT_TO_SECTION_XN (PageAttributes) |
TT_DESCRIPTOR_CONVERT_TO_SECTION_AP (PageAttributes);
}
@@ -494,6 +496,7 @@ GetMemoryRegion (
// Scan the page table to find the end of the region.
Status = GetMemoryRegionPage (PageTable, BaseAddress, RegionLength, RegionAttributes);
+ ASSERT (*RegionLength > 0);
// If we have found the end of the region (Status == EFI_SUCCESS) then we exit the for-loop
if (Status == EFI_SUCCESS) {
diff --git a/ArmPkg/Include/Chipset/ArmV7Mmu.h b/ArmPkg/Include/Chipset/ArmV7Mmu.h
index db99527d6efa..4f51041e29ed 100644
--- a/ArmPkg/Include/Chipset/ArmV7Mmu.h
+++ b/ArmPkg/Include/Chipset/ArmV7Mmu.h
@@ -144,6 +144,8 @@
(((((Desc) & (0x3 << 12)) >> 6) | (Desc & (0x3 << 2)))))
#define TT_DESCRIPTOR_CONVERT_TO_SECTION_AP(Desc) ((((Desc) & TT_DESCRIPTOR_PAGE_AP_MASK) << 6) & TT_DESCRIPTOR_SECTION_AP_MASK)
+#define TT_DESCRIPTOR_CONVERT_TO_SECTION_S(Desc) ((((Desc) & TT_DESCRIPTOR_PAGE_S_MASK) << 6) & TT_DESCRIPTOR_SECTION_S_MASK)
+#define TT_DESCRIPTOR_CONVERT_TO_SECTION_XN(Desc) ((((Desc) & TT_DESCRIPTOR_PAGE_XN_MASK) << 4) & TT_DESCRIPTOR_SECTION_XN_MASK)
#define TT_DESCRIPTOR_CONVERT_TO_SECTION_CACHE_POLICY(Desc, IsLargePage) (IsLargePage? \
(((Desc) & TT_DESCRIPTOR_LARGEPAGE_CACHE_POLICY_MASK) & TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK): \
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 3/5] ArmPkg/CpuDxe: Expose unified region-to-EFI attribute conversion
2023-02-06 22:20 [PATCH v3 0/5] ArmPkg: implement EFI memory attributes protocol Ard Biesheuvel
2023-02-06 22:20 ` [PATCH v3 1/5] MdePkg: Add Memory Attribute Protocol definition Ard Biesheuvel
2023-02-06 22:20 ` [PATCH v3 2/5] ArmPkg/CpuDxe ARM: Fix page-to-section attribute conversion Ard Biesheuvel
@ 2023-02-06 22:20 ` Ard Biesheuvel
2023-02-06 22:20 ` [PATCH v3 4/5] ArmPkg/CpuDxe: Implement EFI memory attributes protocol Ard Biesheuvel
2023-02-06 22:21 ` [NOT FOR MERGE v3 5/5] MdeModulePkg/DxeCore: add DEBUG code for memory attribute handling Ard Biesheuvel
4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2023-02-06 22:20 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
Sami Mujawar, Taylor Beebe
In preparation for introducing an implementation of the EFI memory
attributes protocol that is shared between ARM and AArch64, unify the
existing code that converts a page table descriptor into a
EFI_MEMORY_xxx bitfield, so it can be called from the generic code.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 8 ++++++++
ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 13 +++++++++++++
ArmPkg/Drivers/CpuDxe/CpuDxe.h | 5 +++++
3 files changed, 26 insertions(+)
diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
index 8bb33046e707..a4bde2034fa0 100644
--- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
@@ -79,6 +79,14 @@ PageAttributeToGcdAttribute (
return GcdAttributes;
}
+UINT64
+RegionAttributeToGcdAttribute (
+ IN UINTN PageAttributes
+ )
+{
+ return PageAttributeToGcdAttribute (PageAttributes);
+}
+
STATIC
UINT64
GetFirstPageAttribute (
diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index e7acd84b8af9..a3a7fde9a872 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MemoryAllocationLib.h>
#include "CpuDxe.h"
+STATIC
EFI_STATUS
SectionToGcdAttributes (
IN UINT32 SectionAttributes,
@@ -77,6 +78,18 @@ SectionToGcdAttributes (
return EFI_SUCCESS;
}
+UINT64
+RegionAttributeToGcdAttribute (
+ IN UINTN PageAttributes
+ )
+{
+ UINT64 Result;
+
+ SectionToGcdAttributes (PageAttributes, &Result);
+ return Result;
+}
+
+STATIC
EFI_STATUS
PageToGcdAttributes (
IN UINT32 PageAttributes,
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
index ff672390ce51..5a9f1ef1f969 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
@@ -126,4 +126,9 @@ SetGcdMemorySpaceAttributes (
IN UINT64 Attributes
);
+UINT64
+RegionAttributeToGcdAttribute (
+ IN UINTN PageAttributes
+ );
+
#endif // CPU_DXE_H_
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 4/5] ArmPkg/CpuDxe: Implement EFI memory attributes protocol
2023-02-06 22:20 [PATCH v3 0/5] ArmPkg: implement EFI memory attributes protocol Ard Biesheuvel
` (2 preceding siblings ...)
2023-02-06 22:20 ` [PATCH v3 3/5] ArmPkg/CpuDxe: Expose unified region-to-EFI " Ard Biesheuvel
@ 2023-02-06 22:20 ` Ard Biesheuvel
2023-02-06 22:21 ` [NOT FOR MERGE v3 5/5] MdeModulePkg/DxeCore: add DEBUG code for memory attribute handling Ard Biesheuvel
4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2023-02-06 22:20 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
Sami Mujawar, Taylor Beebe
Expose the protocol introduced in v2.10 that permits the caller to
manage mapping permissions in the page tables.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
ArmPkg/Drivers/CpuDxe/CpuDxe.c | 2 +
ArmPkg/Drivers/CpuDxe/CpuDxe.h | 3 +
ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +
ArmPkg/Drivers/CpuDxe/MemoryAttribute.c | 252 ++++++++++++++++++++
4 files changed, 259 insertions(+)
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
index e6742f0a25fc..d04958e79e52 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
@@ -244,6 +244,8 @@ CpuDxeInitialize (
&mCpuHandle,
&gEfiCpuArchProtocolGuid,
&mCpu,
+ &gEfiMemoryAttributeProtocolGuid,
+ &mMemoryAttribute,
NULL
);
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
index 5a9f1ef1f969..6cb5cd3d2e70 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
@@ -30,9 +30,12 @@
#include <Protocol/Cpu.h>
#include <Protocol/DebugSupport.h>
#include <Protocol/LoadedImage.h>
+#include <Protocol/MemoryAttribute.h>
extern BOOLEAN mIsFlushingGCD;
+extern EFI_MEMORY_ATTRIBUTE_PROTOCOL mMemoryAttribute;
+
/**
This function registers and enables the handler specified by InterruptHandler for a processor
interrupt or exception type specified by InterruptType. If InterruptHandler is NULL, then the
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index 10792b393fc8..e732e21cb94a 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -23,6 +23,7 @@ [Sources.Common]
CpuDxe.h
CpuMmuCommon.c
Exception.c
+ MemoryAttribute.c
[Sources.ARM]
Arm/Mmu.c
@@ -53,6 +54,7 @@ [LibraryClasses]
[Protocols]
gEfiCpuArchProtocolGuid
+ gEfiMemoryAttributeProtocolGuid
[Guids]
gEfiDebugImageInfoTableGuid
diff --git a/ArmPkg/Drivers/CpuDxe/MemoryAttribute.c b/ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
new file mode 100644
index 000000000000..379e90bd9d1d
--- /dev/null
+++ b/ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
@@ -0,0 +1,252 @@
+/** @file
+
+ Copyright (c) 2023, Google LLC. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "CpuDxe.h"
+
+/**
+ This function retrieves the attributes of the memory region specified by
+ BaseAddress and Length. If different attributes are got from different part
+ of the memory region, EFI_NO_MAPPING will be returned.
+
+ @param This The EFI_MEMORY_ATTRIBUTE_PROTOCOL instance.
+ @param BaseAddress The physical address that is the start address of
+ a memory region.
+ @param Length The size in bytes of the memory region.
+ @param Attributes Pointer to attributes returned.
+
+ @retval EFI_SUCCESS The attributes got for the memory region.
+ @retval EFI_INVALID_PARAMETER Length is zero.
+ Attributes is NULL.
+ @retval EFI_NO_MAPPING Attributes are not consistent cross the memory
+ region.
+ @retval EFI_UNSUPPORTED The processor does not support one or more
+ bytes of the memory resource range specified
+ by BaseAddress and Length.
+
+**/
+STATIC
+EFI_STATUS
+GetMemoryAttributes (
+ IN EFI_MEMORY_ATTRIBUTE_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ OUT UINT64 *Attributes
+ )
+{
+ UINTN RegionAddress;
+ UINTN RegionLength;
+ UINTN RegionAttributes;
+ UINTN Union;
+ UINTN Intersection;
+ EFI_STATUS Status;
+
+ if ((Length == 0) || (Attributes == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ DEBUG ((DEBUG_VERBOSE,
+ "%a: BaseAddress == 0x%lx, Length == 0x%lx\n",
+ __FUNCTION__,
+ BaseAddress,
+ Length
+ ));
+
+ Union = 0;
+ Intersection = MAX_UINTN;
+
+ for (RegionAddress = (UINTN)BaseAddress;
+ RegionAddress < (UINTN)(BaseAddress + Length);
+ RegionAddress += RegionLength) {
+
+ Status = GetMemoryRegion (&RegionAddress,
+ &RegionLength,
+ &RegionAttributes
+ );
+
+ DEBUG ((DEBUG_VERBOSE,
+ "%a: RegionAddress == 0x%lx, RegionLength == 0x%lx, RegionAttributes == 0x%lx\n",
+ __FUNCTION__,
+ (UINT64)RegionAddress,
+ (UINT64)RegionLength,
+ (UINT64)RegionAttributes
+ ));
+
+ if (EFI_ERROR (Status)) {
+ return EFI_NO_MAPPING;
+ }
+
+ Union |= RegionAttributes;
+ Intersection &= RegionAttributes;
+ }
+
+ DEBUG ((DEBUG_VERBOSE,
+ "%a: Union == %lx, Intersection == %lx\n",
+ __FUNCTION__,
+ (UINT64)Union,
+ (UINT64)Intersection
+ ));
+
+ if (Union != Intersection) {
+ return EFI_NO_MAPPING;
+ }
+
+ *Attributes = RegionAttributeToGcdAttribute (Union) & (EFI_MEMORY_RO | EFI_MEMORY_XP);
+ return EFI_SUCCESS;
+}
+
+/**
+ This function set given attributes of the memory region specified by
+ BaseAddress and Length.
+
+ The valid Attributes is EFI_MEMORY_RP, EFI_MEMORY_XP, and EFI_MEMORY_RO.
+
+ @param This The EFI_MEMORY_ATTRIBUTE_PROTOCOL instance.
+ @param BaseAddress The physical address that is the start address of
+ a memory region.
+ @param Length The size in bytes of the memory region.
+ @param Attributes The bit mask of attributes to set for the memory
+ region.
+
+ @retval EFI_SUCCESS The attributes were set for the memory region.
+ @retval EFI_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of
+ attributes that cannot be set together.
+ @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.
+ @retval EFI_ACCESS_DENIED Attributes for the requested memory region are
+ controlled by system firmware and cannot be
+ updated via the protocol.
+
+**/
+STATIC
+EFI_STATUS
+SetMemoryAttributes (
+ IN EFI_MEMORY_ATTRIBUTE_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes
+ )
+{
+ EFI_STATUS Status;
+
+ DEBUG ((DEBUG_INFO,
+ "%a: BaseAddress == 0x%lx, Length == 0x%lx, Attributes == 0x%lx\n",
+ __FUNCTION__,
+ (UINTN)BaseAddress,
+ (UINTN)Length,
+ (UINTN)Attributes
+ ));
+
+ if ((Length == 0) ||
+ ((Attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_RP | EFI_MEMORY_XP)) != 0)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if ((Attributes & EFI_MEMORY_RP) != 0) {
+ 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;
+}
+
+/**
+ This function clears given attributes of the memory region specified by
+ BaseAddress and Length.
+
+ The valid Attributes is EFI_MEMORY_RP, EFI_MEMORY_XP, and EFI_MEMORY_RO.
+
+ @param This The EFI_MEMORY_ATTRIBUTE_PROTOCOL instance.
+ @param BaseAddress The physical address that is the start address of
+ a memory region.
+ @param Length The size in bytes of the memory region.
+ @param Attributes The bit mask of attributes to clear for the memory
+ region.
+
+ @retval EFI_SUCCESS The attributes were cleared for the memory region.
+ @retval EFI_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of
+ attributes that cannot be cleared together.
+ @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.
+ @retval EFI_ACCESS_DENIED Attributes for the requested memory region are
+ controlled by system firmware and cannot be
+ updated via the protocol.
+
+**/
+STATIC
+EFI_STATUS
+ClearMemoryAttributes (
+ IN EFI_MEMORY_ATTRIBUTE_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes
+ )
+{
+ EFI_STATUS Status;
+
+ DEBUG ((DEBUG_INFO,
+ "%a: BaseAddress == 0x%lx, Length == 0x%lx, Attributes == 0x%lx\n",
+ __FUNCTION__,
+ (UINTN)BaseAddress,
+ (UINTN)Length,
+ (UINTN)Attributes
+ ));
+
+ if ((Length == 0) ||
+ ((Attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_RP | EFI_MEMORY_XP)) != 0)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ 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;
+}
+
+EFI_MEMORY_ATTRIBUTE_PROTOCOL mMemoryAttribute = {
+ GetMemoryAttributes,
+ SetMemoryAttributes,
+ ClearMemoryAttributes
+};
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [NOT FOR MERGE v3 5/5] MdeModulePkg/DxeCore: add DEBUG code for memory attribute handling
2023-02-06 22:20 [PATCH v3 0/5] ArmPkg: implement EFI memory attributes protocol Ard Biesheuvel
` (3 preceding siblings ...)
2023-02-06 22:20 ` [PATCH v3 4/5] ArmPkg/CpuDxe: Implement EFI memory attributes protocol Ard Biesheuvel
@ 2023-02-06 22:21 ` Ard Biesheuvel
4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2023-02-06 22:21 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
Sami Mujawar, Taylor Beebe
NOT FOR MERGE
Add some DEBUG code to double check that the memory attributes have been
modified as expected by the code that manages read-only and/or non-exec
permissions for page allocations and loaded images.
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 79 +++++++++++++++++++-
2 files changed, 77 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 35d5bf0dee6f..87caff8289ee 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -154,6 +154,7 @@ [Protocols]
gEfiHiiPackageListProtocolGuid ## SOMETIMES_PRODUCES
gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES
gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiMemoryAttributeProtocolGuid ## SOMETIMES_CONSUMES
# Arch Protocols
gEfiBdsArchProtocolGuid ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index b89ab046fa73..18b5a1d2b69d 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -37,6 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/MemoryAttributesTable.h>
#include <Protocol/FirmwareVolume2.h>
+#include <Protocol/MemoryAttribute.h>
#include <Protocol/SimpleFileSystem.h>
#include "DxeMain.h"
@@ -66,6 +67,8 @@ extern LIST_ENTRY mGcdMemorySpaceMap;
STATIC LIST_ENTRY mProtectedImageRecordList;
+EFI_MEMORY_ATTRIBUTE_PROTOCOL *mMemoryAttribute;
+
/**
Sort code section in image record, based upon CodeSegmentBase from low to high.
@@ -226,6 +229,33 @@ SetUefiImageMemoryAttributes (
ASSERT (gCpu != NULL);
gCpu->SetMemoryAttributes (gCpu, BaseAddress, Length, FinalAttributes);
+
+ DEBUG_CODE_BEGIN ();
+
+ UINT64 OldAttributes;
+ EFI_STATUS Status;
+
+ if (mMemoryAttribute != NULL) {
+ Status = mMemoryAttribute->GetMemoryAttributes (mMemoryAttribute,
+ BaseAddress,
+ Length,
+ &OldAttributes
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ FinalAttributes &= EFI_MEMORY_ATTRIBUTE_MASK;
+ if (FinalAttributes != OldAttributes) {
+ DEBUG ((DEBUG_WARN,
+ "%a: Expected 0x%llx for new attributes, actual 0x%llx\n",
+ __FUNCTION__,
+ FinalAttributes,
+ OldAttributes
+ ));
+ ASSERT (FALSE);
+ }
+ }
+
+ DEBUG_CODE_END ();
}
/**
@@ -995,6 +1025,16 @@ MemoryProtectionCpuArchProtocolNotify (
goto Done;
}
+ DEBUG_CODE_BEGIN ();
+ //
+ // Grab a reference to the EFI memory attributes table if it exists
+ //
+ CoreLocateProtocol (&gEfiMemoryAttributeProtocolGuid,
+ NULL,
+ (VOID **)&mMemoryAttribute
+ );
+ DEBUG_CODE_END ();
+
//
// Apply the memory protection policy on non-BScode/RTcode regions.
//
@@ -1246,8 +1286,9 @@ ApplyMemoryProtectionPolicy (
IN UINT64 Length
)
{
- UINT64 OldAttributes;
- UINT64 NewAttributes;
+ UINT64 OldAttributes;
+ UINT64 NewAttributes;
+ EFI_STATUS Status;
//
// The policy configured in PcdDxeNxMemoryProtectionPolicy
@@ -1313,5 +1354,37 @@ ApplyMemoryProtectionPolicy (
return EFI_SUCCESS;
}
- return gCpu->SetMemoryAttributes (gCpu, Memory, Length, NewAttributes);
+ Status = gCpu->SetMemoryAttributes (gCpu, Memory, Length, NewAttributes);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ DEBUG_CODE_BEGIN ();
+
+ //
+ // If available, use the EFI memory attribute protocol to double
+ // check that the entire region has the expected attributes.
+ //
+ if (mMemoryAttribute != NULL) {
+ Status = mMemoryAttribute->GetMemoryAttributes (mMemoryAttribute,
+ Memory,
+ Length,
+ &OldAttributes
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ if (OldAttributes != NewAttributes) {
+ DEBUG ((DEBUG_WARN,
+ "%a: Expected 0x%llx for new attributes, actual 0x%llx\n",
+ __FUNCTION__,
+ NewAttributes,
+ OldAttributes
+ ));
+ ASSERT (FALSE);
+ }
+ }
+
+ DEBUG_CODE_END ();
+
+ return EFI_SUCCESS;
}
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread