[Xen-devel] [PATCH] xen: Plumb an is_priv boolean into domain_create()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The current mechanism of setting dom0->is_privileged after construction means
that the is_control_domain() predicate returns false during construction.

In particular, this means that the CPUID Faulting special case in
init_domain_msr_policy() fails to take effect.  (In actual fact, faulting
support is advertised to dom0, but attempting to configure it is silently
ignored because of the dom0 special case in ctxt_switch_levelling().)

This could be implemented using a flag in xen_domctl_createdomain, but using
an extra boolean parameter like this means that we can't accidentally allow
domain_create() to create a second dom0 due to parameter mis-auditing.

While adjusting the setting of dom0->is_privileged, drop the redundant zeroing
of dom0->target.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>

Compile tested on ARM.  Functionally tested on x86.

This ideally wants backporting, but will probably be prohibitive beyond 4.11

Semi RFC because I'm about to fix the reason for needing the dom0 special case
for Faulting, and avoid the bug that way.  OTOH, is_control_domain() ought to
work sensibly.  Thoughts?
---
 xen/arch/arm/mm.c       | 6 +++---
 xen/arch/arm/setup.c    | 5 +----
 xen/arch/x86/mm.c       | 6 +++---
 xen/arch/x86/setup.c    | 6 +-----
 xen/common/domain.c     | 4 +++-
 xen/common/domctl.c     | 2 +-
 xen/common/schedule.c   | 2 +-
 xen/include/xen/sched.h | 3 ++-
 8 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a6de77c..d234c46 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -520,7 +520,7 @@ void __init arch_init_memory(void)
      * Any Xen-heap pages that we will allow to be mapped will have
      * their domain field set to dom_xen.
      */
-    dom_xen = domain_create(DOMID_XEN, NULL);
+    dom_xen = domain_create(DOMID_XEN, NULL, false);
     BUG_ON(IS_ERR(dom_xen));
 
     /*
@@ -528,14 +528,14 @@ void __init arch_init_memory(void)
      * This domain owns I/O pages that are within the range of the page_info
      * array. Mappings occur at the priv of the caller.
      */
-    dom_io = domain_create(DOMID_IO, NULL);
+    dom_io = domain_create(DOMID_IO, NULL, false);
     BUG_ON(IS_ERR(dom_io));
 
     /*
      * Initialise our COW domain.
      * This domain owns sharable pages.
      */
-    dom_cow = domain_create(DOMID_COW, NULL);
+    dom_cow = domain_create(DOMID_COW, NULL, false);
     BUG_ON(IS_ERR(dom_cow));
 }
 
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 1d6f6bf..216572f 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -843,13 +843,10 @@ void __init start_xen(unsigned long boot_phys_offset,
     dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
     dom0_cfg.arch.nr_spis = gic_number_lines() - 32;
 
-    dom0 = domain_create(0, &dom0_cfg);
+    dom0 = domain_create(0, &dom0_cfg, true);
     if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
             panic("Error creating domain 0");
 
-    dom0->is_privileged = 1;
-    dom0->target = NULL;
-
     if ( construct_dom0(dom0) != 0)
             panic("Could not set up DOM0 guest OS");
 
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index e1032a7..a1a1f5f 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -271,7 +271,7 @@ void __init arch_init_memory(void)
      * Hidden PCI devices will also be associated with this domain
      * (but be [partly] controlled by Dom0 nevertheless).
      */
-    dom_xen = domain_create(DOMID_XEN, NULL);
+    dom_xen = domain_create(DOMID_XEN, NULL, false);
     BUG_ON(IS_ERR(dom_xen));
     INIT_LIST_HEAD(&dom_xen->arch.pdev_list);
 
@@ -280,14 +280,14 @@ void __init arch_init_memory(void)
      * This domain owns I/O pages that are within the range of the page_info
      * array. Mappings occur at the priv of the caller.
      */
-    dom_io = domain_create(DOMID_IO, NULL);
+    dom_io = domain_create(DOMID_IO, NULL, false);
     BUG_ON(IS_ERR(dom_io));
 
     /*
      * Initialise our COW domain.
      * This domain owns sharable pages.
      */
-    dom_cow = domain_create(DOMID_COW, NULL);
+    dom_cow = domain_create(DOMID_COW, NULL, false);
     BUG_ON(IS_ERR(dom_cow));
 
     /*
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 39ac130..419b46c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1660,14 +1660,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     }
 
     /* Create initial domain 0. */
-    dom0 = domain_create(get_initial_domain_id(), &dom0_cfg);
+    dom0 = domain_create(get_initial_domain_id(), &dom0_cfg, !pv_shim);
     if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
         panic("Error creating domain 0");
 
-    if ( !pv_shim )
-        dom0->is_privileged = 1;
-    dom0->target = NULL;
-
     /* Grab the DOM0 command line. */
     cmdline = (char *)(mod[0].string ? __va(mod[0].string) : NULL);
     if ( (cmdline != NULL) || (kextra != NULL) )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6cbf135..08ca4b1 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -261,7 +261,8 @@ static int __init parse_extra_guest_irqs(const char *s)
 custom_param("extra_guest_irqs", parse_extra_guest_irqs);
 
 struct domain *domain_create(domid_t domid,
-                             struct xen_domctl_createdomain *config)
+                             struct xen_domctl_createdomain *config,
+                             bool is_priv)
 {
     struct domain *d, **pd, *old_hwdom = NULL;
     enum { INIT_xsm = 1u<<0, INIT_watchdog = 1u<<1, INIT_rangeset = 1u<<2,
@@ -272,6 +273,7 @@ struct domain *domain_create(domid_t domid,
         return ERR_PTR(-ENOMEM);
 
     d->domain_id = domid;
+    d->is_privileged = is_priv;
 
     /* Debug sanity. */
     ASSERT(is_system_domain(d) ? config == NULL : config != NULL);
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 9b7bc08..39eb819 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -532,7 +532,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
             rover = dom;
         }
 
-        d = domain_create(dom, &op->u.createdomain);
+        d = domain_create(dom, &op->u.createdomain, false);
         if ( IS_ERR(d) )
         {
             ret = PTR_ERR(d);
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 049f93f..9718ce3 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1809,7 +1809,7 @@ void __init scheduler_init(void)
         sched_ratelimit_us = SCHED_DEFAULT_RATELIMIT_US;
     }
 
-    idle_domain = domain_create(DOMID_IDLE, NULL);
+    idle_domain = domain_create(DOMID_IDLE, NULL, false);
     BUG_ON(IS_ERR(idle_domain));
     idle_domain->vcpu = idle_vcpu;
     idle_domain->max_vcpus = nr_cpu_ids;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index e79d5a3..851f11e 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -546,7 +546,8 @@ void domain_update_node_affinity(struct domain *d);
  * (domid < DOMID_FIRST_RESERVED).
  */
 struct domain *domain_create(domid_t domid,
-                             struct xen_domctl_createdomain *config);
+                             struct xen_domctl_createdomain *config,
+                             bool is_priv);
 
 /*
  * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel