Re: [PATCH V4 04/15] x86: Initialize mapcache for PV, HVM, and idle domains

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/11/2024 1:11 pm, Elias El Yandouzi wrote:
> From: Wei Liu <wei.liu2@xxxxxxxxxx>
>
> To support the transition away from the direct map, the mapcache will now
> be used by HVM and idle domains as well. This patch lifts the `mapcache`
> to the arch level and moves its initialization earlier in
> `arch_domain_create()` to cover PV, HVM, and idle domains.

This part of the commit message is a correct description of how the
logic wants to end up, but ...


> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 92263a4fbdc5..a7f4929b5893 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -738,6 +738,11 @@ struct domain *domain_create(domid_t domid,
>  
>      rangeset_domain_initialise(d);
>  
> +#ifdef CONFIG_X86
> +    if ( (err = mapcache_domain_init(d)) != 0)
> +        goto fail;
> +#endif
> +
>      if ( is_idle_domain(d) )
>          arch_init_idle_domain(d);
>  
> @@ -820,6 +825,10 @@ struct domain *domain_create(domid_t domid,
>      ASSERT(err < 0);      /* Sanity check paths leading here. */
>      err = err ?: -EILSEQ; /* Release build safety. */
>  
> +#ifdef CONFIG_X86
> +    free_perdomain_mappings(d);
> +#endif
> +
>      d->is_dying = DOMDYING_dead;
>      if ( hardware_domain == d )
>          hardware_domain = old_hwdom;

... this is not what the commit message says.

These should be implemented as per the commit message. i.e. living in
arch_*() functions, not ifdef'd in common code.

If there are not arch functions in the right places, we can make the
appear.  It looks like arch_init_idle_domain() needs to grow a failure
case, but that seems to be the extent of the changes needed in order to
move these calls into arch-specific code.

~Andrew