[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 6/7] xen/domain: Pass the full domctl_createdomain struct to create_domain()



Hi Andrew,

On 03/09/2018 01:18 PM, Andrew Cooper wrote:
In future patches, the structure will be extended with further information,
and this is far cleaner than adding extra parameters.

One minor tweak is that the setting of guest_type needs to be deferred until
config is known-good to dereference, but this doesn't result in any changed
behaviour as system domains never used to pass XEN_DOMCTL_CDF_hvm_guest.

Also for completeness, move the setting of d->handle into the tail of
domain_create() where it more logically should live.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Acked-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

---
CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
---
  xen/arch/arm/domain.c    | 16 ++++++++--------
  xen/arch/arm/mm.c        |  6 +++---
  xen/arch/arm/setup.c     |  8 ++++----
  xen/arch/x86/domain.c    | 12 ++++++------
  xen/arch/x86/mm.c        |  6 +++---
  xen/arch/x86/setup.c     | 18 +++++++++++-------
  xen/common/domain.c      | 31 ++++++++++++++++++++-----------
  xen/common/domctl.c      |  8 +-------
  xen/common/schedule.c    |  2 +-
  xen/include/xen/domain.h |  4 ++--
  xen/include/xen/sched.h  |  5 ++---
  11 files changed, 61 insertions(+), 55 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 291c282..4b45fad 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -573,8 +573,8 @@ void vcpu_switch_to_aarch64_mode(struct vcpu *v)
      v->arch.hcr_el2 |= HCR_RW;
  }
