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

Re: [Xen-devel] [PATCH v2 08/21] xen/arm: Initialize the virtual GIC later



On Thu, 31 Jul 2014, Julien Grall wrote:
> The virtual GIC may differ between each guest (number of SPIs, emulate GIC
> version...). Those informations may not be know when the domain is created
> (for instance in case of migration). Therefore, move the VGIC initialization
> in a separate function.
> 
> Introduce a new DOMCTL for ARM to configure the domain. This has to be called
> before setting the maximum number of VCPUs for the guest.
> 
> For the moment, only configure the number SPIs. New informations could be
> added later.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> 
> ---
>     Changes in v2:
>         - Patch added
> ---
>  tools/libxc/xc_domain.c             |   12 ++++++++++++
>  tools/libxc/xenctrl.h               |    4 ++++
>  tools/libxl/libxl_arch.h            |    3 +++
>  tools/libxl/libxl_arm.c             |   19 +++++++++++++++++++
>  tools/libxl/libxl_dom.c             |    4 ++++
>  tools/libxl/libxl_x86.c             |    7 +++++++
>  xen/arch/arm/domain.c               |   28 ++++++++++++++++++++++------
>  xen/arch/arm/domctl.c               |   11 +++++++++++
>  xen/arch/arm/setup.c                |   10 ++++++++--
>  xen/arch/arm/vgic.c                 |   10 +++++-----
>  xen/include/asm-arm/domain.h        |    6 ++++++
>  xen/include/asm-arm/vgic.h          |    4 +++-
>  xen/include/public/domctl.h         |   14 ++++++++++++++
>  xen/xsm/flask/hooks.c               |    3 +++
>  xen/xsm/flask/policy/access_vectors |    2 ++
>  15 files changed, 123 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 27fe3b6..1348905 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -48,6 +48,18 @@ int xc_domain_create(xc_interface *xch,
>      return 0;
>  }
>  
> +#if defined(__arm__) || defined(__arch64__)
> +int xc_domain_configure(xc_interface *xch, uint32_t domid,
> +                        uint32_t nr_spis)

Given that we'll likely add new fields to xen_domctl_configuredomain, I
think it is best if we pass a struct domain_configure to
xc_domain_configure instead of nr_spis. The struct we pass to
xc_domain_configure doesn't have to be identical to struct
xen_domctl_configuredomain, but at the moment it would be.


