Re: [Xen-devel] [XenARM] XEN tools for ARM with Virtualization Extensions

["Eric Trudeau" <etrudeau@xxxxxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@xxxxxxxxxx]
> Sent: Thursday, July 11, 2013 1:18 PM
> To: Eric Trudeau
> Cc: xen-devel; Julien Grall; Stefano.Stabellini@xxxxxxxxxx
> Subject: Re: [Xen-devel] [XenARM] XEN tools for ARM with Virtualization
> Extensions
> 
> On Thu, 2013-07-11 at 16:46 +0000, Eric Trudeau wrote:
> > > > >> > What functions should I call to implement  XEN_DOMCTL_bind_pt_irq
> on
> > > > >> ARM?
> > > > >>
> > > > >> There's a function like route_irq_to_guest which we use to route IRQs
> to
> > > > >> dom0 during boot. In principal that could also be used to reroute an 
> > > > >> IRQ
> > > > >> to a guest, but I'm not sure how it will interact with the 
> > > > >> reassignment,
> > > > >> since in your case the IRQ starts off bound to dom0. Hopefully it's 
> > > > >> just
> > > > >> a small change to make it work for this case.
> > > > >>
> >
> > I have IRQs for devices being passed to ARM DomU guests now.  Thanks for
> your
> > help.
> 
> Excellent! Do you have any useful patches?
> 

I added code in the arm domain.c to release the IRQs when a domain is destroyed.
I am providing my changes, but I believe there may be more work to have a clean
solution.  Specifically, the following items may need to be addressed.

1.      The dom.cfg "irqs" section has the 32 added to the device-tree IRQ 
number because
           I wasn't sure where to add the translation.  This can be cleaned up 
when
           guests are able to be given DTB files and Xen can parse them.

2.      I assume level-triggered IRQs since the "irqs" list is only irq 
numbers.  This also
           could be left until guest DTB files are supported.

3.      I added clearing of the IRQ_GUEST bit in the desc->status in 
release_irq because
            I didn't see where it would be unset.  This is probably not a big 
deal since not
            many situations arise where an IRQ is sometimes host and sometimes 
guest.

4.      I changed vgic.c to use the d->nr_irqs instead of assuming guests have 
no SPIs.
             This allowed me to use the extra_domu_irqs boot param to allow 
guests to
             have SPIs.

5.      I added a check for whether an IRQ was already in use, earlier in the 
flow of
             gic_route_irq_to_guest() so that the desc->handler is not messed 
with before
             __setup_irq does the check and returns an error.  Also, 
gic_set_irq_properties
             will be avoided in the error case as well.

I rebased to commit 9eabb07 and verified my changes.  I needed the fix in 
gic_irq_shutdown
or my release_irq changes caused other IRQs to be disabled when a domain was 
destroyed.

Thanks,
Eric

From 551f174c9653def01890fadd2dd769ab8a9acde7 Mon Sep 17 00:00:00 2001
From: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
Date: Thu, 11 Jul 2013 20:03:51 -0400
Subject: [PATCH] Add support for Guest physdev irqs

---
 xen/arch/arm/domain.c  | 16 ++++++++++++++++
 xen/arch/arm/gic.c     | 15 ++++++++++-----
 xen/arch/arm/physdev.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 xen/arch/arm/vgic.c    |  5 +----
 4 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 4c434a1..52d3429 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -31,6 +31,8 @@
 #include <asm/gic.h>
 #include "vtimer.h"
 #include "vpl011.h"
+#include <xen/iocap.h>
+#include <xen/irq.h>

 DEFINE_PER_CPU(struct vcpu *, curr_vcpu);

@@ -513,8 +515,22 @@ fail:
     return rc;
 }

+static int release_domain_irqs(struct domain *d)
+{
+    int i;
+    for (i = 0; i <= d->nr_pirqs; i++) {
+        if (irq_access_permitted(d, i)) {
+            release_irq(i);
+        }
+    }
+    return 0;
+}
+
+
 void arch_domain_destroy(struct domain *d)
 {
+    if (d->irq_caps != NULL)
+        release_domain_irqs(d);
     p2m_teardown(d);
     domain_vgic_free(d);
     domain_uart0_free(d);
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index cafb681..1f576d1 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -510,7 +510,7 @@ void gic_route_spis(void)
     }
 }

