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

From: Jan Beulich <jbeulich@suse.com>
To: Elias El Yandouzi <eliasely@amazon.com>
Cc: julien@xen.org, pdurrant@amazon.com, dwmw@amazon.com,
	"Hongyan Xia" <hongyxia@amazon.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Wei Wang" <wawei@amazon.de>, "Julien Grall" <jgrall@amazon.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v2 (resend) 12/27] x86/mapcache: Initialise the mapcache for the idle domain
Date: Tue, 20 Feb 2024 11:51:38 +0100	[thread overview]
Message-ID: <d6f3993e-5e96-4e3d-9334-9b44152f9f81@suse.com> (raw)
In-Reply-To: <20240116192611.41112-13-eliasely@amazon.com>

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.

Jan