[patch] allow pass-through devices to share guest GSI

[patch] allow pass-through devices to share guest GSI

[Simon Horman]

Allow multiple pass-through devices to use the same guest_gsi.

The motivation for this is:

* Allow multi-function devices to be passed through as multi-function devices
  - This implies that the devices may have functions that use INTB, C or D.
    With the current scheme this would clash with the guest GSI of
    guest INTA on guest device 13, 14, 15, 21, 22, 23, 29 ,30 and 31.

    The guest INTX and  device to guest GSI mapping is described in
    xen/include/asm-x86/hvm/irq.h as:

    #define hvm_pci_intx_gsi(dev, intx)  \
        (((((dev)<<2) + ((dev)>>3) + (intx)) & 31) + 16)

    And is illustrated in this diagram
    http://lists.xensource.com/archives/html/xen-devel/2009-02/pngmIO1Sm0VEX.png

Cc: Weidong Han <weidong.han@intel.com>
Cc: Yuji Shimada <shimada-yxb@necst.nec.co.jp>
Signed-off-by: Simon Horman <horms@verge.net.au>

--- 
* Fri, 13 Mar 2009 18:24:55 +1100

  Weidong Han explained to me that the reson that the existing
  pt_irq_time_out() code only used the last machine_gsi found is that the
  machine_gsi for a guest_gsi was a 1:1 mapping - silly me for not noticing
  that was the reason.

  With this in mind there seems little need for a separate patch that
  handles all machine_gsi of a guest_gsi without introducing sharing of
  guest_gsi.

  I have also edited the description above.

  I am resubmitting this single patch.

* Mon, 09 Mar 2009 20:01:28 +1100

  This seems to work.

  Tested by changing the guest GSI mapping so that dev 6 and 7 both
  use the same guest GSI. Then assign through two ethernet MICs.
  I was able to ping using both NICs and confirmed with
  some debugging code that they were assigned the same guest GSI.

 xen/drivers/passthrough/io.c |  135 ++++++++++++++++++++++++++++--------------
 xen/include/xen/hvm/irq.h    |    4 -
 2 files changed, 92 insertions(+), 47 deletions(-)

Index: xen-unstable.hg/xen/drivers/passthrough/io.c
===================================================================
--- xen-unstable.hg.orig/xen/drivers/passthrough/io.c	2009-03-12 14:04:55.000000000 +1100
+++ xen-unstable.hg/xen/drivers/passthrough/io.c	2009-03-13 18:22:20.000000000 +1100
@@ -36,7 +36,11 @@ static void pt_irq_time_out(void *data)
     int vector;
     struct hvm_irq_dpci *dpci = NULL;
     struct dev_intx_gsi_link *digl;
+    struct hvm_girq_dpci_mapping *girq;
     uint32_t device, intx;
+    DECLARE_BITMAP(machine_gsi_map, NR_IRQS);
+
+    bitmap_zero(machine_gsi_map, NR_IRQS);
 
     spin_lock(&irq_map->dom->event_lock);
 
@@ -45,17 +49,35 @@ static void pt_irq_time_out(void *data)
     list_for_each_entry ( digl, &irq_map->digl_list, list )
     {
         guest_gsi = digl->gsi;
-        machine_gsi = dpci->girq[guest_gsi].machine_gsi;
+        list_for_each_entry ( girq, &dpci->girq[guest_gsi], list )
+        {
+            machine_gsi = girq->machine_gsi;
+            set_bit(machine_gsi, machine_gsi_map);
+        }
         device = digl->device;
         intx = digl->intx;
         hvm_pci_intx_deassert(irq_map->dom, device, intx);
     }
 
-    clear_bit(machine_gsi, dpci->dirq_mask);
-    vector = domain_irq_to_vector(irq_map->dom, machine_gsi);
-    dpci->mirq[machine_gsi].pending = 0;
+    for ( machine_gsi = find_first_bit(machine_gsi_map, NR_IRQS);
+          machine_gsi < NR_IRQS;
+          machine_gsi = find_next_bit(machine_gsi_map, NR_IRQS,
+                                      machine_gsi + 1) )
+    {
+        clear_bit(machine_gsi, dpci->dirq_mask);
+        vector = domain_irq_to_vector(irq_map->dom, machine_gsi);
+        dpci->mirq[machine_gsi].pending = 0;
+    }
+
     spin_unlock(&irq_map->dom->event_lock);
-    pirq_guest_eoi(irq_map->dom, machine_gsi);
+
+    for ( machine_gsi = find_first_bit(machine_gsi_map, NR_IRQS);
+          machine_gsi < NR_IRQS;
+          machine_gsi = find_next_bit(machine_gsi_map, NR_IRQS,
+                                      machine_gsi + 1) )
+    {
+        pirq_guest_eoi(irq_map->dom, machine_gsi);
+    }
 }
 
 extern int msixtbl_pt_register(struct domain *d, int pirq, uint64_t gtable);