-void __init release_irq(unsigned int irq)
+void release_irq(unsigned int irq)
 {
     struct irq_desc *desc;
     unsigned long flags;
@@ -522,6 +522,7 @@ void __init release_irq(unsigned int irq)
     action = desc->action;
     desc->action  = NULL;
     desc->status |= IRQ_DISABLED;
+    desc->status &= ~IRQ_GUEST;

     spin_lock(&gic.lock);
     desc->handler->shutdown(desc);
@@ -707,6 +708,12 @@ int gic_route_irq_to_guest(struct domain *d, const struct 
dt_irq *irq,
     spin_lock_irqsave(&desc->lock, flags);
     spin_lock(&gic.lock);

+    if ( desc->action != NULL )
+    {
+        retval = -EBUSY;
+        goto out;
+    }
+
     desc->handler = &gic_guest_irq_type;
     desc->status |= IRQ_GUEST;

@@ -715,12 +722,10 @@ int gic_route_irq_to_guest(struct domain *d, const struct 
dt_irq *irq,
     gic_set_irq_properties(irq->irq, level, 1u << smp_processor_id(), 0xa0);

     retval = __setup_irq(desc, irq->irq, action);
-    if (retval) {
-        xfree(action);
-        goto out;
-    }

 out:
+    if (retval)
+        xfree(action);
     spin_unlock(&gic.lock);
     spin_unlock_irqrestore(&desc->lock, flags);
     return retval;
diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
index 61b4a18..8a5f770 100644
--- a/xen/arch/arm/physdev.c
+++ b/xen/arch/arm/physdev.c
@@ -9,12 +9,56 @@
 #include <xen/lib.h>
 #include <xen/errno.h>
 #include <asm/hypercall.h>
+#include <public/physdev.h>
+#include <xen/guest_access.h>
+#include <xen/irq.h>
+#include <xen/sched.h>
+#include <asm/gic.h>


 int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
-    printk("%s %d cmd=%d: not implemented yet\n", __func__, __LINE__, cmd);
-    return -ENOSYS;
+    int ret;
+
+    switch ( cmd )
+    {
+    case PHYSDEVOP_map_pirq: {
+        physdev_map_pirq_t map;
+        struct dt_irq irq;
+        struct domain *d;
+
+        ret = -EFAULT;
+        if ( copy_from_guest(&map, arg, 1) != 0 )
+            break;
+
+        d = rcu_lock_domain_by_any_id(map.domid);
+        if ( d == NULL ) {
+            ret = -ESRCH;
+            break;
+        }
+
+        irq.irq = map.pirq;
+        irq.type = DT_IRQ_TYPE_LEVEL_MASK;
+
+        ret = gic_route_irq_to_guest(d, &irq, "GuestIRQ");
+        if (!ret)
+            printk("%s gic_route_irq_to_guest added IRQ %d to dom%d\n",
+                   __FUNCTION__, irq.irq, d->domain_id);
+
+        rcu_unlock_domain(d);
+
+        if (!ret && __copy_to_guest(arg, &map, 1) )
+            ret = -EFAULT;
+        break;
+    }
+
+    default:
+        printk("%s %d cmd=%d: not implemented yet\n", __func__, __LINE__, cmd);
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
 }

 /*
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 2e4b11f..0ebcdac 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -82,10 +82,7 @@ int domain_vgic_init(struct domain *d)
     /* Currently nr_lines in vgic and gic doesn't have the same meanings
      * Here nr_lines = number of SPIs
      */
-    if ( d->domain_id == 0 )
-        d->arch.vgic.nr_lines = gic_number_lines() - 32;
-    else
-        d->arch.vgic.nr_lines = 0; /* We don't need SPIs for the guest */
+    d->arch.vgic.nr_lines = d->nr_pirqs - 32;

     d->arch.vgic.shared_irqs =
         xzalloc_array(struct vgic_irq_rank, DOMAIN_NR_RANKS(d));
--
1.8.1.2

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