Hi Julien, > On 20 Jul 2022, at 19:44, Julien Grall <jul...@xen.org> wrote: > > From: Julien Grall <jgr...@amazon.com> > > At the moment, the domheap mappings initialization is done separately for > the boot CPU and secondary CPUs. The main difference is for the former > the pages are part of Xen binary whilst for the latter they are > dynamically allocated. > > It would be good to have a single helper so it is easier to rework > on the domheap is initialized. > > For CPU0, we still need to use pre-allocated pages because the > allocators may use domain_map_page(), so we need to have the domheap > area ready first. But we can still delay the initialization to setup_mm(). > > Introduce a new helper init_domheap_mappings() that will be called > from setup_mm() for the boot CPU and from init_secondary_pagetables() > for secondary CPUs. > > Signed-off-by: Julien Grall <jgr...@amazon.com>
With a small typo mentioned after which can be fixed on commit: Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com> > > ---- > Changes in v2: > - Fix function name in the commit message > - Remove duplicated 'been' in the comment > --- > xen/arch/arm/include/asm/arm32/mm.h | 2 + > xen/arch/arm/mm.c | 92 +++++++++++++++++++---------- > xen/arch/arm/setup.c | 8 +++ > 3 files changed, 71 insertions(+), 31 deletions(-) > > diff --git a/xen/arch/arm/include/asm/arm32/mm.h > b/xen/arch/arm/include/asm/arm32/mm.h > index 6b039d9ceaa2..575373aeb985 100644 > --- a/xen/arch/arm/include/asm/arm32/mm.h > +++ b/xen/arch/arm/include/asm/arm32/mm.h > @@ -10,6 +10,8 @@ static inline bool arch_mfns_in_directmap(unsigned long > mfn, unsigned long nr) > return false; > } > > +bool init_domheap_mappings(unsigned int cpu); > + > #endif /* __ARM_ARM32_MM_H__ */ > > /* > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 0177bc6b34d2..9311f3530066 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -372,6 +372,58 @@ void clear_fixmap(unsigned int map) > } > > #ifdef CONFIG_DOMAIN_PAGE > +/* > + * Prepare the area that will be used to map domheap pages. They are > + * mapped in 2MB chunks, so we need to allocate the page-tables up to > + * the 2nd level. > + * > + * The caller should make sure the root page-table for @cpu has been > + * allocated. > + */ > +bool init_domheap_mappings(unsigned int cpu) > +{ > + unsigned int order = get_order_from_pages(DOMHEAP_SECOND_PAGES); > + lpae_t *root = per_cpu(xen_pgtable, cpu); > + unsigned int i, first_idx; > + lpae_t *domheap; > + mfn_t mfn; > + > + ASSERT(root); > + ASSERT(!per_cpu(xen_dommap, cpu)); > + > + /* > + * The domheap for cpu0 is before the heap is initialized. So we There is a “initialized” missing. Cheers Bertrand > + * need to use pre-allocated pages. > + */ > + if ( !cpu ) > + domheap = cpu0_dommap; > + else > + domheap = alloc_xenheap_pages(order, 0); > + > + if ( !domheap ) > + return false; > + > + /* Ensure the domheap has no stray mappings */ > + memset(domheap, 0, DOMHEAP_SECOND_PAGES * PAGE_SIZE); > + > + /* > + * Update the first level mapping to reference the local CPUs > + * domheap mapping pages. > + */ > + mfn = virt_to_mfn(domheap); > + first_idx = first_table_offset(DOMHEAP_VIRT_START); > + for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ ) > + { > + lpae_t pte = mfn_to_xen_entry(mfn_add(mfn, i), MT_NORMAL); > + pte.pt.table = 1; > + write_pte(&root[first_idx + i], pte); > + } > + > + per_cpu(xen_dommap, cpu) = domheap; > + > + return true; > +} > + > void *map_domain_page_global(mfn_t mfn) > { > return vmap(&mfn, 1); > @@ -633,16 +685,6 @@ void __init setup_pagetables(unsigned long > boot_phys_offset) > p[i].pt.xn = 0; > } > > -#ifdef CONFIG_ARM_32 > - for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ ) > - { > - p[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)] > - = pte_of_xenaddr((uintptr_t)(cpu0_dommap + > - i * XEN_PT_LPAE_ENTRIES)); > - p[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)].pt.table = 1; > - } > -#endif > - > /* Break up the Xen mapping into 4k pages and protect them separately. */ > for ( i = 0; i < XEN_PT_LPAE_ENTRIES; i++ ) > { > @@ -686,7 +728,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; > #endif > } > > @@ -719,39 +760,28 @@ int init_secondary_pagetables(int cpu) > #else > int init_secondary_pagetables(int cpu) > { > - lpae_t *first, *domheap, pte; > - int i; > + lpae_t *first; > > first = alloc_xenheap_page(); /* root == first level on 32-bit 3-level > trie */ > - domheap = > alloc_xenheap_pages(get_order_from_pages(DOMHEAP_SECOND_PAGES), 0); > > - if ( domheap == NULL || first == NULL ) > + if ( !first ) > { > - printk("Not enough free memory for secondary CPU%d pagetables\n", > cpu); > - free_xenheap_pages(domheap, > get_order_from_pages(DOMHEAP_SECOND_PAGES)); > - free_xenheap_page(first); > + printk("CPU%u: Unable to allocate the first page-table\n", cpu); > return -ENOMEM; > } > > /* Initialise root pagetable from root of boot tables */ > memcpy(first, cpu0_pgtable, PAGE_SIZE); > + per_cpu(xen_pgtable, cpu) = first; > > - /* Ensure the domheap has no stray mappings */ > - memset(domheap, 0, DOMHEAP_SECOND_PAGES*PAGE_SIZE); > - > - /* Update the first level mapping to reference the local CPUs > - * domheap mapping pages. */ > - for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ ) > + if ( !init_domheap_mappings(cpu) ) > { > - pte = mfn_to_xen_entry(virt_to_mfn(domheap + i * > XEN_PT_LPAE_ENTRIES), > - MT_NORMAL); > - pte.pt.table = 1; > - > write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte); > + printk("CPU%u: Unable to prepare the domheap page-tables\n", cpu); > + per_cpu(xen_pgtable, cpu) = NULL; > + free_xenheap_page(first); > + return -ENOMEM; > } > > - per_cpu(xen_pgtable, cpu) = first; > - per_cpu(xen_dommap, cpu) = domheap; > - > clear_boot_pagetables(); > > /* Set init_ttbr for this CPU coming up */ > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 85ff956ec2e3..068e84b10335 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -783,6 +783,14 @@ static void __init setup_mm(void) > setup_frametable_mappings(ram_start, ram_end); > max_page = PFN_DOWN(ram_end); > > + /* > + * The allocators may need to use map_domain_page() (such as for > + * scrubbing pages). So we need to prepare the domheap area first. > + */ > + if ( !init_domheap_mappings(smp_processor_id()) ) > + panic("CPU%u: Unable to prepare the domheap page-tables\n", > + smp_processor_id()); > + > /* Add xenheap memory that was not already added to the boot allocator. */ > init_xenheap_pages(mfn_to_maddr(xenheap_mfn_start), > mfn_to_maddr(xenheap_mfn_end)); > -- > 2.32.0 >