diff mbox

[Xen-devel,v2,07/21] xen/arm: route_irq_to_guest: Check validity of the IRQ

Message ID 1406818852-31856-8-git-send-email-julien.grall@linaro.org
State Superseded, archived
Headers show

Commit Message

Julien Grall July 31, 2014, 3 p.m. UTC
Currently Xen only supports SPIs routing for guest, add a function
is_assign_irq to check if we can assign 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@linaro.org>

---
    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        |   39 ++++++++++++++++++++++++++++++++++++---
 xen/include/asm-arm/irq.h |    2 ++
 2 files changed, 38 insertions(+), 3 deletions(-)

Comments

Stefano Stabellini Aug. 6, 2014, 2:56 p.m. UTC | #1
On Thu, 31 Jul 2014, Julien Grall wrote:
> Currently Xen only supports SPIs routing for guest, add a function
> is_assign_irq to check if we can assign a given IRQ to the guest.

   ^ is_assignable_irq


> 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@linaro.org>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>



> ---
>     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        |   39 ++++++++++++++++++++++++++++++++++++---
>  xen/include/asm-arm/irq.h |    2 ++
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 830832c..7eeb8dd 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -379,6 +379,15 @@ 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 assignabled to the guest.
> + */
>  int route_irq_to_guest(struct domain *d, unsigned int virq,
>                         unsigned int irq, const char * devname)
>  {
> @@ -388,6 +397,22 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
>      unsigned long flags;
>      int retval = 0;
>  
> +    if ( !is_assignable_irq(irq) )
> +    {
> +        dprintk(XENLOG_G_ERR, "the IRQ%u is not routable\n", irq);
> +        return -EINVAL;
> +    }
> +
> +    desc = irq_to_desc(irq);
> +
> +    if ( virq >= 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;
> +    }
> +
>      action = xmalloc(struct irqaction);
>      if ( !action )
>          return -ENOMEM;
> @@ -408,6 +433,14 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
>  
>      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
> @@ -421,10 +454,10 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
>              goto out;
>  
>          if ( test_bit(_IRQ_GUEST, &desc->status) )
> -            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/irq.h b/xen/include/asm-arm/irq.h
> index 9bc3492..a7174f3 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);
>  
> -- 
> 1.7.10.4
>
diff mbox

Patch

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 830832c..7eeb8dd 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -379,6 +379,15 @@  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 assignabled to the guest.
+ */
 int route_irq_to_guest(struct domain *d, unsigned int virq,
                        unsigned int irq, const char * devname)
 {
@@ -388,6 +397,22 @@  int route_irq_to_guest(struct domain *d, unsigned int virq,
     unsigned long flags;
     int retval = 0;
 
+    if ( !is_assignable_irq(irq) )
+    {
+        dprintk(XENLOG_G_ERR, "the IRQ%u is not routable\n", irq);
+        return -EINVAL;
+    }
+
+    desc = irq_to_desc(irq);
+
+    if ( virq >= 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;
+    }
+
     action = xmalloc(struct irqaction);
     if ( !action )
         return -ENOMEM;
@@ -408,6 +433,14 @@  int route_irq_to_guest(struct domain *d, unsigned int virq,
 
     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
@@ -421,10 +454,10 @@  int route_irq_to_guest(struct domain *d, unsigned int virq,
             goto out;
 
         if ( test_bit(_IRQ_GUEST, &desc->status) )
-            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/irq.h b/xen/include/asm-arm/irq.h
index 9bc3492..a7174f3 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);