From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22c.google.com (mail-wm0-x22c.google.com [IPv6:2a00:1450:400c:c09::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C66C582212 for ; Tue, 21 Feb 2017 06:04:05 -0800 (PST) Received: by mail-wm0-x22c.google.com with SMTP id v186so111578674wmd.0 for ; Tue, 21 Feb 2017 06:04:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=04E40cyCKkyATScm1DHuIz6u/zn4VAZ5Zzxng3PWxIM=; b=eH+Cv9i4TxC//3U7Jw34euf7cIWMikbr7EyXVCS0EvpeVP26ZdnPtOdu3TmsvcSHhV RkBdvIm55K9y3TvQYU22IFmBZJMIyUasKBoRjMa1FUvJLA3Hw0h/RJoZCQd6RVpGsGEQ V1ckC0W5SoT23BTAx/180r/ii1Md9pYxWe5og= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=04E40cyCKkyATScm1DHuIz6u/zn4VAZ5Zzxng3PWxIM=; b=RuZh59GesuWYfs5VkISrz3vff+h/30zixjsn/Hp/Y+WM1q5h7sCvUyjMT8KItCSbxl WhH+no1ok6kYPO1upfcUBOYM2pJKbOdezJq1p6fQN3sh8jxQ5pv8zprOjL6Lt6Tj2lup KSnAWLWN8BPGUF/vo9J/7CqRpYzXhbpb+kia3GjmGB+v7f00JVX+Y4Fl5ue9BlVkH80m HvZg601imeipzhkWZxiAm7GDwOicZaoVYmb89zmJKz5EienQS6pOyTNSd5zU4YBCu1h5 oud0hFozuC4bvWNdRSfzL5rUhmhZtAp7+QdupATEMfZCaex1kW0OuwQ4bpIQiX6JQr7P hH+g== X-Gm-Message-State: AMke39mc5hT7ZFMLDI8JO4vm+gvZ5XAR0Y9Ed1IP3UumZ3SrJjUTzwVoUmXnJAWAWopuJTqD X-Received: by 10.28.158.74 with SMTP id h71mr15212041wme.59.1487685843931; Tue, 21 Feb 2017 06:04:03 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id d29sm15560766wrc.34.2017.02.21.06.04.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 21 Feb 2017 06:04:03 -0800 (PST) Date: Tue, 21 Feb 2017 14:04:01 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, jiewen.yao@intel.com Message-ID: <20170221140401.GM16034@bivouac.eciton.net> References: <1487178716-24569-1-git-send-email-ard.biesheuvel@linaro.org> <1487178716-24569-5-git-send-email-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <1487178716-24569-5-git-send-email-ard.biesheuvel@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH v2 4/4] ArmPkg/ArmMmuLib: AARCH64: add support for modifying only permissions X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Feb 2017 14:04:06 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Feb 15, 2017 at 05:11:56PM +0000, Ard Biesheuvel wrote: > Since the new DXE page protection for PE/COFF images may invoke > EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes() with only permission > attributes set, add support for this in the AARCH64 MMU code. > > Move the EFI_MEMORY_CACHETYPE_MASK macro to a shared location between > CpuDxe and ArmMmuLib so we don't have to introduce yet another > definition. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel Reviewed-by: Leif Lindholm (This diff is also a lot less confusing :) > --- > ArmPkg/Drivers/CpuDxe/CpuDxe.h | 8 -- > ArmPkg/Include/Library/ArmLib.h | 4 + > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 91 ++++++++++++++------ > 3 files changed, 67 insertions(+), 36 deletions(-) > > diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h > index d16abe400ef3..80c305d53dd1 100644 > --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h > +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h > @@ -39,14 +39,6 @@ > #include > > > -#define EFI_MEMORY_CACHETYPE_MASK (EFI_MEMORY_UC | \ > - EFI_MEMORY_WC | \ > - EFI_MEMORY_WT | \ > - EFI_MEMORY_WB | \ > - EFI_MEMORY_UCE \ > - ) > - > - > /** > This function registers and enables the handler specified by InterruptHandler for a processor > interrupt or exception type specified by InterruptType. If InterruptHandler is NULL, then the > diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h > index 19501efa991f..24ffe9f1aaa7 100644 > --- a/ArmPkg/Include/Library/ArmLib.h > +++ b/ArmPkg/Include/Library/ArmLib.h > @@ -26,6 +26,10 @@ > #error "Unknown chipset." > #endif > > +#define EFI_MEMORY_CACHETYPE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | \ > + EFI_MEMORY_WT | EFI_MEMORY_WB | \ > + EFI_MEMORY_UCE) > + > /** > * The UEFI firmware must not use the ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_* attributes. > * > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > index 6aa970bc0514..9e0593ce598b 100644 > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > @@ -101,27 +101,6 @@ PageAttributeToGcdAttribute ( > return GcdAttributes; > } > > -ARM_MEMORY_REGION_ATTRIBUTES > -GcdAttributeToArmAttribute ( > - IN UINT64 GcdAttributes > - ) > -{ > - switch (GcdAttributes & 0xFF) { > - case EFI_MEMORY_UC: > - return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > - case EFI_MEMORY_WC: > - return ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED; > - case EFI_MEMORY_WT: > - return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH; > - case EFI_MEMORY_WB: > - return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK; > - default: > - DEBUG ((EFI_D_ERROR, "GcdAttributeToArmAttribute: 0x%lX attributes is not supported.\n", GcdAttributes)); > - ASSERT (0); > - return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > - } > -} > - > #define MIN_T0SZ 16 > #define BITS_PER_LEVEL 9 > > @@ -425,6 +404,48 @@ FillTranslationTable ( > ); > } > > +STATIC > +UINT64 > +GcdAttributeToPageAttribute ( > + IN UINT64 GcdAttributes > + ) > +{ > + UINT64 PageAttributes; > + > + switch (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) { > + case EFI_MEMORY_UC: > + PageAttributes = TT_ATTR_INDX_DEVICE_MEMORY; > + break; > + case EFI_MEMORY_WC: > + PageAttributes = TT_ATTR_INDX_MEMORY_NON_CACHEABLE; > + break; > + case EFI_MEMORY_WT: > + PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_THROUGH | TT_SH_INNER_SHAREABLE; > + break; > + case EFI_MEMORY_WB: > + PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE; > + break; > + default: > + PageAttributes = TT_ATTR_INDX_MASK; > + break; > + } > + > + if ((GcdAttributes & EFI_MEMORY_XP) != 0 || > + (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) == EFI_MEMORY_UC) { > + if (ArmReadCurrentEL () == AARCH64_EL2) { > + PageAttributes |= TT_XN_MASK; > + } else { > + PageAttributes |= TT_UXN_MASK | TT_PXN_MASK; > + } > + } > + > + if ((GcdAttributes & EFI_MEMORY_RO) != 0) { > + PageAttributes |= TT_AP_RO_RO; > + } > + > + return PageAttributes | TT_AF; > +} > + > RETURN_STATUS > SetMemoryAttributes ( > IN EFI_PHYSICAL_ADDRESS BaseAddress, > @@ -434,17 +455,31 @@ SetMemoryAttributes ( > ) > { > RETURN_STATUS Status; > - ARM_MEMORY_REGION_DESCRIPTOR MemoryRegion; > UINT64 *TranslationTable; > - > - MemoryRegion.PhysicalBase = BaseAddress; > - MemoryRegion.VirtualBase = BaseAddress; > - MemoryRegion.Length = Length; > - MemoryRegion.Attributes = GcdAttributeToArmAttribute (Attributes); > + UINT64 PageAttributes; > + UINT64 PageAttributeMask; > + > + PageAttributes = GcdAttributeToPageAttribute (Attributes); > + PageAttributeMask = 0; > + > + if ((Attributes & EFI_MEMORY_CACHETYPE_MASK) == 0) { > + // > + // No memory type was set in Attributes, so we are going to update the > + // permissions only. > + // > + PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK; > + PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK | > + TT_PXN_MASK | TT_XN_MASK); > + } > > TranslationTable = ArmGetTTBR0BaseAddress (); > > - Status = FillTranslationTable (TranslationTable, &MemoryRegion); > + Status = UpdateRegionMapping ( > + TranslationTable, > + BaseAddress, > + Length, > + PageAttributes, > + PageAttributeMask); > if (RETURN_ERROR (Status)) { > return Status; > } > -- > 2.7.4 >