Re: [Xen-devel] [PATCH] xen/arm: setup_dt_irq: don't enable the IRQ if the creation has failed

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-01-10 at 20:49 +0000, Julien Grall wrote:
> For now __setup_dt_irq can only fail if the action is already set.

Can this ever happen with the current code base?

> If in the future, the function is updated we don't want to enable the IRQ.

Such an update is likely to be post-4.4 (unless there is a relationship
with "IRQ: Protect IRQ to be shared between domains and XEN" AND the RM
becomes convinced to grant a freeze exception for that patch).

On that basis this patch could also easily be post-4.4.

> Assuming the function can fail with action = NULL, when Xen will receive the
> IRQ it will segfault because do_IRQ doesn't check if action is NULL.

It seems unlikely that the system would be fully functional after such
an error even with this patch -- it would have failed to register either
timer, maintenance or the console interrupt.

> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  xen/arch/arm/gic.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index e6257a7..62510e3 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -605,8 +605,8 @@ int __init setup_dt_irq(const struct dt_irq *irq, struct 
> irqaction *new)
>      rc = __setup_irq(desc, irq->irq, new);
>      spin_unlock_irqrestore(&desc->lock, flags);
>  
> -    desc->handler->startup(desc);
> -
> +    if ( !rc )
> +        desc->handler->startup(desc);
>  
>      return rc;
>  }



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel