Re: [Xen-devel] [PATCH v4 11/33] xen/arm: route_irq_to_guest: Check validity of the IRQ

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 19 Mar 2015, Julien Grall wrote:
> Currently Xen only supports SPIs routing for guest, add a function
> is_assignable_irq to check if we can assign a given IRQ to the guest.
> 
> Secondly, make sure the vIRQ is not the greater that the number of IRQs
> configured in the vGIC and it's an SPI.
> 
> Thirdly, when the IRQ is already assigned to the domain, check the user
> is not asking to use a different vIRQ than the one already bound.
> 
> Finally, desc->arch.type which contains the IRQ type (i.e level/edge) must
> be correctly configured before. The misconfiguration can happen when:
>     - the device has been blacklisted for the current platform
>     - the IRQ has not been described in the device tree
> 
> 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>
> 

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


>     Changes in v4:
>         - Use NR_LOCAL_IRQS rather than 32
>         - Move the check to the IRQ and irq_to_desc after the vIRQ check
>         - Typoes and rewording the commit message and in the patch
>         - Use printk rather than dprintk.
> 
>     Changes in v3:
>         - Fix typo in commit message and comment
>         - Add a check that the vIRQ is an SPI
>         - Check if the user is not asking for a different vIRQ when the
>         IRQ is already assigned to the guest
> 
>     Changes in v2:
>         - Rename is_routable_irq into is_assignable_irq
>         - Check if the IRQ is not greater than the number handled by the
>         number of IRQs handled by the gic
>         - Move is_assignable_irq in irq.c rather than defining in the
>         header irq.h
>         - Retrieve the irq descriptor after checking the validity of the
>         IRQ
>         - vgic_num_irqs has been moved in a separate patch
>         - Fix the irq check against vgic_num_irqs
>         - Use virq instead of irq for vGIC sanity check
> ---
>  xen/arch/arm/irq.c        | 59 
> +++++++++++++++++++++++++++++++++++++++++++----
>  xen/include/asm-arm/irq.h |  2 ++
>  2 files changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index beb746a..4c3e381 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -387,6 +387,16 @@ err:
>      return rc;
>  }
>  
> +bool_t is_assignable_irq(unsigned int irq)
> +{
> +    /* For now, we can only route SPIs to the guest */
> +    return ((irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines()));
> +}
> +
> +/*
> + * Route an IRQ to a specific guest.
> + * For now only SPIs are assignable to the guest.
> + */
>  int route_irq_to_guest(struct domain *d, unsigned int virq,
>                         unsigned int irq, const char * devname)
>  {
> @@ -396,6 +406,28 @@ int route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>      unsigned long flags;
>      int retval = 0;
>  
> +    if ( virq >= vgic_num_irqs(d) )
> +    {
> +        printk(XENLOG_G_ERR
> +               "the vIRQ number %u is too high for domain %u (max = %u)\n",
> +               irq, d->domain_id, vgic_num_irqs(d));
> +        return -EINVAL;
> +    }
> +
> +    /* Only routing to virtual SPIs is supported */
> +    if ( virq < NR_LOCAL_IRQS )
> +    {
> +        printk(XENLOG_G_ERR "IRQ can only be routed to an SPI");
> +        return -EINVAL;
> +    }
> +
> +    if ( !is_assignable_irq(irq) )
> +    {
> +        printk(XENLOG_G_ERR "the IRQ%u is not routable\n", irq);
> +        return -EINVAL;
> +    }
> +    desc = irq_to_desc(irq);
> +
>      action = xmalloc(struct irqaction);
>      if ( !action )
>          return -ENOMEM;
> @@ -416,8 +448,18 @@ int route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>  
>      spin_lock_irqsave(&desc->lock, flags);
>  
> -    /* If the IRQ is already used by someone
> -     *  - If it's the same domain -> Xen doesn't need to update the IRQ desc
> +    if ( desc->arch.type == DT_IRQ_TYPE_INVALID )
> +    {
> +        printk(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.
> +     *  For safety check if we are not trying to assign the IRQ to a
> +     *  different vIRQ.
>       *  - Otherwise -> For now, don't allow the IRQ to be shared between
>       *  Xen and domains.
>       */
> @@ -426,13 +468,22 @@ int route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>          struct domain *ad = irq_get_domain(desc);
>  
>          if ( test_bit(_IRQ_GUEST, &desc->status) && d == ad )
> +        {
> +            if ( irq_get_guest_info(desc)->virq != virq )
> +            {
> +                printk(XENLOG_G_ERR
> +                       "d%u: IRQ %u is already assigned to vIRQ %u\n",
> +                       d->domain_id, irq, irq_get_guest_info(desc)->virq);
> +                retval = -EBUSY;
> +            }
>              goto out;
> +        }
>  
>          if ( test_bit(_IRQ_GUEST, &desc->status) )
> -            printk(XENLOG_ERR "ERROR: IRQ %u is already used by domain %u\n",
> +            printk(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);
> +            printk(XENLOG_G_ERR "IRQ %u is already used by Xen\n", irq);
>          retval = -EBUSY;
>          goto out;
>      }
> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
> index f00eb11..71b39e7 100644
> --- a/xen/include/asm-arm/irq.h
> +++ b/xen/include/asm-arm/irq.h
> @@ -37,6 +37,8 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, 
> int is_fiq);
>  
>  #define domain_pirq_to_irq(d, pirq) (pirq)
>  
> +bool_t is_assignable_irq(unsigned int irq);
> +
>  void init_IRQ(void);
>  void init_secondary_IRQ(void);
>  
> -- 
> 2.1.4
> 

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