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

Reply via email to