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

[Jeremy Fitzhardinge <jeremy@xxxxxxxx>]

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?

    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