@@ -68,6 +90,7 @@ int pt_irq_create_bind_vtd(
     uint32_t machine_gsi, guest_gsi;
     uint32_t device, intx, link;
     struct dev_intx_gsi_link *digl;
+    struct hvm_girq_dpci_mapping *girq;
     int rc, pirq = pt_irq_bind->machine_irq;
 
     if ( pirq < 0 || pirq >= NR_IRQS )
@@ -86,7 +109,10 @@ int pt_irq_create_bind_vtd(
         }
         memset(hvm_irq_dpci, 0, sizeof(*hvm_irq_dpci));
         for ( int i = 0; i < NR_IRQS; i++ )
+        {
             INIT_LIST_HEAD(&hvm_irq_dpci->mirq[i].digl_list);
+            INIT_LIST_HEAD(&hvm_irq_dpci->girq[i]);
+        }
 
         if ( domain_set_irq_dpci(d, hvm_irq_dpci) == 0 )
         {
@@ -145,6 +171,14 @@ int pt_irq_create_bind_vtd(
             return -ENOMEM;
         }
 
+        girq = xmalloc(struct hvm_girq_dpci_mapping);
+        if ( !girq )
+        {
+            xfree(digl);
+            spin_unlock(&d->event_lock);
+            return -ENOMEM;
+        }
+
         digl->device = device;
         digl->intx = intx;
         digl->gsi = guest_gsi;
@@ -152,10 +186,10 @@ int pt_irq_create_bind_vtd(
         list_add_tail(&digl->list,
                       &hvm_irq_dpci->mirq[machine_gsi].digl_list);
 
-        hvm_irq_dpci->girq[guest_gsi].valid = 1;
-        hvm_irq_dpci->girq[guest_gsi].device = device;
-        hvm_irq_dpci->girq[guest_gsi].intx = intx;
-        hvm_irq_dpci->girq[guest_gsi].machine_gsi = machine_gsi;
+        girq->device = device;
+        girq->intx = intx;
+        girq->machine_gsi = machine_gsi;
+        list_add_tail(&girq->list, &hvm_irq_dpci->girq[guest_gsi]);
 
         /* Bind the same mirq once in the same domain */
         if ( !test_and_set_bit(machine_gsi, hvm_irq_dpci->mapping))
@@ -190,10 +224,8 @@ int pt_irq_create_bind_vtd(
                     kill_timer(&hvm_irq_dpci->hvm_timer[vector]);
                 hvm_irq_dpci->mirq[machine_gsi].dom = NULL;
                 clear_bit(machine_gsi, hvm_irq_dpci->mapping);
-                hvm_irq_dpci->girq[guest_gsi].machine_gsi = 0;
-                hvm_irq_dpci->girq[guest_gsi].intx = 0;
-                hvm_irq_dpci->girq[guest_gsi].device = 0;
-                hvm_irq_dpci->girq[guest_gsi].valid = 0;
+                list_del(&girq->list);
+                xfree(girq);
                 list_del(&digl->list);
                 hvm_irq_dpci->link_cnt[link]--;
                 spin_unlock(&d->event_lock);
@@ -218,6 +250,7 @@ int pt_irq_destroy_bind_vtd(
     uint32_t device, intx, link;
     struct list_head *digl_list, *tmp;
     struct dev_intx_gsi_link *digl;
+    struct hvm_girq_dpci_mapping *girq;
 
     machine_gsi = pt_irq_bind->machine_irq;
     device = pt_irq_bind->u.pci.device;
@@ -240,8 +273,16 @@ int pt_irq_destroy_bind_vtd(
     }
 
     hvm_irq_dpci->link_cnt[link]--;
-    memset(&hvm_irq_dpci->girq[guest_gsi], 0,
-           sizeof(struct hvm_girq_dpci_mapping));
+
+    list_for_each_entry ( girq, &hvm_irq_dpci->girq[guest_gsi], list )
+    {
+        if ( girq->machine_gsi == machine_gsi )
+        {
+                list_del(&girq->list);
+                xfree(girq);
+                break;
+        }
+    }
 
     /* clear the mirq info */
     if ( test_bit(machine_gsi, hvm_irq_dpci->mapping))
@@ -410,13 +451,39 @@ void hvm_dirq_assist(struct vcpu *v)
     }
 }
 
+static void __hvm_dpci_eoi(struct domain *d,
+                           struct hvm_irq_dpci *hvm_irq_dpci,
+                           struct hvm_girq_dpci_mapping *girq,
+                           union vioapic_redir_entry *ent)
+{
+    uint32_t device, intx, machine_gsi;
+
+    device = girq->device;
+    intx = girq->intx;
+    hvm_pci_intx_deassert(d, device, intx);
+
+    machine_gsi = girq->machine_gsi;
+
+    /*
+     * No need to get vector lock for timer
+     * since interrupt is still not EOIed
+     */
+    if ( --hvm_irq_dpci->mirq[machine_gsi].pending ||
+         ( ent && ent->fields.mask ) ||
+         ! pt_irq_need_timer(hvm_irq_dpci->mirq[machine_gsi].flags) )
+        return;
+
+    stop_timer(&hvm_irq_dpci->hvm_timer[domain_irq_to_vector(d, machine_gsi)]);
+    pirq_guest_eoi(d, machine_gsi);
+}
+
 void hvm_dpci_eoi(struct domain *d, unsigned int guest_gsi,
                   union vioapic_redir_entry *ent)
 {
-    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
-    uint32_t device, intx, machine_gsi;
+    struct hvm_irq_dpci *hvm_irq_dpci;
+    struct hvm_girq_dpci_mapping *girq;
 
-    if ( !iommu_enabled)
+    if ( !iommu_enabled )
         return;
 
     if ( guest_gsi < NR_ISAIRQS )
@@ -428,34 +495,12 @@ void hvm_dpci_eoi(struct domain *d, unsi
     spin_lock(&d->event_lock);
     hvm_irq_dpci = domain_get_irq_dpci(d);
 
-    if((hvm_irq_dpci == NULL) ||
-         (guest_gsi >= NR_ISAIRQS &&
-          !hvm_irq_dpci->girq[guest_gsi].valid) )
-    {
-        spin_unlock(&d->event_lock);
-        return;
-    }
+    if ( !hvm_irq_dpci )
+        goto unlock;
 
-    device = hvm_irq_dpci->girq[guest_gsi].device;
-    intx = hvm_irq_dpci->girq[guest_gsi].intx;
-    hvm_pci_intx_deassert(d, device, intx);
+    list_for_each_entry ( girq, &hvm_irq_dpci->girq[guest_gsi], list )
+        __hvm_dpci_eoi(d, hvm_irq_dpci, girq, ent);
 
-    machine_gsi = hvm_irq_dpci->girq[guest_gsi].machine_gsi;
-    if ( --hvm_irq_dpci->mirq[machine_gsi].pending == 0 )
-    {
-        if ( (ent == NULL) || !ent->fields.mask )
-        {
-            /*
-             * No need to get vector lock for timer
-             * since interrupt is still not EOIed
-             */
-            if ( pt_irq_need_timer(hvm_irq_dpci->mirq[machine_gsi].flags) )
-            {
-                stop_timer(&hvm_irq_dpci->hvm_timer[
-                    domain_irq_to_vector(d, machine_gsi)]);
-                pirq_guest_eoi(d, machine_gsi);
-            }
-        }
-    }
+unlock:
     spin_unlock(&d->event_lock);
 }
Index: xen-unstable.hg/xen/include/xen/hvm/irq.h
===================================================================
--- xen-unstable.hg.orig/xen/include/xen/hvm/irq.h	2009-03-13 18:21:54.000000000 +1100
+++ xen-unstable.hg/xen/include/xen/hvm/irq.h	2009-03-13 18:22:20.000000000 +1100
@@ -60,7 +60,7 @@ struct hvm_mirq_dpci_mapping {
 };
 
 struct hvm_girq_dpci_mapping {
-    uint8_t valid;
+    struct list_head list;
     uint8_t device;
     uint8_t intx;
     uint8_t machine_gsi;
@@ -75,7 +75,7 @@ struct hvm_irq_dpci {
     DECLARE_BITMAP(mapping, NR_IRQS);
     struct hvm_mirq_dpci_mapping mirq[NR_IRQS];
     /* Guest IRQ to guest device/intx mapping. */
-    struct hvm_girq_dpci_mapping girq[NR_IRQS];
+    struct list_head girq[NR_IRQS];
     uint8_t msi_gvec_pirq[NR_VECTORS];
     DECLARE_BITMAP(dirq_mask, NR_IRQS);
     /* Record of mapped ISA IRQs */

Re: [patch] allow pass-through devices to share guest GSI

[Keir Fraser]

On 13/03/2009 07:31, "Simon Horman" <horms@verge.net.au> wrote:

>  With this in mind there seems little need for a separate patch that
>   handles all machine_gsi of a guest_gsi without introducing sharing of
>   guest_gsi.
> 
>   I have also edited the description above.
> 
>   I am resubmitting this single patch.

Please send a patch which applies to xen-unstable tip, which includes your
original patch.

 -- Keir

Re: [patch] allow pass-through devices to share guest GSI

[Simon Horman]

On Fri, Mar 13, 2009 at 07:55:45AM +0000, Keir Fraser wrote:
> On 13/03/2009 07:31, "Simon Horman" <horms@verge.net.au> wrote:
> 
> >  With this in mind there seems little need for a separate patch that
> >   handles all machine_gsi of a guest_gsi without introducing sharing of
> >   guest_gsi.
> > 
> >   I have also edited the description above.
> > 
> >   I am resubmitting this single patch.
> 
> Please send a patch which applies to xen-unstable tip, which includes your
> original patch.

Actually, it seems that this change has already been
applied as 19302:b5d074255c38 and 19303:6357628c678f.
Sorry for not checking that before reposting.

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en