[Xen-devel,13/20] xen/arm32: mm: Avoid to zero and clean cache for CPU0 domheap

Message ID 20190422164937.21350-14-julien.grall@arm.com
State Superseded
Headers show
Series
  • xen/arm: Clean-up & fixes in boot/mm code
Related show

Commit Message

Julien Grall April 22, 2019, 4:49 p.m.
The page-table walker is configured to use the same shareability and
cacheability as the access performed when updating the page-tables. This
means cleaning the cache for CPU0 domheap is unnecessary.

Furthermore, CPU0 page-tables are part of Xen binary and will already be
zeroed beforehand. So it is pointless to zero the domheap again.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 5 -----
 1 file changed, 5 deletions(-)

Comments

Andrii Anisov May 3, 2019, 3:57 p.m. | #1
Hello Julien,

On 22.04.19 19:49, Julien Grall wrote:
> The page-table walker is configured to use the same shareability and
> cacheability as the access performed when updating the page-tables. This
> means cleaning the cache for CPU0 domheap is unnecessary.
> 
> Furthermore, CPU0 page-tables are part of Xen binary and will already be
> zeroed beforehand.

IMO it is a bit confusing.
As I understand, `cpu0_dommap` resides in BSS which is not a part of the binary unlike initialized data. Yet it is unconditionally cleared during the boot on ARM32.

>  So it is pointless to zero the domheap again.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---

Though:

Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
Julien Grall May 3, 2019, 5:06 p.m. | #2
On 03/05/2019 16:57, Andrii Anisov wrote:
> Hello Julien,

Hi,

> 
> On 22.04.19 19:49, Julien Grall wrote:
>> The page-table walker is configured to use the same shareability and
>> cacheability as the access performed when updating the page-tables. This
>> means cleaning the cache for CPU0 domheap is unnecessary.
>>
>> Furthermore, CPU0 page-tables are part of Xen binary and will already be
>> zeroed beforehand.
> 
> IMO it is a bit confusing.
> As I understand, `cpu0_dommap` resides in BSS which is not a part of the binary 
> unlike initialized data. Yet it is unconditionally cleared during the boot on 
> ARM32.

In C, uninitialized global variable will be zero by default. It is a bit of 
waste to allocate space in the binary for them. So the compiler will commonly 
put them in a section BSS that are going to be zeroed when at launch.

On Arm32, this is always done in CPU0 at early boot. For Arm64, UEFI will do it 
for us, so we don't want to do it when using UEFI as we may override global

The reason I chose to say "will always be zeroed beforehand" than specifically 
mention "BSS" is I wasn't entirely convinced the compiler will always put in BSS.

Cheers,
Andrii Anisov May 6, 2019, 8:28 a.m. | #3
On 03.05.19 20:06, Julien Grall wrote:
> In C, uninitialized global variable will be zero by default. It is a bit of waste to allocate space in the binary for them. So the compiler will commonly put them in a section BSS that are going to be zeroed when at launch.
> 
> On Arm32, this is always done in CPU0 at early boot. For Arm64, UEFI will do it for us, so we don't want to do it when using UEFI as we may override global
> 
> The reason I chose to say "will always be zeroed beforehand" than specifically mention "BSS" is I wasn't entirely convinced the compiler will always put in BSS.

OK.

Patch

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index e090afb976..cda2847d00 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -724,11 +724,6 @@  void __init setup_pagetables(unsigned long boot_phys_offset)
 #ifdef CONFIG_ARM_32
     per_cpu(xen_pgtable, 0) = cpu0_pgtable;
     per_cpu(xen_dommap, 0) = cpu0_dommap;
-
-    /* Make sure it is clear */
-    memset(this_cpu(xen_dommap), 0, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
-    clean_dcache_va_range(this_cpu(xen_dommap),
-                              DOMHEAP_SECOND_PAGES*PAGE_SIZE);
 #endif
 }