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

[Jeremy Fitzhardinge <jeremy@goop.org>]

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