From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Fitzhardinge Subject: Re: [PATCH][RFC] pv-ops: fix shared irq device passthrough Date: Mon, 26 Oct 2009 13:27:50 -0700 Message-ID: <4AE60646.7050307@goop.org> References: <715D42877B251141A38726ABF5CABF2C05509A75F3@pdsmsx503.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <715D42877B251141A38726ABF5CABF2C05509A75F3@pdsmsx503.ccr.corp.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "Han, Weidong" Cc: "'xen-devel@lists.xensource.com'" , "'keir.fraser@eu.citrix.com'" List-Id: xen-devel@lists.xenproject.org On 10/23/09 04:39, Han, Weidong wrote: > From 9c91f23076c555690488cbd81f889f279d4cf2fa Mon Sep 17 00:00:00 2001 > From: Weidong Han > Date: Sat, 24 Oct 2009 03:18:30 +0800 > Subject: [PATCH] pv-ops: fix shared irq device passthrough > > In driver/xen/events.c, whether bind_pirq is shareable or not is > determined by desc->action is NULL or not. But in __setup_irq, > startup(irq) is invoked before desc->action is assigned with > new action. So desc->action in startup_irq is alwasy NULL, and > bind_pirq is always not shareable. This results in pt_irq_create_bind > failure when passthrough a device which shares irq to other devices. > > This patch move desc->action before startup(irq), therefore fix > the problem. > Hm, not sure about this. The logic in __setup_irq already looks pretty tortured, and I'd like to avoid touching it unless there's definitively a bug in there. I think the right question is whether probe_irq() is really doing the right test, and whether it could do something else instead? J > Signed-off-by: Weidong Han > --- > kernel/irq/manage.c | 14 ++++++++++---- > 1 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index 07a11dc..3b85b72 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -565,6 +565,7 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > { > struct irqaction *old, **old_ptr; > const char *old_name = NULL; > + int old_irq; > unsigned long flags; > int shared = 0; > int ret; > @@ -644,6 +645,11 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > shared = 1; > } > > + old = *old_ptr; > + old_irq = new->irq; > + new->irq = irq; > + *old_ptr = new; > + > if (!shared) { > irq_chip_set_defaults(desc->chip); > > @@ -654,8 +660,11 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > ret = __irq_set_trigger(desc, irq, > new->flags & IRQF_TRIGGER_MASK); > > - if (ret) > + if (ret) { > + new->irq = old_irq; > + *old_ptr = old; > goto out_thread; > + } > } else > compat_irq_chip_set_default_handler(desc); > #if defined(CONFIG_IRQ_PER_CPU) > @@ -690,9 +699,6 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > (int)(new->flags & IRQF_TRIGGER_MASK)); > } > > - new->irq = irq; > - *old_ptr = new; > - > /* Reset broken irq detection when installing new handler */ > desc->irq_count = 0; > desc->irqs_unhandled = 0; >