-int arch_domain_create(struct domain *d, unsigned int domcr_flags,
-                       struct xen_arch_domainconfig *config)
+int arch_domain_create(struct domain *d,
+                       struct xen_domctl_createdomain *config)
  {
      int rc, count = 0;
@@ -585,7 +585,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
      if ( is_idle_domain(d) )
          return 0;
- if ( domcr_flags != (XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap) )
+    if ( config->flags != (XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap) )
          return -EINVAL;
ASSERT(config != NULL);
@@ -605,18 +605,18 @@ int arch_domain_create(struct domain *d, unsigned int 
domcr_flags,
      share_xen_page_with_guest(
          virt_to_page(d->shared_info), d, XENSHARE_writable);
- switch ( config->gic_version )
+    switch ( config->config.gic_version )
      {
      case XEN_DOMCTL_CONFIG_GIC_NATIVE:
          switch ( gic_hw_version () )
          {
          case GIC_V2:
-            config->gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
+            config->config.gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
              d->arch.vgic.version = GIC_V2;
              break;
case GIC_V3:
-            config->gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
+            config->config.gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
              d->arch.vgic.version = GIC_V3;
              break;
@@ -644,10 +644,10 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
      if ( (rc = domain_io_init(d, count + MAX_IO_HANDLER)) != 0 )
          goto fail;
- if ( (rc = domain_vgic_init(d, config->nr_spis)) != 0 )
+    if ( (rc = domain_vgic_init(d, config->config.nr_spis)) != 0 )
          goto fail;
- if ( (rc = domain_vtimer_init(d, config)) != 0 )
+    if ( (rc = domain_vtimer_init(d, &config->config)) != 0 )
          goto fail;
update_domain_wallclock_time(d);
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index ce83f69..a09bea2 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, 0, 0, NULL);
+    dom_xen = domain_create(DOMID_XEN, NULL);
      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, 0, 0, NULL);
+    dom_io = domain_create(DOMID_IO, NULL);
      BUG_ON(IS_ERR(dom_io));
/*
       * Initialise our COW domain.
       * This domain owns sharable pages.
       */
-    dom_cow = domain_create(DOMID_COW, 0, 0, NULL);
+    dom_cow = domain_create(DOMID_COW, NULL);
      BUG_ON(IS_ERR(dom_cow));
  }
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 4627366..b17797d 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -693,7 +693,7 @@ void __init start_xen(unsigned long boot_phys_offset,
      const char *cmdline;
      struct bootmodule *xen_bootmodule;
      struct domain *dom0;
-    struct xen_arch_domainconfig config;
+    struct xen_domctl_createdomain dom0_cfg = {};
dcache_line_bytes = read_dcache_line_bytes(); @@ -840,10 +840,10 @@ void __init start_xen(unsigned long boot_phys_offset, /* Create initial domain 0. */
      /* The vGIC for DOM0 is exactly emulating the hardware GIC */
-    config.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
-    config.nr_spis = gic_number_lines() - 32;
+    dom0_cfg.config.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
+    dom0_cfg.config.nr_spis = gic_number_lines() - 32;
- dom0 = domain_create(0, 0, 0, &config);
+    dom0 = domain_create(0, &dom0_cfg);
      if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
              panic("Error creating domain 0");
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 48dc2b9..12d0766 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -426,8 +426,8 @@ static bool emulation_flags_ok(const struct domain *d, 
uint32_t emflags)
      return true;
  }
-int arch_domain_create(struct domain *d, unsigned int domcr_flags,
-                       struct xen_arch_domainconfig *config)
+int arch_domain_create(struct domain *d,
+                       struct xen_domctl_createdomain *config)
  {
      bool paging_initialised = false;
      uint32_t emflags;
@@ -473,9 +473,9 @@ int arch_domain_create(struct domain *d, unsigned int 
domcr_flags,
                 d->domain_id);
      }
- d->arch.s3_integrity = domcr_flags & XEN_DOMCTL_CDF_s3_integrity;
+    d->arch.s3_integrity = config->flags & XEN_DOMCTL_CDF_s3_integrity;
- emflags = config->emulation_flags;
+    emflags = config->config.emulation_flags;
if ( is_hardware_domain(d) && is_pv_domain(d) )
          emflags |= XEN_X86_EMU_PIT;
@@ -502,9 +502,9 @@ int arch_domain_create(struct domain *d, unsigned int 
domcr_flags,
      /* Need to determine if HAP is enabled before initialising paging */
      if ( is_hvm_domain(d) )
          d->arch.hvm_domain.hap_enabled =
-            hvm_funcs.hap_supported && (domcr_flags & XEN_DOMCTL_CDF_hap);
+            hvm_funcs.hap_supported && (config->flags & XEN_DOMCTL_CDF_hap);
- if ( (rc = paging_domain_init(d, domcr_flags)) != 0 )
+    if ( (rc = paging_domain_init(d, config->flags)) != 0 )
          goto fail;
      paging_initialised = true;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index c275d4b..1d4e396 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, 0, 0, NULL);
+    dom_xen = domain_create(DOMID_XEN, NULL);
      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, 0, 0, NULL);
+    dom_io = domain_create(DOMID_IO, NULL);
      BUG_ON(IS_ERR(dom_io));
/*
       * Initialise our COW domain.
       * This domain owns sharable pages.
       */
-    dom_cow = domain_create(DOMID_COW, 0, 0, NULL);
+    dom_cow = domain_create(DOMID_COW, NULL);
      BUG_ON(IS_ERR(dom_cow));
/*
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a6dc5df..a4405b4 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -660,7 +660,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  {
      char *memmap_type = NULL;
      char *cmdline, *kextra, *loader;
-    unsigned int initrdidx, domcr_flags = XEN_DOMCTL_CDF_s3_integrity;
+    unsigned int initrdidx;
      multiboot_info_t *mbi;
      module_t *mod;
      unsigned long nr_pages, raw_max_page, modules_headroom, *module_map;
@@ -671,7 +671,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
          .parity    = 'n',
          .stop_bits = 1
      };
-    struct xen_arch_domainconfig config = { .emulation_flags = 0 };
+    struct xen_domctl_createdomain dom0_cfg = {
+        .flags = XEN_DOMCTL_CDF_s3_integrity,
+    };
/* Critical region without IDT or TSS. Any fault is deadly! */ @@ -1632,14 +1634,16 @@ void __init noreturn __start_xen(unsigned long mbi_p) if ( dom0_pvh )
      {
-        domcr_flags |= XEN_DOMCTL_CDF_hvm_guest |
-                       ((hvm_funcs.hap_supported && !opt_dom0_shadow) ?
-                         XEN_DOMCTL_CDF_hap : 0);
-        config.emulation_flags = XEN_X86_EMU_LAPIC|XEN_X86_EMU_IOAPIC;
+        dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm_guest |
+                           ((hvm_funcs.hap_supported && !opt_dom0_shadow) ?
+                            XEN_DOMCTL_CDF_hap : 0));
+
+        dom0_cfg.config.emulation_flags =
+            XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC;
      }
