|
[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |