Re: [Xen-devel] [RFC 04/19] xen/arm: route_irq_to_guest: Check validity of the IRQ

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 16 Jun 2014, Julien Grall wrote:
> Currently Xen only supports SPIs routing for guest, add a function
> is_routable_irq to check if we can route a given IRQ to the guest.
> 
> Secondly, make sure the vIRQ (which is currently the same as the pIRQ) is not
> the greater that the number of IRQs handle to the vGIC.
> 
> Finally, desc->arch.type which contains the IRQ type (i.e level/edge) must
> be correctly configured before. The IRQ type won't be configure when:
>     - the device has been blacklist for the current platform
>     - the IRQ has not been describe in the device tree
> 
> I think we can safely assume that a user won't never ask to route
> as such IRQ to the guest.
> 
> Also, use XENLOG_G_ERR in the error message within the function as it will
> be later called from a guest.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  xen/arch/arm/irq.c        |   32 +++++++++++++++++++++++++++++---
>  xen/include/asm-arm/gic.h |    2 ++
>  xen/include/asm-arm/irq.h |    6 ++++++
>  3 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 9c141bc..4e51fee 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -361,6 +361,10 @@ err:
>      return rc;
>  }
>  
> +/* Route an IRQ to a specific guest.
> + * For now the vIRQ is equal to the pIRQ and only SPIs are routabled to
> + * the guest.
> + */
>  int route_irq_to_guest(struct domain *d, unsigned int irq,
>                         const char * devname)
>  {
> @@ -369,6 +373,20 @@ int route_irq_to_guest(struct domain *d, unsigned int 
> irq,
>      unsigned long flags;
>      int retval = 0;
>  
> +    if ( !is_routable_irq(irq) )
> +    {
> +        dprintk(XENLOG_G_ERR, "the IRQ%u is not routable\n", irq);
> +        return -EINVAL;
> +    }
> +
> +    if ( irq > vgic_num_irqs(d) )
> +    {
> +        dprintk(XENLOG_G_ERR,
> +                "the IRQ number %u is too high for domain %u (max = %u)\n",
> +                irq, d->domain_id, vgic_num_irqs(d));
> +        return -EINVAL;
> +    }

I think it makes sense to move the "irq > vgic_num_irqs(d)" check
within is_routable_irq.


>      action = xmalloc(struct irqaction);
>      if (!action)
>          return -ENOMEM;
> @@ -379,6 +397,14 @@ int route_irq_to_guest(struct domain *d, unsigned int 
> irq,
>  
>      spin_lock_irqsave(&desc->lock, flags);
>  
> +    if ( desc->arch.type == DT_IRQ_TYPE_INVALID )
> +    {
> +        dprintk(XENLOG_G_ERR, "IRQ %u has not been configured\n",
> +                irq);
> +        retval = -EIO;
> +        goto out;
> +    }
> +
>      /* If the IRQ is already used by someone
>       *  - If it's the same domain -> Xen doesn't need to update the IRQ desc
>       *  - Otherwise -> For now, don't allow the IRQ to be shared between
> @@ -392,10 +418,10 @@ int route_irq_to_guest(struct domain *d, unsigned int 
> irq,
>              goto out;
>  
>          if ( desc->status & IRQ_GUEST )
> -            printk(XENLOG_ERR "ERROR: IRQ %u is already used by domain %u\n",
> -                   irq, ad->domain_id);
> +            dprintk(XENLOG_G_ERR, "IRQ %u is already used by domain %u\n",
> +                    irq, ad->domain_id);
>          else
> -            printk(XENLOG_ERR "ERROR: IRQ %u is already used by Xen\n", irq);
> +            dprintk(XENLOG_G_ERR, "IRQ %u is already used by Xen\n", irq);
>          retval = -EBUSY;
>          goto out;
>      }
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 8e37ccf..6e7375c 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -163,6 +163,8 @@
>  #define DT_MATCH_GIC    DT_MATCH_COMPATIBLE("arm,cortex-a15-gic"), \
>                          DT_MATCH_COMPATIBLE("arm,cortex-a7-gic")
>  
> +#define vgic_num_irqs(d)    ((d)->arch.vgic.nr_lines + 32)
> +
>  extern int domain_vgic_init(struct domain *d);
>  extern void domain_vgic_free(struct domain *d);
>  
> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
> index e567f71..63926a5 100644
> --- a/xen/include/asm-arm/irq.h
> +++ b/xen/include/asm-arm/irq.h
> @@ -37,6 +37,12 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, 
> int is_fiq);
>  
>  #define domain_pirq_to_irq(d, pirq) (pirq)
>  
> +static inline bool_t is_routable_irq(unsigned int irq)
> +{
> +    /* For now, we can only route SPIs to the guest */
> +    return (irq >= NR_LOCAL_IRQS);
> +}
> +
>  void init_IRQ(void);
>  void init_secondary_IRQ(void);
>  
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

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