On Mon, Mar 13, 2023 at 18:16:47 +0100, Ard Biesheuvel wrote:
> Expose the protocol introduced in v2.10 that permits the caller to
> manage mapping permissions in the page tables.
Nitpicks and a question:
> Signed-off-by: Ard Biesheuvel <a...@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 | 271 ++++++++++++++++++++
> 4 files changed, 278 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 8cb105dcc841..ce2981361aca 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..b47464c0269e
> --- /dev/null
> +++ b/ArmPkg/Drivers/CpuDxe/MemoryAttribute.c
> @@ -0,0 +1,271 @@
> +/** @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
"from different part" -> "for different parts"?
> + 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.
Question: this implementation never returns EFI_UNSUPPORTED.
Is this seen as a "some architectures may have some restricted ranges
not configurable by MMU" which simply does not apply for ARM* -
i.e. the operation is considered "supported" even if on a region not
backed by anything?
> +
> +**/
> +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);
> + *Attributes &= EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP;
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + This function set given attributes of the memory region specified by
sets
/
Leif
> + 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) {
> + 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;
> +}
> +
> +/**
> + 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_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;
> +}
> +
> +EFI_MEMORY_ATTRIBUTE_PROTOCOL mMemoryAttribute = {
> + GetMemoryAttributes,
> + SetMemoryAttributes,
> + ClearMemoryAttributes
> +};
> --
> 2.39.2
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101242): https://edk2.groups.io/g/devel/message/101242
Mute This Topic: https://groups.io/mt/97586007/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
