Re: [PATCH v2 (resend) 12/27] x86/mapcache: Initialise the mapcache for the idle domain

[Elias El Yandouzi <eliasely@xxxxxxxxxx>]

On 20/02/2024 10:51, Jan Beulich wrote:
> On 16.01.2024 20:25, Elias El Yandouzi wrote:
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -750,9 +750,16 @@ int arch_domain_create(struct domain *d,
> >   
> >       spin_lock_init(&d->arch.e820_lock);
> >   
> > +    if ( (rc = mapcache_domain_init(d)) != 0)
> > +    {
> > +        free_perdomain_mappings(d);
> > +        return rc;
> > +    }
> > +
> >       /* Minimal initialisation for the idle domain. */
> >       if ( unlikely(is_idle_domain(d)) )
> >       {
> > +        struct page_info *pg = d->arch.perdomain_l3_pg;
> >           static const struct arch_csw idle_csw = {
> >               .from = paravirt_ctxt_switch_from,
> >               .to   = paravirt_ctxt_switch_to,
> > @@ -763,6 +770,9 @@ int arch_domain_create(struct domain *d,
> >   
> >           d->arch.cpu_policy = ZERO_BLOCK_PTR; /* Catch stray misuses. */
> >   
> > +        idle_pg_table[l4_table_offset(PERDOMAIN_VIRT_START)] =
> > +            l4e_from_page(pg, __PAGE_HYPERVISOR_RW);
> > +
> >           return 0;
> >       }
> Why not add another call to mapcache_domain_init() right here, allowing
> a more specific panic() to be invoked in case of failure (compared to
> the BUG_ON() upon failure of creation of the idle domain as a whole)?
> Then the other mapcache_domain_init() call doesn't need moving a 2nd
> time in close succession.
To be honest, I don't really like the idea of having twice the same call 
just for the benefit of having a panic() call in case of failure for the 
idle domain.
If you don't mind, I'd rather keep just a single call to 
mapcache_domain_init() as it is now.
Elias