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 <a...@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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105653): https://edk2.groups.io/g/devel/message/105653
Mute This Topic: https://groups.io/mt/99288483/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
