From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:400e:c05::22d; helo=mail-pg0-x22d.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pg0-x22d.google.com (mail-pg0-x22d.google.com [IPv6:2607:f8b0:400e:c05::22d]) (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 05C0F2034D81B for ; Tue, 7 Nov 2017 03:34:29 -0800 (PST) Received: by mail-pg0-x22d.google.com with SMTP id o7so1981805pgc.4 for ; Tue, 07 Nov 2017 03:38:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=Yy8Eu7Hib3Mo/xxD+IAUOCHJVD0LrOlz5Q9rC8FZEF0=; b=BpuIDTEXmYi92ihrra6WhOqUaCql/wHWyZ/JZMqZ4gRnK9bOXrdp9Bnfr8HcX9PB24 J6A7NAk+0Nd95aA08RdUhE4tGDlmYaY2MOPmXOptd2yQmScE5MdpdNht8PMFQnNr8fXe GgvLYgSj/QQpax5MTICl+hjNUVEY5fA5B6DJo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=Yy8Eu7Hib3Mo/xxD+IAUOCHJVD0LrOlz5Q9rC8FZEF0=; b=apEL01dsgevYh0SHDVUdVHonyYXskrmVqmo5XvCrjWhTRqMJwExf9oA2tofVpWkKpo Q0FY/xupLlCj4p3DHMC6NtPRwUZqfpOx/IcJ3Fu5HpHpjQ9i+F/oBg6MMVJwe7WZozal QkGJXFm41Qam3V/t4UO+94UsRD1NHdqcoDu4LXXTlIrEtwooTV8kysmmpv5Sdz1cIl/t ozDRFk2zZvIr5AywVqtRjFKU1altrAJ/2U3s8qK48KRXPEUR1+1wjwUU8dCy5HEYAKNb gXkIUkQ+Q3IHH5ENkLDkn/09/30EGDNJePWaTmZY//E7oV3ufGVXp1cFR6u6WFI8pVwk QmQw== X-Gm-Message-State: AMCzsaUDNkuG00Rus1NWLz1zRPutVC04Wp9SxN9m6/aoD7gst+nDSlgx 88Dmxbz8NZwtvoECqAkiy7HW5A== X-Google-Smtp-Source: ABhQp+SIfeH6z+ictMwOuMq7Y4J37qo3GhrObnQyMsiWpZaH89VSY4kBolTza/Qo72omxtF212vGpg== X-Received: by 10.84.235.137 with SMTP id p9mr16989399plk.291.1510054708835; Tue, 07 Nov 2017 03:38:28 -0800 (PST) Received: from [10.189.19.158] ([45.56.152.90]) by smtp.gmail.com with ESMTPSA id r18sm2944482pfi.59.2017.11.07.03.38.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 07 Nov 2017 03:38:28 -0800 (PST) To: Ard Biesheuvel , Leif Lindholm Cc: linaro-uefi , "edk2-devel@lists.01.org" , Peicong Li References: <1510052748-5564-1-git-send-email-heyi.guo@linaro.org> <20171107112326.atof7lwtgxn5s7nv@bivouac.eciton.net> From: Heyi Guo Message-ID: <4c87b613-a25f-9c6b-6ca9-faf730dc19c5@linaro.org> Date: Tue, 7 Nov 2017 19:37:16 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [RFC] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHARE X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Nov 2017 11:34:30 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US 在 11/7/2017 7:25 PM, Ard Biesheuvel 写道: > On 7 November 2017 at 11:23, Leif Lindholm wrote: >> On Tue, Nov 07, 2017 at 11:08:39AM +0000, Ard Biesheuvel wrote: >>> On 7 November 2017 at 11:05, Heyi Guo wrote: >>>> From: Peicong Li >>>> >>>> Flash region needs to be set as cacheable (write back) to increase >>>> performance, if PEI is still XIP on flash or DXE FV is decompressed >>>> from flash FV. However some ARM platforms do not support to set flash >>>> as inner shareable since flash is not normal DDR memory and it will >>>> not respond to cache snoop request, which will causes system hang >>>> after MMU is enabled. >>>> >>>> So we need a new ARM memory region attribute WRITE_BACK_NONSHARE for >>>> flash region on these platforms specifically. This attribute will set >>>> the region as write back but not inner shared. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Peicong Li >>>> Signed-off-by: Heyi Guo >>>> Cc: Leif Lindholm >>>> Cc: Ard Biesheuvel >>>> --- >>>> ArmPkg/Include/Library/ArmLib.h | 2 ++ >>>> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 ++++ >>>> 2 files changed, 6 insertions(+) >>>> >>>> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h >>>> index 24ffe9f..e43e375 100644 >>>> --- a/ArmPkg/Include/Library/ArmLib.h >>>> +++ b/ArmPkg/Include/Library/ArmLib.h >>>> @@ -39,6 +39,8 @@ >>>> typedef enum { >>>> ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED = 0, >>>> ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED, >>>> + ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE, >>>> + ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE, >>>> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK, >>>> ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK, >>>> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH, >>>> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c >>>> index 8bd1c6f..cc10143 100644 >>>> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c >>>> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c >>>> @@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute ( >>>> ) >>>> { >>>> switch (Attributes) { >>>> + case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE: >>>> + case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE: >>>> + return TT_ATTR_INDX_MEMORY_WRITE_BACK; >>>> + >>>> case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK: >>>> case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK: >>>> return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE; >>>> -- >>>> 2.7.2.windows.1 >>>> >>> I'd prefer the name >>> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE but other than >>> that, this looks sensible to me. Leif? >> And the same for NONSECURE, yes. >> With that modification, this sounds like something absolutely required >> in this situation. >> >> Does this scenario have any further implications for runtime use? >> > I don't think so. These attributes are only used to select the > attributes UEFI uses for its own mapping, and they should only be used > for non-DRAM, so they shouldn't leak into the UEFI memory map in a way > the OS would be able to notice. Agree, and we will convert flash region attribute into "UC" to support flash write and erase operation in DXE. We may also add some comment in the code to warn the usage of these two types :) Thanks, Heyi