| Message ID | 1426793399-6283-17-git-send-email-julien.grall@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Ian, On 31/03/15 11:54, Ian Campbell wrote: > On Thu, 2015-03-19 at 19:29 +0000, Julien Grall wrote: >> @@ -68,16 +68,17 @@ static void vgic_init_pending_irq(struct pending_irq *p, unsigned int 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; >> > [...] >> + /* Limit the number of SPIs supported base on the hardware */ >> + if ( nr_spis > (gic_number_lines() - NR_LOCAL_IRQS) ) >> + return -EINVAL; > > Is there anything in the h/w which leads to this restriction? The h/w accept an VirtualID < 1020. Although, given that we can only route a physical interrupt to the guest, it's pointless to support more SPIs than the h/w does. > If not then I think we should just check against the architectural 1020 > limit and leave it at that. I would have to check on 988 (1020 - 32) which I find less readable. >> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h >> index 9e0419e..6dacfef 100644 >> --- a/xen/include/asm-arm/domain.h >> +++ b/xen/include/asm-arm/domain.h >> @@ -125,6 +125,8 @@ struct arch_domain >> unsigned int evtchn_irq; >> } __cacheline_aligned; >> >> +#define domain_is_configured(d) ((d)->arch.is_configured) >> + >> struct arch_vcpu >> { >> struct { >> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h >> index ba5a67d..254cc17 100644 >> --- a/xen/include/asm-arm/setup.h >> +++ b/xen/include/asm-arm/setup.h >> @@ -54,6 +54,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len); >> void arch_get_xen_caps(xen_capabilities_info_t *info); >> >> int construct_dom0(struct domain *d); >> +int configure_dom0(struct domain *d); > > Are these two hunks stray from some other patch in the series? No. It's part of an old version of this patch and I forgot to drop it. I will resend a new version. Regards,
On 31/03/15 12:59, Ian Campbell wrote: >>> If not then I think we should just check against the architectural 1020 >>> limit and leave it at that. >> >> I would have to check on 988 (1020 - 32) which I find less readable. > > I'd test nr_spis+32 < 1020, which I think is clear enough. I will do that. Regards,
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index 448e958..579d266 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -67,6 +67,7 @@ int xc_domain_create(xc_interface *xch, /* No arch-specific configuration for now */ #elif defined (__arm__) || defined(__aarch64__) config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; + config.nr_spis = 0; #else errno = ENOSYS; return -1; diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index dc0919a..2407c2e 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -39,6 +39,27 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, libxl_domain_config *d_config, xc_domain_configuration_t *xc_config) { + uint32_t nr_spis = 0; + unsigned int i; + + for (i = 0; i < d_config->b_info.num_irqs; i++) { + uint32_t irq = d_config->b_info.irqs[i]; + uint32_t spi; + + if (irq < 32) + continue; + + spi = irq - 32; + + if (nr_spis <= spi) + nr_spis = spi + 1; + } + + LOG(DEBUG, "Configure the domain"); + + xc_config->nr_spis = nr_spis; + LOG(DEBUG, " - Allocate %u SPIs", nr_spis); + xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; return 0; diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index ce9f349..7c5bf9f 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -562,7 +562,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags, if ( (rc = gicv_setup(d)) != 0 ) goto fail; - if ( (rc = domain_vgic_init(d)) != 0 ) + if ( (rc = domain_vgic_init(d, config->nr_spis)) != 0 ) goto fail; if ( (rc = domain_vtimer_init(d)) != 0 ) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index defe39e..c935266 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -830,6 +830,7 @@ void __init start_xen(unsigned long boot_phys_offset, /* Create initial domain 0. */ /* The vGIC for DOM0 is exactly emulated the hardware GIC */ config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; + config.nr_spis = gic_number_lines() - 32; dom0 = domain_create(0, 0, 0, &config); if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index c07822f..74751e0 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -68,16 +68,17 @@ static void vgic_init_pending_irq(struct pending_irq *p, unsigned int 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 */ + /* Limit the number of SPIs supported base on the hardware */ + if ( nr_spis > (gic_number_lines() - NR_LOCAL_IRQS) ) + return -EINVAL; + + d->arch.vgic.nr_spis = nr_spis; switch ( gic_hw_version() ) { diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 9e0419e..6dacfef 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -125,6 +125,8 @@ struct arch_domain unsigned int evtchn_irq; } __cacheline_aligned; +#define domain_is_configured(d) ((d)->arch.is_configured) + struct arch_vcpu { struct { diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index ba5a67d..254cc17 100644 --- a/xen/include/asm-arm/setup.h +++ b/xen/include/asm-arm/setup.h @@ -54,6 +54,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len); void arch_get_xen_caps(xen_capabilities_info_t *info); int construct_dom0(struct domain *d); +int configure_dom0(struct domain *d); void discard_initial_modules(void); diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h index aba0d80..647f2fe 100644 --- a/xen/include/asm-arm/vgic.h +++ b/xen/include/asm-arm/vgic.h @@ -177,7 +177,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/arch-arm.h b/xen/include/public/arch-arm.h index ed7e98f..c029e0f 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -315,6 +315,8 @@ typedef uint64_t xen_callback_t; struct xen_arch_domainconfig { /* IN/OUT */ uint8_t gic_version; + /* IN */ + uint32_t nr_spis; }; #endif
Each domain may have a different number of IRQs depending on the devices assigned to it. Rather re-using the number of IRQs used by the hardwared GIC, let the toolstack specify the number of SPIs when the domain is created. This will avoid to waste memory. To calculate the number of SPIs, we take advantage of the fact that the libxl interface can only expose 1:1 mapping and look for the largest SPI in the list. Signed-off-by: Julien Grall <julien.grall@linaro.org> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Wei Liu <wei.liu2@citrix.com> --- Changes in v4: - Check the number of SPIs supported by the virtual GIC against the number supported by the hardware GIC. - Use uint32_t rather than int in the toolstack code. - Initialize spi after the check in the toolstack code - Typoes Changes in v3: - Fix typoes - A separate has been created to extend the DOMCTL create domain Changes in v2: - Patch added --- tools/libxc/xc_domain.c | 1 + tools/libxl/libxl_arm.c | 21 +++++++++++++++++++++ xen/arch/arm/domain.c | 2 +- xen/arch/arm/setup.c | 1 + xen/arch/arm/vgic.c | 11 ++++++----- xen/include/asm-arm/domain.h | 2 ++ xen/include/asm-arm/setup.h | 1 + xen/include/asm-arm/vgic.h | 2 +- xen/include/public/arch-arm.h | 2 ++ 9 files changed, 36 insertions(+), 7 deletions(-)