From: Peicong Li lipeicong@huawei.com
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 lipeicong@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Cc: Leif Lindholm leif.lindholm@linaro.org Cc: Ard Biesheuvel ard.biesheuvel@linaro.org --- 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;
On 7 November 2017 at 11:05, Heyi Guo heyi.guo@linaro.org wrote:
From: Peicong Li lipeicong@huawei.com
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 lipeicong@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Cc: Leif Lindholm leif.lindholm@linaro.org Cc: Ard Biesheuvel ard.biesheuvel@linaro.org
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?
On Tue, Nov 07, 2017 at 11:08:39AM +0000, Ard Biesheuvel wrote:
On 7 November 2017 at 11:05, Heyi Guo heyi.guo@linaro.org wrote:
From: Peicong Li lipeicong@huawei.com
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 lipeicong@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Cc: Leif Lindholm leif.lindholm@linaro.org Cc: Ard Biesheuvel ard.biesheuvel@linaro.org
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?
/ Leif
On 7 November 2017 at 11:23, Leif Lindholm leif.lindholm@linaro.org wrote:
On Tue, Nov 07, 2017 at 11:08:39AM +0000, Ard Biesheuvel wrote:
On 7 November 2017 at 11:05, Heyi Guo heyi.guo@linaro.org wrote:
From: Peicong Li lipeicong@huawei.com
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 lipeicong@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Cc: Leif Lindholm leif.lindholm@linaro.org Cc: Ard Biesheuvel ard.biesheuvel@linaro.org
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.
On Tue, Nov 07, 2017 at 11:25:16AM +0000, Ard Biesheuvel wrote:
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.
Yes, that makes sense. Just wanted to double-check.
/ Leif
在 11/7/2017 7:25 PM, Ard Biesheuvel 写道:
On 7 November 2017 at 11:23, Leif Lindholm leif.lindholm@linaro.org wrote:
On Tue, Nov 07, 2017 at 11:08:39AM +0000, Ard Biesheuvel wrote:
On 7 November 2017 at 11:05, Heyi Guo heyi.guo@linaro.org wrote:
From: Peicong Li lipeicong@huawei.com
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 lipeicong@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Cc: Leif Lindholm leif.lindholm@linaro.org Cc: Ard Biesheuvel ard.biesheuvel@linaro.org
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
-
Ard Biesheuvel -
Heyi Guo -
Leif Lindholm