| Message ID | 1396968247-8768-13-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: > The current dt_route_irq_to_guest implementation sets IRQ_GUEST even if the > IRQ is correctly setup. > > An IRQ can be shared between devices, if the devices are not assigned to the > same domain or Xen, then this could result in routing the IRQ to the domain > instead of Xen ... > > Also avoid to relying on wrong the behaviour when Xen is routing an IRQ to > DOM0. Therefore check the return code from route_dt_irq_to_guest in > map_device. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Hi Ian, On 08/04/14 15:44, Julien Grall wrote: > + /* 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 > + * Xen and domains. > + */ > + if ( desc->action != NULL ) > + { > + struct domain *ad = irq_get_domain(desc); > + > + if ( (desc->status & IRQ_GUEST) && d == ad ) > + goto out; > + > + if ( desc->status & IRQ_GUEST ) > + printk(XENLOG_ERR "ERROR: IRQ %u is already used by domain %u\n", > + irq->irq, ad->domain_id); > + else > + printk(XENLOG_ERR "ERROR: IRQ %u is already used by Xen\n", > + irq->irq); > + retval = -EBUSY; > + goto out; This code is buggy, I forgot to free the action in this case. I will fix it in the next version. Ian: As said in the cover letter, I will only resend a V4 with all patches from this one (i.e #12 and onwards) as the other are already acked. Let me know if I need to resend #1-#11. Regards,
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index e7b1674..f85e5a9 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -718,8 +718,13 @@ static int map_device(struct domain *d, const struct dt_device_node *dev) } DPRINT("irq %u = %u type = 0x%x\n", i, irq.irq, irq.type); - /* Don't check return because the IRQ can be use by multiple device */ - route_dt_irq_to_guest(d, &irq, dt_node_name(dev)); + res = route_dt_irq_to_guest(d, &irq, dt_node_name(dev)); + if ( res ) + { + printk(XENLOG_ERR "Unable to route IRQ %u to domain %u\n", + irq.irq, d->domain_id); + return res; + } } /* Map the address ranges */ diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c index aac39b0..ae92919 100644 --- a/xen/arch/arm/irq.c +++ b/xen/arch/arm/irq.c @@ -253,6 +253,16 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new) spin_lock_irqsave(&desc->lock, flags); + if ( desc->status & IRQ_GUEST ) + { + struct domain *d = irq_get_domain(desc); + + spin_unlock_irqrestore(&desc->lock, flags); + printk(XENLOG_ERR "ERROR: IRQ %u is already in use by the domain %u\n", + irq->irq, d->domain_id); + return -EBUSY; + } + disabled = (desc->action == NULL); rc = __setup_irq(desc, new); @@ -289,7 +299,7 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq, struct irqaction *action; struct irq_desc *desc = irq_to_desc(irq->irq); unsigned long flags; - int retval; + int retval = 0; bool_t level; action = xmalloc(struct irqaction); @@ -302,6 +312,28 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq, spin_lock_irqsave(&desc->lock, flags); + /* 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 + * Xen and domains. + */ + if ( desc->action != NULL ) + { + struct domain *ad = irq_get_domain(desc); + + if ( (desc->status & IRQ_GUEST) && d == ad ) + goto out; + + if ( desc->status & IRQ_GUEST ) + printk(XENLOG_ERR "ERROR: IRQ %u is already used by domain %u\n", + irq->irq, ad->domain_id); + else + printk(XENLOG_ERR "ERROR: IRQ %u is already used by Xen\n", + irq->irq); + retval = -EBUSY; + goto out; + } + retval = __setup_irq(desc, action); if ( retval ) {
The current dt_route_irq_to_guest implementation sets IRQ_GUEST even if the IRQ is correctly setup. An IRQ can be shared between devices, if the devices are not assigned to the same domain or Xen, then this could result in routing the IRQ to the domain instead of Xen ... Also avoid to relying on wrong the behaviour when Xen is routing an IRQ to DOM0. Therefore check the return code from route_dt_irq_to_guest in map_device. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- Changes in v3: - Fix typoes and update commit message Changes in v2: - Use EBUSY instead of EADDRINUSE for error code - Don't hardcode the domain in error message - Fix typo in error message - Use irq_get_domain --- xen/arch/arm/domain_build.c | 9 +++++++-- xen/arch/arm/irq.c | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-)