From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.4814.1685135252536876043 for ; Fri, 26 May 2023 14:07:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=cI7j6joO; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 1F62320FBE82; Fri, 26 May 2023 14:07:31 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1F62320FBE82 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1685135252; bh=3QN70Es0IGp8pTAI+HFtZPy/7biDWY2UnMM2lfORDAk=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=cI7j6joOky74hutlIKIsu63JbXRVmus/oVj5gXyGvVEa0QvIwzRQ7Oy0H0UBf5vQ0 0HRv1cpiTvJsl987SH487YUJNbH/+cfNoVnMTAIB5egdOlhvNoARN9Sdy9TXRd+Ug3 36craOaCAL3UXHKA7BsYV8auDsz6ZPJishflGi+4= Message-ID: <86829b06-99bc-4e68-65d3-5fa028dbed1a@linux.microsoft.com> Date: Fri, 26 May 2023 17:07:29 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [edk2-devel][PATCH v1 1/1] ArmPkg: CpuDxe: Report AARCH64 Memory Protections Attributes To GCD To: Oliver Smith-Denny , devel@edk2.groups.io Cc: Leif Lindholm , Ard Biesheuvel , Sami Mujawar , Taylor Beebe , Sean Brogan References: <20230523203623.2387-1-osde@linux.microsoft.com> From: "Michael Kubacki" In-Reply-To: <20230523203623.2387-1-osde@linux.microsoft.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Reviewed-by: Michael Kubacki On 5/23/2023 4:36 PM, Oliver Smith-Denny 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 > Cc: Ard Biesheuvel > Cc: Sami Mujawar > Cc: Taylor Beebe > Cc: Michael Kubacki > Cc: Sean Brogan > > Signed-off-by: Oliver Smith-Denny > --- > 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) ||