RE: [Xen-devel] [PATCH][RFC] pv-ops: fix shared irq device passthrough

["Han, Weidong" <weidong.han@xxxxxxxxx>]

Jeremy Fitzhardinge wrote:
> On 10/23/09 04:39, Han, Weidong wrote:
>> From 9c91f23076c555690488cbd81f889f279d4cf2fa Mon Sep 17 00:00:00
>> 2001 
>> From: Weidong Han <weidong.han@xxxxxxxxx>
>> 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?

Totally agree. The better way is to change probe_irq, therefore needn't touch 
kernel irq stuffs. probe_irq is just check if desc->action == NULL or not. The 
code is there for a long long time (from c/s 26 in linux-2.6.18-xen.hg). I 
don't know whether it can be replaced. 

Regards,
Weidong

> 
>     J
> 
>> Signed-off-by: Weidong Han <weidong.han@xxxxxxxxx>
>> ---
>>  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;


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel