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