Re: [PATCH 1/1] xen/arm: populate a default P2M pool for new domains

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 16-Oct-26 10:16, Julian Vetter wrote:
> Since commit 9c962e07fb ("xen/arm: Clean-up in p2m_init() and
> p2m_final_teardown()"), ARM domains start with an empty P2M pool. So,
> populating the physmap fails, as no page tables can be allocated until
> the pool is sized by, e.g., the toolstack.
The history on Arm I found is as follows:
 - no dedicated P2M pool for domains
 - cbea5a1149 adds a dedicated P2M pool for non-hw domains
 - c7cff11888 adds initial P2M pool due to GICv2 requirements
 - 9c962e07fb removes the above

> 
> Restore the call to add a small default pool of 16 pages in p2m_init()
> again, as before 9c962e07fb. The hardware domain is skipped, as it
> doesn't allocate its P2M pages from the page pool. Also skip it if LLC
> coloring is enabled. The colors are only configured after domain
> creation, so the pool pages would be allocated with the default colors.
This does not say anything about why we need to have a default pool. The cover
letter you sent mentions that x86 does that but does not rationalize why. Also,
why 16 and not any other number (previously on Arm it was 16 due to GICv2
worst-case). And if we really need to have a default pool, then shouldn't we
just initialize it to the default set in dom0less case by
domain_p2m_set_allocation() and in xl case by 
libxl__get_required_paging_memory()?

~Michal

> 
> Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
> ---
>  xen/arch/arm/mmu/p2m.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/xen/arch/arm/mmu/p2m.c b/xen/arch/arm/mmu/p2m.c
> index 2cf35d8a37..de8dd26a72 100644
> --- a/xen/arch/arm/mmu/p2m.c
> +++ b/xen/arch/arm/mmu/p2m.c
> @@ -3,6 +3,7 @@
>  #include <xen/domain_page.h>
>  #include <xen/ioreq.h>
>  #include <xen/lib.h>
> +#include <xen/llc-coloring.h>
>  #include <xen/sched.h>
>  #include <xen/softirq.h>
>  
> @@ -1541,6 +1542,21 @@ int p2m_init(struct domain *d)
>      if ( rc )
>          return rc;
>  
> +    /* Populate 16 pages to the P2M pages pool here.
> +     *
> +     * Skip the hardware domain, which allocates its P2M pages from the heap,
> +     * and LLC colored setups, as the domain's colors are only configured 
> after
> +     * domain creation.
> +     */
> +    if ( !is_hardware_domain(d) && !llc_coloring_enabled )
> +    {
> +        spin_lock(&d->arch.paging.lock);
> +        rc = p2m_set_allocation(d, 16, false);
> +        spin_unlock(&d->arch.paging.lock);
> +        if ( rc )
> +            return rc;
> +    }
> +
>      return 0;
>  }
>