From nobody Mon Sep 16 19:28:47 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+105301+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+105301+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=kernel.org ARC-Seal: i=1; a=rsa-sha256; t=1685025066; cv=none; d=zohomail.com; s=zohoarc; b=LLz6dEPo5bHd5ick8M6S5eqml9BPqr99739rRFPrsc8yACMvdEhOGr1zzai5fZv7CJVGKTdyZj0zvY1cMnpROafc5kGgAPlJQB3Unz5LsmwrRQHMxdkoqKjK0BVyeu918022Wxm16St4JUrbtBKijc4qnnpDNQmt/P6Jzm9vEro= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1685025066; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=G0WHnj2XgRu0uAURzqBVkk/G20/rLXjsqW3Jr0oWjGE=; b=KkIPYRLYi8ggOibzvEtF2LtIMmkPn+4v1tdEOyqBJJ8VsULeTpSzajHH/+KSxcloMg1FTrj/u0z9QAdIDFqPFafdnxPMzxYkBcq3UyXHXolEoKdtIjc9Qys5nJS0gb76as8EvNUBZx6VjXFbdf+rHGu2tpbqdsY4yKL/uf/B0nk= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+105301+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1685025066508250.33998375056387; Thu, 25 May 2023 07:31:06 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id jVUBYY1788612xk9TiXvE80s; Thu, 25 May 2023 07:31:06 -0700 X-Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.13269.1685025062370989750 for ; Thu, 25 May 2023 07:31:02 -0700 X-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 C1754645F3; Thu, 25 May 2023 14:31:01 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id A71BBC4339C; Thu, 25 May 2023 14:30:58 +0000 (UTC) From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: Ard Biesheuvel , Ray Ni , Jiewen Yao , Gerd Hoffmann , Taylor Beebe , Oliver Smith-Denny , Dandan Bi , Liming Gao , "Kinney, Michael D" , Leif Lindholm , Sunil V L , Andrei Warkentin Subject: [edk2-devel] [RFC PATCH 01/10] ArmPkg/ArmMmuLib: Extend API to manage memory permissions better Date: Thu, 25 May 2023 16:30:32 +0200 Message-Id: <20230525143041.1172989-2-ardb@kernel.org> In-Reply-To: <20230525143041.1172989-1-ardb@kernel.org> References: <20230525143041.1172989-1-ardb@kernel.org> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org X-Gm-Message-State: 9ujUK79cldbLu6PNksHC3NuOx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1685025066; bh=tlxHqLgyk1XYCpkjB5P5UOAeM7tpl4hCnUPfD3vo1xI=; h=Cc:Date:From:Reply-To:Subject:To; b=DcwETW5yPHIUoZS69YVxJ2RfAvUxtTHmIDhS4d2O+LAeE1fZAHJ90qFqrxsOqUKXBuf IxHZzYZPmuNY3hiOq90JsYEiZouJ5LD2butG0VycN4Yq1UQEh7tqOfcL6cwMjkTDzvI2S wZfwa68NiX1rr92FXU6UHq6l+7whbjv+8eU= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1685025068620100006 Content-Type: text/plain; charset="utf-8" Currently, ArmSetMemoryAttributes () takes a combination of EFI_MEMORY_xx constants describing the memory type and permission attributes that should be set on a region of memory. In cases where the memory type is omitted, we assume that the memory permissions being set are final, and that existing memory permissions can be discarded. This is problematic, because we aim to map memory non-executable (EFI_MEMORY_XP) by default, and only relax this requirement for code regions that are mapped read-only (EFI_MEMORY_RO). Currently, setting one permission clears the other, and so code managing these permissions has to be aware of the existing permissions in order to be able to preserve them, and this is not always tractable (e.g., the UEFI memory attribute protocol implements an abstraction that promises to preserve memory permissions that it is not operating on explicitly). So let's add an AttributeMask parameter to ArmSetMemoryAttributes(), which is permitted to be non-zero if no memory type is being provided, in which case only memory permission attributes covered in the mask will be affected by the update. Signed-off-by: Ard Biesheuvel --- ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +- ArmPkg/Include/Library/ArmMmuLib.h | 36 +++++++- ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 52 +++++++++++- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 88 +++++++++++++++++--- ArmPkg/Library/OpteeLib/Optee.c | 2 +- 5 files changed, 165 insertions(+), 15 deletions(-) diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/C= puMmuCommon.c index 2e73719dce04ceb5..2d60c7d24dc05ee9 100644 --- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c +++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c @@ -217,7 +217,7 @@ CpuSetMemoryAttributes ( if (EFI_ERROR (Status) || (RegionArmAttributes !=3D ArmAttributes) || ((BaseAddress + Length) > (RegionBaseAddress + RegionLength))) { - return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes); + return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0); } else { return EFI_SUCCESS; } diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/Ar= mMmuLib.h index 4cf59a1e376b123c..91d112314fdf4859 100644 --- a/ArmPkg/Include/Library/ArmMmuLib.h +++ b/ArmPkg/Include/Library/ArmMmuLib.h @@ -92,11 +92,45 @@ ArmReplaceLiveTranslationEntry ( IN BOOLEAN DisableMmu ); =20 +/** + Set the requested memory permission attributes on a region of memory. + + BaseAddress and Length must be aligned to EFI_PAGE_SIZE. + + If Attributes contains a memory type attribute (EFI_MEMORY_UC/WC/WT/WB),= the + region is mapped according to this memory type, and additional memory + permission attributes (EFI_MEMORY_RP/RO/XP) are taken into account as we= ll, + discarding any permission attributes that are currently set for the regi= on. + AttributeMask is ignored in this case, and must be set to 0x0. + + If Attributes contains only a combination of memory permission attributes + (EFI_MEMORY_RP/RO/XP), each page in the region will retain its existing + memory type, even if it is not uniformly set across the region. In this = case, + AttributesMask may be set to a mask of permission attributes, and memory + permissions omitted from this mask will not be updated for any page in t= he + region. All attributes appearing in Attributes must appear in AttributeM= ask + as well. (Attributes & ~AttributeMask must produce 0x0) + + @param[in] BaseAddress The physical address that is the start addre= ss of + a memory region. + @param[in] Length The size in bytes of the memory region. + @param[in] Attributes Mask of memory attributes to set. + @param[in] AttributeMask Mask of memory attributes to take into accou= nt. + + @retval EFI_SUCCESS The attributes were set for the memory reg= ion. + @retval EFI_INVALID_PARAMETER BaseAddress or Length is not suitably alig= ned. + Invalid combination of Attributes and + AttributeMask. + @retval EFI_OUT_OF_RESOURCES Requested attributes cannot be applied due= to + lack of system resources. + +**/ EFI_STATUS ArmSetMemoryAttributes ( IN EFI_PHYSICAL_ADDRESS BaseAddress, IN UINT64 Length, - IN UINT64 Attributes + IN UINT64 Attributes, + IN UINT64 AttributeMask ); =20 #endif // ARM_MMU_LIB_H_ diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Libr= ary/ArmMmuLib/AArch64/ArmMmuLibCore.c index 7ed758fbbc699732..22623572b9cb931c 100644 --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c @@ -469,11 +469,45 @@ GcdAttributeToPageAttribute ( return PageAttributes; } =20 +/** + Set the requested memory permission attributes on a region of memory. + + BaseAddress and Length must be aligned to EFI_PAGE_SIZE. + + If Attributes contains a memory type attribute (EFI_MEMORY_UC/WC/WT/WB),= the + region is mapped according to this memory type, and additional memory + permission attributes (EFI_MEMORY_RP/RO/XP) are taken into account as we= ll, + discarding any permission attributes that are currently set for the regi= on. + AttributeMask is ignored in this case, and must be set to 0x0. + + If Attributes contains only a combination of memory permission attributes + (EFI_MEMORY_RP/RO/XP), each page in the region will retain its existing + memory type, even if it is not uniformly set across the region. In this = case, + AttributesMask may be set to a mask of permission attributes, and memory + permissions omitted from this mask will not be updated for any page in t= he + region. All attributes appearing in Attributes must appear in AttributeM= ask + as well. (Attributes & ~AttributeMask must produce 0x0) + + @param[in] BaseAddress The physical address that is the start addre= ss of + a memory region. + @param[in] Length The size in bytes of the memory region. + @param[in] Attributes Mask of memory attributes to set. + @param[in] AttributeMask Mask of memory attributes to take into accou= nt. + + @retval EFI_SUCCESS The attributes were set for the memory reg= ion. + @retval EFI_INVALID_PARAMETER BaseAddress or Length is not suitably alig= ned. + Invalid combination of Attributes and + AttributeMask. + @retval EFI_OUT_OF_RESOURCES Requested attributes cannot be applied due= to + lack of system resources. + +**/ EFI_STATUS ArmSetMemoryAttributes ( IN EFI_PHYSICAL_ADDRESS BaseAddress, IN UINT64 Length, - IN UINT64 Attributes + IN UINT64 Attributes, + IN UINT64 AttributeMask ) { UINT64 PageAttributes; @@ -490,6 +524,22 @@ ArmSetMemoryAttributes ( PageAttributes &=3D TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK | TT_AF; PageAttributeMask =3D ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK | TT_PXN_MASK | TT_XN_MASK | TT_AF); + if (AttributeMask !=3D 0) { + if (((AttributeMask & ~(UINT64)(EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMO= RY_XP)) !=3D 0) || + ((Attributes & ~AttributeMask) !=3D 0)) + { + return EFI_INVALID_PARAMETER; + } + + // Add attributes omitted from AttributeMask to the set of attribute= s to preserve + PageAttributeMask |=3D GcdAttributeToPageAttribute (~AttributeMask) & + (TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK | TT_AF= ); + } + } else { + ASSERT (AttributeMask =3D=3D 0); + if (AttributeMask !=3D 0) { + return EFI_INVALID_PARAMETER; + } } =20 return UpdateRegionMapping ( diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c b/ArmPkg/Librar= y/ArmMmuLib/Arm/ArmMmuLibUpdate.c index 299d38ad07e85059..61405965a73eaeb8 100644 --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c @@ -10,6 +10,7 @@ #include =20 #include +#include #include #include #include @@ -451,31 +452,96 @@ SetMemoryAttributes ( } =20 /** - Update the permission or memory type attributes on a range of memory. + Set the requested memory permission attributes on a region of memory. =20 - @param BaseAddress The start of the region. - @param Length The size of the region. - @param Attributes A mask of EFI_MEMORY_xx constants. + BaseAddress and Length must be aligned to EFI_PAGE_SIZE. =20 - @retval EFI_SUCCESS The attributes were set successfully. - @retval EFI_OUT_OF_RESOURCES The operation failed due to insufficient m= emory. + If Attributes contains a memory type attribute (EFI_MEMORY_UC/WC/WT/WB),= the + region is mapped according to this memory type, and additional memory + permission attributes (EFI_MEMORY_RP/RO/XP) are taken into account as we= ll, + discarding any permission attributes that are currently set for the regi= on. + AttributeMask is ignored in this case, and must be set to 0x0. + + If Attributes contains only a combination of memory permission attributes + (EFI_MEMORY_RP/RO/XP), each page in the region will retain its existing + memory type, even if it is not uniformly set across the region. In this = case, + AttributesMask may be set to a mask of permission attributes, and memory + permissions omitted from this mask will not be updated for any page in t= he + region. All attributes appearing in Attributes must appear in AttributeM= ask + as well. (Attributes & ~AttributeMask must produce 0x0) + + @param[in] BaseAddress The physical address that is the start addre= ss of + a memory region. + @param[in] Length The size in bytes of the memory region. + @param[in] Attributes Mask of memory attributes to set. + @param[in] AttributeMask Mask of memory attributes to take into accou= nt. + + @retval EFI_SUCCESS The attributes were set for the memory reg= ion. + @retval EFI_INVALID_PARAMETER BaseAddress or Length is not suitably alig= ned. + Invalid combination of Attributes and + AttributeMask. + @retval EFI_OUT_OF_RESOURCES Requested attributes cannot be applied due= to + lack of system resources. =20 **/ EFI_STATUS ArmSetMemoryAttributes ( IN EFI_PHYSICAL_ADDRESS BaseAddress, IN UINT64 Length, - IN UINT64 Attributes + IN UINT64 Attributes, + IN UINT64 AttributeMask ) { + UINT32 TtEntryMask; + + if (((BaseAddress | Length) & EFI_PAGE_MASK) !=3D 0) { + return EFI_INVALID_PARAMETER; + } + + if ((Attributes & EFI_MEMORY_CACHETYPE_MASK) =3D=3D 0) { + // + // No memory type was set in Attributes, so we are going to update the + // permissions only. + // + if (AttributeMask !=3D 0) { + if (((AttributeMask & ~(UINT64)(EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMO= RY_XP)) !=3D 0) || + ((Attributes & ~AttributeMask) !=3D 0)) + { + return EFI_INVALID_PARAMETER; + } + } else { + AttributeMask =3D EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP; + } + + TtEntryMask =3D 0; + if ((AttributeMask & EFI_MEMORY_RP) !=3D 0) { + TtEntryMask |=3D TT_DESCRIPTOR_SECTION_AF; + } + + if ((AttributeMask & EFI_MEMORY_RO) !=3D 0) { + TtEntryMask |=3D TT_DESCRIPTOR_SECTION_AP_MASK; + } + + if ((AttributeMask & EFI_MEMORY_XP) !=3D 0) { + TtEntryMask |=3D TT_DESCRIPTOR_SECTION_XN_MASK; + } + } else { + ASSERT (AttributeMask =3D=3D 0); + if (AttributeMask !=3D 0) { + return EFI_INVALID_PARAMETER; + } + + TtEntryMask =3D TT_DESCRIPTOR_SECTION_TYPE_MASK | + TT_DESCRIPTOR_SECTION_XN_MASK | + TT_DESCRIPTOR_SECTION_AP_MASK | + TT_DESCRIPTOR_SECTION_AF; + } + return SetMemoryAttributes ( BaseAddress, Length, Attributes, - TT_DESCRIPTOR_SECTION_TYPE_MASK | - TT_DESCRIPTOR_SECTION_XN_MASK | - TT_DESCRIPTOR_SECTION_AP_MASK | - TT_DESCRIPTOR_SECTION_AF + TtEntryMask ); } =20 diff --git a/ArmPkg/Library/OpteeLib/Optee.c b/ArmPkg/Library/OpteeLib/Opte= e.c index 48e33cb3d5ee4ab6..46464f17ef06653e 100644 --- a/ArmPkg/Library/OpteeLib/Optee.c +++ b/ArmPkg/Library/OpteeLib/Optee.c @@ -86,7 +86,7 @@ OpteeSharedMemoryRemap ( return EFI_BUFFER_TOO_SMALL; } =20 - Status =3D ArmSetMemoryAttributes (PhysicalAddress, Size, EFI_MEMORY_WB); + Status =3D ArmSetMemoryAttributes (PhysicalAddress, Size, EFI_MEMORY_WB,= 0); if (EFI_ERROR (Status)) { return Status; } --=20 2.39.2 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105301): https://edk2.groups.io/g/devel/message/105301 Mute This Topic: https://groups.io/mt/99131174/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-