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
>