> +{
> +    DECLARE_DOMCTL;
> +    domctl.cmd = XEN_DOMCTL_configure_domain;
> +    domctl.domain = (domid_t)domid;
> +    domctl.u.configuredomain.nr_spis = nr_spis;
> +    return do_domctl(xch, &domctl);
> +}
> +#endif
> +
>  int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
>                           xen_pfn_t start_pfn, xen_pfn_t nr_pfns)
>  {
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 5beb846..cdda4e5 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -482,6 +482,10 @@ int xc_domain_create(xc_interface *xch,
>                       uint32_t flags,
>                       uint32_t *pdomid);
>  
> +#if defined(__arm__) || defined(__aarch64__)
> +int xc_domain_configure(xc_interface *xch, uint32_t domid,
> +                        uint32_t nr_spis);
> +#endif
>  
>  /* Functions to produce a dump of a given domain
>   *  xc_domain_dumpcore - produces a dump to a specified file
> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
> index d3bc136..454f8db 100644
> --- a/tools/libxl/libxl_arch.h
> +++ b/tools/libxl/libxl_arch.h
> @@ -16,6 +16,9 @@
>  #define LIBXL_ARCH_H
>  
>  /* arch specific internal domain creation function */
> +int libxl__arch_domain_create_pre(libxl__gc *gc, libxl_domain_config 
> *d_config,
> +                                  libxl__domain_build_state *state,
> +                                  uint32_t domid);
>  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
>                 uint32_t domid);
>  
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index e19e2f4..b0491c3 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -23,6 +23,25 @@
>  #define DT_IRQ_TYPE_LEVEL_HIGH     0x00000004
>  #define DT_IRQ_TYPE_LEVEL_LOW      0x00000008
>  
> +int libxl__arch_domain_create_pre(libxl__gc *gc, libxl_domain_config 
> *d_config,
> +                                  libxl__domain_build_state *state,
> +                                  uint32_t domid)
> +{
> +    uint32_t nr_spis = 0;
> +
> +    nr_spis += d_config->b_info.num_irqs;
> +
> +    LOG(DEBUG, "Allocate %u SPIs\n", nr_spis);
> +
> +    if (xc_domain_configure(CTX->xch, domid, nr_spis) != 0) {
> +        LOG(ERROR, "Couldn't configure the domain");
> +        return ERROR_FAIL;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
>                                uint32_t domid)
>  {
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index c944804..a0e4e34 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -235,6 +235,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>      char *xs_domid, *con_domid;
>      int rc;
>  
> +    rc = libxl__arch_domain_create_pre(gc, d_config, state, domid);
> +    if (rc != 0)
> +        return rc;
> +
>      if (xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus) != 0) {
>          LOG(ERROR, "Couldn't set max vcpu count");
>          return ERROR_FAIL;
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index 7589060..0abc1aa 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -244,6 +244,13 @@ static int libxl__e820_alloc(libxl__gc *gc, uint32_t 
> domid,
>      return 0;
>  }
>  
> +int libxl__arch_domain_create_pre(libxl__gc *gc, libxl_domain_config 
> *d_config,
> +                                  libxl__domain_build_state *state,
> +                                  uint32_t domid)
> +{
> +    return 0;
> +}
> +
>  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
>          uint32_t domid)
>  {
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index aa4a8d7..fc97f8c 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -472,6 +472,13 @@ int vcpu_initialise(struct vcpu *v)
>      if ( is_idle_vcpu(v) )
>          return rc;
>  
> +    /* We can't initialize the VCPU if the VGIC has not been correctly
> +     * initialized.
> +     */
> +    rc = -ENXIO;
> +    if ( !domain_vgic_is_initialized(v->domain) )
> +        goto fail;
> +
>      v->arch.sctlr = SCTLR_GUEST_INIT;
>  
>      /*
> @@ -534,12 +541,6 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags)
>      if ( (rc = p2m_alloc_table(d)) != 0 )
>          goto fail;
>  
> -    if ( (rc = gicv_setup(d)) != 0 )
> -        goto fail;
> -
> -    if ( (rc = domain_vgic_init(d)) != 0 )
> -        goto fail;
> -
>      if ( (rc = domain_vtimer_init(d)) != 0 )
>          goto fail;
>  
> @@ -580,6 +581,21 @@ void arch_domain_destroy(struct domain *d)
>      free_xenheap_page(d->shared_info);
>  }
>  
> +int domain_configure_vgic(struct domain *d, unsigned int nr_spis)
> +{
> +    int rc;
> +
> +    if ( (rc = gicv_setup(d)) != 0 )
> +        return rc;
> +
> +    if ( (rc = domain_vgic_init(d, nr_spis)) != 0 )
> +        return rc;
> +
> +    d->arch.vgic.initialized = 1;
> +
> +    return 0;
> +}
> +
>  static int is_guest_pv32_psr(uint32_t psr)
>  {
>      switch (psr & PSR_MODE_MASK)
> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> index 45974e7..bab92b2 100644
> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -30,6 +30,17 @@ long arch_do_domctl(struct xen_domctl *domctl, struct 
> domain *d,
>  
>          return p2m_cache_flush(d, s, e);
>      }
> +    case XEN_DOMCTL_configure_domain:
> +    {
> +        if ( domain_vgic_is_initialized(d) )
> +            return -EBUSY;

Given that XEN_DOMCTL_configure_domain should be called exactly once at
domain creation, instead of introducing domain_vgic_is_initialized, I
would make sure that XEN_DOMCTL_configure_domain hasn't been called for
this domain before. In other words, I would make this check more
generic, rather than vgic specific.


> +        /* Sanity check on the number of SPIs */
> +        if ( domctl->u.configuredomain.nr_spis > (gic_number_lines() - 32) )
> +            return -EINVAL;
> +
> +        return domain_configure_vgic(d, domctl->u.configuredomain.nr_spis);
> +    }
>  
>      default:
>          return subarch_do_domctl(domctl, d, u_domctl);
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 446b4dc..2be7dd1 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -840,8 +840,14 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>      /* Create initial domain 0. */
>      dom0 = domain_create(0, 0, 0);
> -    if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
> -            panic("Error creating domain 0");
> +    if ( IS_ERR(dom0) )
> +        panic("Error creating domain 0");
> +
> +    if ( domain_configure_vgic(dom0, gic_number_lines() - 32) )
> +         panic("Error configure the vgic for domain 0");
> +
> +    if ( alloc_dom0_vcpu0(dom0) == NULL )
> +        panic("Error create vcpu0 for domain 0");
>  
>      dom0->is_privileged = 1;
>      dom0->target = NULL;
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 17cde7a..a037ecc 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -60,16 +60,16 @@ static void vgic_init_pending_irq(struct pending_irq *p, 
> unsigned virq)
>      p->irq = virq;
>  }
>  
> -int domain_vgic_init(struct domain *d)
> +int domain_vgic_init(struct domain *d, unsigned int nr_spis)
>  {
>      int i;
>  
>      d->arch.vgic.ctlr = 0;
>  
> -    if ( is_hardware_domain(d) )
> -        d->arch.vgic.nr_spis = gic_number_lines() - 32;
> -    else
> -        d->arch.vgic.nr_spis = 0; /* We don't need SPIs for the guest */
> +    /* The number of SPIs as to be aligned to 32 see
> +     * GICD_TYPER.ITLinesNumber definition
> +     */
> +    d->arch.vgic.nr_spis = ROUNDUP(nr_spis, 32);
>  
>      switch ( gic_hw_version() )
>      {
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 5719fe5..44727b2 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -76,6 +76,10 @@ struct arch_domain
>      } virt_timer_base;
>  
>      struct {
> +        /* The VGIC initialization is defered to let the toolstack
> +         * configure the emulated GIC (such as number of SPIs, version...)
> +         */
> +        bool_t initialized;
>          /* GIC HW version specific vGIC driver handler */
>          const struct vgic_ops *handler;
>          /*
> @@ -241,6 +245,8 @@ struct arch_vcpu
>  void vcpu_show_execution_state(struct vcpu *);
>  void vcpu_show_registers(const struct vcpu *);
>  
> +int domain_configure_vgic(struct domain *d, unsigned int spis_number);
> +
>  #endif /* __ASM_DOMAIN_H__ */
>  
>  /*
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 5ddc681..84ae441 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -148,6 +148,8 @@ static inline void vgic_byte_write(uint32_t *reg, 
> uint32_t var, int offset)
>      *reg |= var;
>  }
>  
> +#define domain_vgic_is_initialized(d) ((d)->arch.vgic.initialized)
> +
>  enum gic_sgi_mode;
>  
>  /*
> @@ -158,7 +160,7 @@ enum gic_sgi_mode;
>  
>  #define vgic_num_irqs(d)        ((d)->arch.vgic.nr_spis + 32)
>  
> -extern int domain_vgic_init(struct domain *d);
> +extern int domain_vgic_init(struct domain *d, unsigned int nr_spis);
>  extern void domain_vgic_free(struct domain *d);
>  extern int vcpu_vgic_init(struct vcpu *v);
>  extern struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq);
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 5b11bbf..b5f2ed7 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -67,6 +67,16 @@ struct xen_domctl_createdomain {
>  typedef struct xen_domctl_createdomain xen_domctl_createdomain_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_createdomain_t);
>  
> +#if defined(__arm__) || defined(__aarch64__)
> +/* XEN_DOMCTL_configure_domain */
> +struct xen_domctl_configuredomain {
> +    /* IN parameters */
> +    uint32_t nr_spis;
> +};
> +typedef struct xen_domctl_configuredomain xen_domctl_configuredomain_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_configuredomain_t);
> +#endif
> +
>  /* XEN_DOMCTL_getdomaininfo */
>  struct xen_domctl_getdomaininfo {
>      /* OUT variables. */
> @@ -1008,6 +1018,7 @@ struct xen_domctl {
>  #define XEN_DOMCTL_cacheflush                    71
>  #define XEN_DOMCTL_get_vcpu_msrs                 72
>  #define XEN_DOMCTL_set_vcpu_msrs                 73
> +#define XEN_DOMCTL_configure_domain              74
>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -1016,6 +1027,9 @@ struct xen_domctl {
>      domid_t  domain;
>      union {
>          struct xen_domctl_createdomain      createdomain;
> +#if defined(__arm__) || defined(__aarch64__)
> +        struct xen_domctl_configuredomain   configuredomain;
> +#endif
>          struct xen_domctl_getdomaininfo     getdomaininfo;
>          struct xen_domctl_getmemlist        getmemlist;
>          struct xen_domctl_getpageframeinfo  getpageframeinfo;
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index f2f59ea..4759461 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -715,6 +715,9 @@ static int flask_domctl(struct domain *d, int cmd)
>      case XEN_DOMCTL_cacheflush:
>          return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__CACHEFLUSH);
>  
> +    case XEN_DOMCTL_configure_domain:
> +        return current_has_perm(d, SECCLASS_DOMAIN2, 
> DOMAIN2__CONFIGURE_DOMAIN);
> +
>      default:
>          printk("flask_domctl: Unknown op %d\n", cmd);
>          return -EPERM;
> diff --git a/xen/xsm/flask/policy/access_vectors 
> b/xen/xsm/flask/policy/access_vectors
> index 32371a9..33eec66 100644
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -200,6 +200,8 @@ class domain2
>      cacheflush
>  # Creation of the hardware domain when it is not dom0
>      create_hardware_domain
> +# XEN_DOMCTL_configure_domain
> +    configure_domain
>  }
>  
>  # Similar to class domain, but primarily contains domctls related to HVM 
> domains
> -- 
> 1.7.10.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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