| Message ID | 20190422164937.21350-14-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Clean-up & fixes in boot/mm code | expand |
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>
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,
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.
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 }
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(-)