| Message ID | 1402935486-29136-5-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Mon, 16 Jun 2014, Julien Grall wrote: > Currently Xen only supports SPIs routing for guest, add a function > is_routable_irq to check if we can route a given IRQ to the guest. > > Secondly, make sure the vIRQ (which is currently the same as the pIRQ) is not > the greater that the number of IRQs handle to the vGIC. > > Finally, desc->arch.type which contains the IRQ type (i.e level/edge) must > be correctly configured before. The IRQ type won't be configure when: > - the device has been blacklist for the current platform > - the IRQ has not been describe in the device tree > > I think we can safely assume that a user won't never ask to route > as such IRQ to the guest. > > Also, use XENLOG_G_ERR in the error message within the function as it will > be later called from a guest. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > xen/arch/arm/irq.c | 32 +++++++++++++++++++++++++++++--- > xen/include/asm-arm/gic.h | 2 ++ > xen/include/asm-arm/irq.h | 6 ++++++ > 3 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c > index 9c141bc..4e51fee 100644 > --- a/xen/arch/arm/irq.c > +++ b/xen/arch/arm/irq.c > @@ -361,6 +361,10 @@ err: > return rc; > } > > +/* Route an IRQ to a specific guest. > + * For now the vIRQ is equal to the pIRQ and only SPIs are routabled to > + * the guest. > + */ > int route_irq_to_guest(struct domain *d, unsigned int irq, > const char * devname) > { > @@ -369,6 +373,20 @@ int route_irq_to_guest(struct domain *d, unsigned int irq, > unsigned long flags; > int retval = 0; > > + if ( !is_routable_irq(irq) ) > + { > + dprintk(XENLOG_G_ERR, "the IRQ%u is not routable\n", irq); > + return -EINVAL; > + } > + > + if ( irq > vgic_num_irqs(d) ) > + { > + dprintk(XENLOG_G_ERR, > + "the IRQ number %u is too high for domain %u (max = %u)\n", > + irq, d->domain_id, vgic_num_irqs(d)); > + return -EINVAL; > + } I think it makes sense to move the "irq > vgic_num_irqs(d)" check within is_routable_irq. > action = xmalloc(struct irqaction); > if (!action) > return -ENOMEM; > @@ -379,6 +397,14 @@ int route_irq_to_guest(struct domain *d, unsigned int irq, > > spin_lock_irqsave(&desc->lock, flags); > > + if ( desc->arch.type == DT_IRQ_TYPE_INVALID ) > + { > + dprintk(XENLOG_G_ERR, "IRQ %u has not been configured\n", > + irq); > + retval = -EIO; > + goto out; > + } > + > /* If the IRQ is already used by someone > * - If it's the same domain -> Xen doesn't need to update the IRQ desc > * - Otherwise -> For now, don't allow the IRQ to be shared between > @@ -392,10 +418,10 @@ int route_irq_to_guest(struct domain *d, unsigned int irq, > goto out; > > if ( desc->status & IRQ_GUEST ) > - printk(XENLOG_ERR "ERROR: IRQ %u is already used by domain %u\n", > - irq, ad->domain_id); > + dprintk(XENLOG_G_ERR, "IRQ %u is already used by domain %u\n", > + irq, ad->domain_id); > else > - printk(XENLOG_ERR "ERROR: IRQ %u is already used by Xen\n", irq); > + dprintk(XENLOG_G_ERR, "IRQ %u is already used by Xen\n", irq); > retval = -EBUSY; > goto out; > } > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 8e37ccf..6e7375c 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -163,6 +163,8 @@ > #define DT_MATCH_GIC DT_MATCH_COMPATIBLE("arm,cortex-a15-gic"), \ > DT_MATCH_COMPATIBLE("arm,cortex-a7-gic") > > +#define vgic_num_irqs(d) ((d)->arch.vgic.nr_lines + 32) > + > extern int domain_vgic_init(struct domain *d); > extern void domain_vgic_free(struct domain *d); > > diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h > index e567f71..63926a5 100644 > --- a/xen/include/asm-arm/irq.h > +++ b/xen/include/asm-arm/irq.h > @@ -37,6 +37,12 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq); > > #define domain_pirq_to_irq(d, pirq) (pirq) > > +static inline bool_t is_routable_irq(unsigned int irq) > +{ > + /* For now, we can only route SPIs to the guest */ > + return (irq >= NR_LOCAL_IRQS); > +} > + > void init_IRQ(void); > void init_secondary_IRQ(void); > > -- > 1.7.10.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
Hi Stefano, On 18/06/14 19:52, Stefano Stabellini wrote: >> +/* Route an IRQ to a specific guest. >> + * For now the vIRQ is equal to the pIRQ and only SPIs are routabled to >> + * the guest. >> + */ >> int route_irq_to_guest(struct domain *d, unsigned int irq, >> const char * devname) >> { >> @@ -369,6 +373,20 @@ int route_irq_to_guest(struct domain *d, unsigned int irq, >> unsigned long flags; >> int retval = 0; >> >> + if ( !is_routable_irq(irq) ) >> + { >> + dprintk(XENLOG_G_ERR, "the IRQ%u is not routable\n", irq); >> + return -EINVAL; >> + } >> + >> + if ( irq > vgic_num_irqs(d) ) >> + { >> + dprintk(XENLOG_G_ERR, >> + "the IRQ number %u is too high for domain %u (max = %u)\n", >> + irq, d->domain_id, vgic_num_irqs(d)); >> + return -EINVAL; >> + } > > I think it makes sense to move the "irq > vgic_num_irqs(d)" check > within is_routable_irq. is_routable_irq checks that Xen is effectively able to route the IRQ to a guest, rather than the check "irq > vgic_num_irqs(d)" is here because we assume a virq == pirq. I suspect we will have to handle virq != pirq sooner or later because allocate 1000 irq_pending structure unconditionally per guest is a waste of memory. Furthermore, I will use it in different place is_routable_irq (see patch #6, #9) where we don't necessary have the domain in hand. Regards,
On Mon, 2014-06-16 at 17:17 +0100, Julien Grall wrote: > diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h > index e567f71..63926a5 100644 > --- a/xen/include/asm-arm/irq.h > +++ b/xen/include/asm-arm/irq.h > @@ -37,6 +37,12 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq); > > #define domain_pirq_to_irq(d, pirq) (pirq) > > +static inline bool_t is_routable_irq(unsigned int irq) is_assignable_irq I think better suits your intention. routable doesn't imply "to the guest" since you might also route it to Xen. Ian.
On 07/03/2014 12:04 PM, Ian Campbell wrote: > On Mon, 2014-06-16 at 17:17 +0100, Julien Grall wrote: >> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h >> index e567f71..63926a5 100644 >> --- a/xen/include/asm-arm/irq.h >> +++ b/xen/include/asm-arm/irq.h >> @@ -37,6 +37,12 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq); >> >> #define domain_pirq_to_irq(d, pirq) (pirq) >> >> +static inline bool_t is_routable_irq(unsigned int irq) > > is_assignable_irq I think better suits your intention. > > routable doesn't imply "to the guest" since you might also route it to > Xen. Hmmm... right. I will do the change in the next version. Regards,
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c index 9c141bc..4e51fee 100644 --- a/xen/arch/arm/irq.c +++ b/xen/arch/arm/irq.c @@ -361,6 +361,10 @@ err: return rc; } +/* Route an IRQ to a specific guest. + * For now the vIRQ is equal to the pIRQ and only SPIs are routabled to + * the guest. + */ int route_irq_to_guest(struct domain *d, unsigned int irq, const char * devname) { @@ -369,6 +373,20 @@ int route_irq_to_guest(struct domain *d, unsigned int irq, unsigned long flags; int retval = 0; + if ( !is_routable_irq(irq) ) + { + dprintk(XENLOG_G_ERR, "the IRQ%u is not routable\n", irq); + return -EINVAL; + } + + if ( irq > vgic_num_irqs(d) ) + { + dprintk(XENLOG_G_ERR, + "the IRQ number %u is too high for domain %u (max = %u)\n", + irq, d->domain_id, vgic_num_irqs(d)); + return -EINVAL; + } + action = xmalloc(struct irqaction); if (!action) return -ENOMEM; @@ -379,6 +397,14 @@ int route_irq_to_guest(struct domain *d, unsigned int irq, spin_lock_irqsave(&desc->lock, flags); + if ( desc->arch.type == DT_IRQ_TYPE_INVALID ) + { + dprintk(XENLOG_G_ERR, "IRQ %u has not been configured\n", + irq); + retval = -EIO; + goto out; + } + /* If the IRQ is already used by someone * - If it's the same domain -> Xen doesn't need to update the IRQ desc * - Otherwise -> For now, don't allow the IRQ to be shared between @@ -392,10 +418,10 @@ int route_irq_to_guest(struct domain *d, unsigned int irq, goto out; if ( desc->status & IRQ_GUEST ) - printk(XENLOG_ERR "ERROR: IRQ %u is already used by domain %u\n", - irq, ad->domain_id); + dprintk(XENLOG_G_ERR, "IRQ %u is already used by domain %u\n", + irq, ad->domain_id); else - printk(XENLOG_ERR "ERROR: IRQ %u is already used by Xen\n", irq); + dprintk(XENLOG_G_ERR, "IRQ %u is already used by Xen\n", irq); retval = -EBUSY; goto out; } diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index 8e37ccf..6e7375c 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -163,6 +163,8 @@ #define DT_MATCH_GIC DT_MATCH_COMPATIBLE("arm,cortex-a15-gic"), \ DT_MATCH_COMPATIBLE("arm,cortex-a7-gic") +#define vgic_num_irqs(d) ((d)->arch.vgic.nr_lines + 32) + extern int domain_vgic_init(struct domain *d); extern void domain_vgic_free(struct domain *d); diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h index e567f71..63926a5 100644 --- a/xen/include/asm-arm/irq.h +++ b/xen/include/asm-arm/irq.h @@ -37,6 +37,12 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq); #define domain_pirq_to_irq(d, pirq) (pirq) +static inline bool_t is_routable_irq(unsigned int irq) +{ + /* For now, we can only route SPIs to the guest */ + return (irq >= NR_LOCAL_IRQS); +} + void init_IRQ(void); void init_secondary_IRQ(void);
Currently Xen only supports SPIs routing for guest, add a function is_routable_irq to check if we can route a given IRQ to the guest. Secondly, make sure the vIRQ (which is currently the same as the pIRQ) is not the greater that the number of IRQs handle to the vGIC. Finally, desc->arch.type which contains the IRQ type (i.e level/edge) must be correctly configured before. The IRQ type won't be configure when: - the device has been blacklist for the current platform - the IRQ has not been describe in the device tree I think we can safely assume that a user won't never ask to route as such IRQ to the guest. Also, use XENLOG_G_ERR in the error message within the function as it will be later called from a guest. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/irq.c | 32 +++++++++++++++++++++++++++++--- xen/include/asm-arm/gic.h | 2 ++ xen/include/asm-arm/irq.h | 6 ++++++ 3 files changed, 37 insertions(+), 3 deletions(-)