/* Create initial domain 0. */
-    dom0 = domain_create(get_initial_domain_id(), domcr_flags, 0, &config);
+    dom0 = domain_create(get_initial_domain_id(), &dom0_cfg);
      if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
          panic("Error creating domain 0");
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 582e3e5..b00cc1f 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -260,9 +260,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, unsigned int domcr_flags,
-                             uint32_t ssidref,
-                             struct xen_arch_domainconfig *config)
+struct domain *domain_create(domid_t domid,
+                             struct xen_domctl_createdomain *config)
  {
      struct domain *d, **pd, *old_hwdom = NULL;
      enum { INIT_xsm = 1u<<0, INIT_watchdog = 1u<<1, INIT_rangeset = 1u<<2,
@@ -274,6 +273,9 @@ struct domain *domain_create(domid_t domid, unsigned int 
domcr_flags,
d->domain_id = domid; + /* Debug sanity. */
+    ASSERT(is_system_domain(d) ? config == NULL : config != NULL);
+
      TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
lock_profile_register_struct(LOCKPROF_TYPE_PERDOM, d, domid, "Domain");
@@ -304,11 +306,6 @@ struct domain *domain_create(domid_t domid, unsigned int 
domcr_flags,
      if ( !zalloc_cpumask_var(&d->dirty_cpumask) )
          goto fail;
- if ( domcr_flags & XEN_DOMCTL_CDF_hvm_guest )
-        d->guest_type = guest_type_hvm;
-    else
-        d->guest_type = guest_type_pv;
-
      rangeset_domain_initialise(d);
      init_status |= INIT_rangeset;
@@ -318,6 +315,11 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags, if ( !is_idle_domain(d) )
      {
+        if ( config->flags & XEN_DOMCTL_CDF_hvm_guest )
+            d->guest_type = guest_type_hvm;
+        else
+            d->guest_type = guest_type_pv;
+
          watchdog_domain_init(d);
          init_status |= INIT_watchdog;
@@ -331,7 +333,7 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
              hardware_domain = d;
          }
- if ( domcr_flags & XEN_DOMCTL_CDF_xs_domain )
+        if ( config->flags & XEN_DOMCTL_CDF_xs_domain )
          {
              d->is_xenstore = 1;
              d->disable_migrate = 1;
@@ -342,7 +344,7 @@ struct domain *domain_create(domid_t domid, unsigned int 
domcr_flags,
          if ( !d->iomem_caps || !d->irq_caps )
              goto fail;
- if ( (err = xsm_domain_create(XSM_HOOK, d, ssidref)) != 0 )
+        if ( (err = xsm_domain_create(XSM_HOOK, d, config->ssidref)) != 0 )
              goto fail;
d->controller_pause_count = 1;
@@ -373,7 +375,7 @@ struct domain *domain_create(domid_t domid, unsigned int 
domcr_flags,
              goto fail;
      }
- if ( (err = arch_domain_create(d, domcr_flags, config)) != 0 )
+    if ( (err = arch_domain_create(d, config)) != 0 )
          goto fail;
      init_status |= INIT_arch;
@@ -385,6 +387,11 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
          if ( (err = late_hwdom_init(d)) != 0 )
              goto fail;
+ /*
+         * Must not fail beyond this point, as our caller doesn't know whether
+         * the domain has been entered into domain_list or not.
+         */
+
          spin_lock(&domlist_update_lock);
          pd = &domain_list; /* NB. domain_list maintained in order of domid. */
          for ( pd = &domain_list; *pd != NULL; pd = &(*pd)->next_in_list )
@@ -395,6 +402,8 @@ struct domain *domain_create(domid_t domid, unsigned int 
domcr_flags,
          rcu_assign_pointer(*pd, d);
          rcu_assign_pointer(domain_hash[DOMAIN_HASH(domid)], d);
          spin_unlock(&domlist_update_lock);
+
+        memcpy(d->handle, config->handle, sizeof(d->handle));
      }
return d;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index a73e1a4..9b7bc08 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -532,9 +532,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
              rover = dom;
          }
- d = domain_create(dom, op->u.createdomain.flags,
-                          op->u.createdomain.ssidref,
-                          &op->u.createdomain.config);
+        d = domain_create(dom, &op->u.createdomain);
          if ( IS_ERR(d) )
          {
              ret = PTR_ERR(d);
@@ -543,10 +541,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
          }
ret = 0;
-
-        memcpy(d->handle, op->u.createdomain.handle,
-               sizeof(xen_domain_handle_t));
-
          op->domain = d->domain_id;
          copyback = 1;
          d = NULL;
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 64524f4..b3c2660 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1734,7 +1734,7 @@ void __init scheduler_init(void)
          sched_ratelimit_us = SCHED_DEFAULT_RATELIMIT_US;
      }
- idle_domain = domain_create(DOMID_IDLE, 0, 0, NULL);
+    idle_domain = domain_create(DOMID_IDLE, NULL);
      BUG_ON(IS_ERR(idle_domain));
      idle_domain->vcpu = idle_vcpu;
      idle_domain->max_vcpus = nr_cpu_ids;
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index eb62f1d..177cb35 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -55,8 +55,8 @@ void vcpu_destroy(struct vcpu *v);
  int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
  void unmap_vcpu_info(struct vcpu *v);
-int arch_domain_create(struct domain *d, unsigned int domcr_flags,
-                       struct xen_arch_domainconfig *config);
+int arch_domain_create(struct domain *d,
+                       struct xen_domctl_createdomain *config);
void arch_domain_destroy(struct domain *d); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 9af78ac..c15c39e 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -539,9 +539,8 @@ void domain_update_node_affinity(struct domain *d);
   * Create a domain: the configuration is only necessary for real domain
   * (domid < DOMID_FIRST_RESERVED).
   */
-struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
-                             uint32_t ssidref,
-                             struct xen_arch_domainconfig *config);
+struct domain *domain_create(domid_t domid,
+                             struct xen_domctl_createdomain *config);
/*
   * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().


--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.