From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.14251.1684874909104963878 for ; Tue, 23 May 2023 13:48:29 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=Ka7MWmxw; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 84CBD6366A for ; Tue, 23 May 2023 20:48:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 866FAC433A4 for ; Tue, 23 May 2023 20:48:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1684874907; bh=NO5SwSDYqTIIWm38VC65tw6amgK7gKcI64MytFAR2GY=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=Ka7MWmxw+hr64Hpy+pRlWImQqa8dM8ysej+P1/Ts0V+FgvHTubiz2xmjjbe2vwoSf HGPstLeSPN1WyppAkvlAgEpDGnE/T9juk0YmXiT6tR/EOQkG9vGpd0hJYvEFC5luIW E5bHkOCvZHABwNroNaBrXW+1MbC7WK7WTwb+xK1j4hnE1QAS1KUS5c1Wcqe23alSl5 j0uZwuQxESubfTHvOXXWcHn5g2HlSd3PCP32r0AECAActCX+Q9n0L+X3jA+dX35XwC QOSN5Symcq5lQYcNL5Ra19jZ+KQ9OwjnKIZNJp1Im6FgTViHvJ+53ubDkLEyTK8BYM 7Uy7ei7RTkz/g== Received: by mail-lj1-f175.google.com with SMTP id 38308e7fff4ca-2af177f12a5so3620871fa.2 for ; Tue, 23 May 2023 13:48:27 -0700 (PDT) X-Gm-Message-State: AC+VfDw6ISUeVlaeesM0JkvgPrZIx/36QALkYEmFqaX9E5PUuqTd2uJB AjITBvDUR3Z/EeVr1yQuhUO/DgAS6ZVi3Ah4WD0= X-Google-Smtp-Source: ACHHUZ6rHBfSQztS1nj1tEZ/121qQogT9VDQKcl/8KKzTJ9oxrRiyHphxJ5+979CJ6qdP9t3+AG/nzaB+GAfuKIaWgQ= X-Received: by 2002:a05:651c:113:b0:2af:a696:3699 with SMTP id a19-20020a05651c011300b002afa6963699mr2990681ljb.52.1684874905300; Tue, 23 May 2023 13:48:25 -0700 (PDT) MIME-Version: 1.0 References: <20230523203623.2387-1-osde@linux.microsoft.com> In-Reply-To: <20230523203623.2387-1-osde@linux.microsoft.com> From: "Ard Biesheuvel" Date: Tue, 23 May 2023 22:48:14 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel][PATCH v1 1/1] ArmPkg: CpuDxe: Report AARCH64 Memory Protections Attributes To GCD To: Oliver Smith-Denny Cc: devel@edk2.groups.io, Leif Lindholm , Ard Biesheuvel , Sami Mujawar , Taylor Beebe , Michael Kubacki , Sean Brogan Content-Type: text/plain; charset="UTF-8" On Tue, 23 May 2023 at 22:36, 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 Thanks for the fix. Reviewed-by: Ard Biesheuvel > --- > 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 >