From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (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 DE0DF82035 for ; Fri, 10 Feb 2017 09:56:10 -0800 (PST) Received: by mail-io0-x233.google.com with SMTP id v96so56464048ioi.0 for ; Fri, 10 Feb 2017 09:56:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=lv2CCLTKmhWBCafCRal/H2CmAglw6yQpTC4o8nQwKBE=; b=SL+wowhqJVnjMZTTkf4xM7bUhAiiVbhtqC0Fv7XSanQIdswPrwkgY+KXsYhmRw8YAs ck1IP4sK+BO8QgxlbWko7DfGGmuoIzO3l6WkdMkP9YODGZF7KtyN6j2ONTfadBRzqFil +2EFGLYW/RqjaBo4Hb2bmDkSurkGdmUkOdhsI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=lv2CCLTKmhWBCafCRal/H2CmAglw6yQpTC4o8nQwKBE=; b=oq1SzbAUlm80QLDYEgNnq3oiakzJfwyTLQZZQzhgabZsZ2fQgxdSP6vL1vyZ1zJw8X xODq7TqmVvdKMD5ZxuzSRocHiFi09LuHYRzYFVZTLsZEt5P60Y6OBVNTFJwd7I6IRnRm ILfZRElrJM9ieRqA/vBU+u0iLjGwQSiSgy2Hhzc5TO+GvubiaSFzrEYKp7mhbQZGX//B /civyEjgk4N7ln5LlB8yp2HTFM8g5BvDa8vqNYgjpzZxRS1zD0dn27Ua07gTBUHjZ1UT aQ4L8wDJaVUKBty27+wvHzfFusyiTFLR2j5hKhmIephQChxWqvrv/nCux8/XHYbyczZn t3Cg== X-Gm-Message-State: AMke39lCW8s2SlMjpz8hxFxFvo0jG9x8D1m80VIl+QGOfBBTz0niGHzQOTz8Rcdz8g7pyVNiw1sq9Z5f5KJRZ+O5 X-Received: by 10.107.32.207 with SMTP id g198mr8950772iog.83.1486749370020; Fri, 10 Feb 2017 09:56:10 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.144.139 with HTTP; Fri, 10 Feb 2017 09:56:09 -0800 (PST) In-Reply-To: <20170210175405.GN16034@bivouac.eciton.net> References: <1486661891-7888-1-git-send-email-ard.biesheuvel@linaro.org> <1486661891-7888-3-git-send-email-ard.biesheuvel@linaro.org> <20170210175405.GN16034@bivouac.eciton.net> From: Ard Biesheuvel Date: Fri, 10 Feb 2017 17:56:09 +0000 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , "Yao, Jiewen" , "Tian, Feng" , "Kinney, Michael D" , "Fan, Jeff" , "Zeng, Star" Subject: Re: [PATCH 2/4] ArmPkg/CpuDxe: translate invalid memory types in EfiAttributeToArmAttribute 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: Fri, 10 Feb 2017 17:56:11 -0000 Content-Type: text/plain; charset=UTF-8 On 10 February 2017 at 17:54, Leif Lindholm wrote: > On Thu, Feb 09, 2017 at 05:38:09PM +0000, Ard Biesheuvel wrote: >> The single user of EfiAttributeToArmAttribute () is the protocol >> method EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), which uses the >> return value to compare against the ARM attributes of an existing mapping, >> to infer whether it is actually necessary to change anything, or whether >> the requested update is redundant. This saves some cache and TLB >> maintenance on 32-bit ARM systems that use uncached translation tables. >> >> However, EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes() may be invoked with >> only permission bits set, in which case the implied requested action is to >> update the permissions of the region without modifying the cacheability >> attributes. This is currently not possible, because >> EfiAttributeToArmAttribute () ASSERT()s [on AArch64] on Attributes arguments >> that lack a cacheability bit. >> >> So let's simply return TT_ATTR_INDX_INVALID (AArch64) or >> TT_DESCRIPTOR_SECTION_TYPE_FAULT (ARM) in these cases (or'ed with the >> appropriate permission bits). This way, the return value is equally >> suitable for checking whether the attributes need to be modified, but >> in a way that accommodates the use without a cacheability bit set. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel >> --- >> ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 4 +--- >> ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 3 --- >> 2 files changed, 1 insertion(+), 6 deletions(-) >> >> diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c >> index 15d5a8173233..7688846e70cb 100644 >> --- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c >> +++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c >> @@ -216,9 +216,7 @@ EfiAttributeToArmAttribute ( >> ArmAttributes = TT_ATTR_INDX_MEMORY_WRITE_BACK; >> break; >> default: >> - DEBUG ((EFI_D_ERROR, "EfiAttributeToArmAttribute: 0x%lX attributes is not supported.\n", EfiAttributes)); >> - ASSERT (0); >> - ArmAttributes = TT_ATTR_INDX_DEVICE_MEMORY; >> + ArmAttributes = TT_ATTR_INDX_MASK; > > Commit message says TT_ATTR_INDX_INVALID - which one is intended to be > correct? > _MASK is the correct one. I will fix up the commit message when applying (assuming there are no other concerns) TT_ATTR_INDX_INVALID is named poorly, and cannnot be and'ed in a meaningful way with other TT_ flags, so I stopped using it, but forgot to update the log message