[edk2-devel][PATCH v1 1/1] ArmPkg: CpuDxe: Report AARCH64 Memory Protections Attributes To GCD

[Ard Biesheuvel]

On Tue, 23 May 2023 at 22:36, Oliver Smith-Denny
<osde at linux.microsoft.com> wrote:
>
> When the AARCH64 CpuDxe attempts to SyncCacheConfig() with the GCD,
> it collects the page attributes as:
>
> EntryAttribute = Entry & TT_ATTR_INDX_MASK
>
> However, TT_ATTR_INDX_MASK only masks the cacheability attributes
> and drops the memory protections attributes. Importantly, it also
> drops the TT_AF (access flag) which is now wired up in EDK2 to
> represent EFI_MEMORY_RP, so by default all SystemMem pages will
> report as EFI_MEMORY_RP to the GCD. The GCD currently drops that
> silently, because the Capabilities field in the GCD does not support
> EFI_MEMORY_RP by default.
>
> However, some ranges may support EFI_MEMORY_RP and incorrectly
> mark those ranges as read protected. In conjunction with
> another change on the mailing list (see:
> https://edk2.groups.io/g/devel/topic/98505340#:~:text=This%20patch%20follows%20the%20UefiCpuPkg%20pattern%20and%20adds%3D0D,between%20the%20GCD%20and%20the%20page%20table.%3D0D%20%3D0D),
> this causes an access flag fault incorrectly. See the linked
> BZ below for full details.
>
> This patch exposes all memory protections attributes to the GCD
> layer so it can correctly set pages as EFI_MEMORY[RP|XP|RO] when
> it initially syncs.
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4463
> Personal GitHub PR: https://github.com/tianocore/edk2/pull/4423
> Github branch: https://github.com/os-d/edk2/tree/aarch64_report_af_v1
>
> Cc: Leif Lindholm <quic_llindhol at quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Sami Mujawar <sami.mujawar at arm.com>
> Cc: Taylor Beebe <t at taylorbeebe.com>
> Cc: Michael Kubacki <mikuback at linux.microsoft.com>
> Cc: Sean Brogan <sean.brogan at microsoft.com>
>
> Signed-off-by: Oliver Smith-Denny <osde at linux.microsoft.com>

Thanks for the fix.

Reviewed-by: Ard Biesheuvel <ardb at kernel.org>

> ---
>  ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
> index 0859c7418a1f..1d02e41e18d8 100644
> --- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
> +++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
> @@ -120,7 +120,7 @@ GetFirstPageAttribute (
>    } else if (((FirstEntry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) ||
>               ((TableLevel == 3) && ((FirstEntry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY_LEVEL3)))
>    {
> -    return FirstEntry & TT_ATTR_INDX_MASK;
> +    return FirstEntry & TT_ATTRIBUTES_MASK;
>    } else {
>      return INVALID_ENTRY;
>    }
> @@ -158,7 +158,7 @@ GetNextEntryAttribute (
>    for (Index = 0; Index < EntryCount; Index++) {
>      Entry          = TableAddress[Index];
>      EntryType      = Entry & TT_TYPE_MASK;
> -    EntryAttribute = Entry  & TT_ATTR_INDX_MASK;
> +    EntryAttribute = Entry & TT_ATTRIBUTES_MASK;
>
>      // If Entry is a Table Descriptor type entry then go through the sub-level table
>      if ((EntryType == TT_TYPE_BLOCK_ENTRY) ||
> --
> 2.40.1
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105206): https://edk2.groups.io/g/devel/message/105206
Mute This Topic: https://groups.io/mt/99095992/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-