Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI CHECK]ArmPkg: CpuDxe: Report AARCH64 Memory Protections Attributes To GCD #4423

Closed
wants to merge 1 commit into from

Conversation

os-d
Copy link

@os-d os-d commented May 23, 2023

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.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4463 Personal GitHub PR:
Github branch:

Cc: Leif Lindholm quic_llindhol@quicinc.com
Cc: Ard Biesheuvel ardb+tianocore@kernel.org
Cc: Sami Mujawar sami.mujawar@arm.com
Cc: Michael Kubacki mikuback@linux.microsoft.com
Cc: Sean Brogan sean.brogan@microsoft.com

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: tianocore#4423
Github branch: https://github.com/os-d/edk2/tree/aarch64_report_af_v1

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Taylor Beebe <t@taylorbeebe.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
@os-d
Copy link
Author

os-d commented Jun 7, 2023

Closing as it was merged as: 647cd40.

@os-d os-d closed this Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant