WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH][RFC] Support S3 for MSI interrupt in latest kernel d

To: Jan Beulich <jbeulich@xxxxxxxxxx>, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH][RFC] Support S3 for MSI interrupt in latest kernel dom0
From: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Date: Wed, 17 Dec 2008 20:11:02 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc:
Delivery-date: Wed, 17 Dec 2008 04:12:09 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AclgQIdFN7aHoBN8ReW8QcHRT1Xb5g==
Thread-topic: [PATCH][RFC] Support S3 for MSI interrupt in latest kernel dom0
In latest kernel, the pci_save_state will not try to save msi/x_state anymore, 
instead, it will try to restore msi state when resume using kernel's msi data 
structure. This cause trouble for us, since thoese MSI data structure is 
meaningless in Xen environment. 

Several option to resolve this issue:
a) Change the latest kernel (as dom0) to still to save/restore the msi content
b) Add a new hypercall, so when dom0 try to restore dom0, it will instruct Xen 
HV to restore the content based on Xen's MSI data structure
c) In Dom0's pci_restore_msi/x_state, try to call map_domain_pirq again. Xen HV 
will found there is a vector assigned already, then it will try to re_startup 
the interrupt.

We try to cook a patch for option c) as following (it is still not fully tested 
because my environment is broken and I send to mailing list to get some 
feedback in advance), but I suspect this option is not so good because it mix 
the function of map_domain_pirq and pirq_guest_bind more (it is very un-clear 
of the boundary between these two function now). And I'm not sure if the 
re-startup will cause potential issue for non-S3 situation.

Any suggestion?

Thanks
Yunhong Jiang

diff -r 045f70d1acdb xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c        Sat Dec 13 17:44:20 2008 +0000
+++ b/xen/arch/x86/irq.c        Wed Dec 17 12:49:18 2008 +0800
@@ -896,12 +896,23 @@ int map_domain_pirq(
         spin_lock_irqsave(&desc->lock, flags);
 
         if ( desc->handler != &no_irq_type )
-            dprintk(XENLOG_G_ERR, "dom%d: vector %d in use\n",
+            dprintk(XENLOG_G_INFO, "dom%d: vector %d in use\n",
               d->domain_id, vector);
         desc->handler = &pci_msi_type;
         d->arch.pirq_vector[pirq] = vector;
         d->arch.vector_pirq[vector] = pirq;
         setup_msi_irq(pdev, msi_desc);
+
+        /* If the vector has been bound, re-startup it again for S3 situation 
*/
+        if (desc->status & IRQ_GUEST)
+        {
+            irq_guest_action_t *action;
+
+            action = (irq_guest_action_t *)desc->action;
+
+            if ((action->nr_guests == 1) && (action->guest[0] == d))
+                desc->handler->startup(vector);
+        }
         spin_unlock_irqrestore(&desc->lock, flags);
     } else
     {
diff -r 045f70d1acdb xen/arch/x86/msi.c
--- a/xen/arch/x86/msi.c        Sat Dec 13 17:44:20 2008 +0000
+++ b/xen/arch/x86/msi.c        Wed Dec 17 14:52:59 2008 +0800
@@ -147,6 +147,9 @@ static int set_vector_msi(struct msi_des
         return -EINVAL;
     }
 
+    BUG_ON( (irq_desc[entry->vector].msi_desc)
+             &&(irq_desc[entry->vector].msi_desc != entry) );
+
     irq_desc[entry->vector].msi_desc = entry;
     return 0;
 }
@@ -573,17 +576,19 @@ static int __pci_enable_msi(struct msi_i
 {
     int status;
     struct pci_dev *pdev;
+    struct msi_desc *msi_desc = NULL;
 
     ASSERT(rw_is_locked(&pcidevs_lock));
     pdev = pci_get_pdev(msi->bus, msi->devfn);
     if ( !pdev )
         return -ENODEV;
 
-    if ( find_msi_entry(pdev, msi->vector, PCI_CAP_ID_MSI) )
+    if ( (msi_desc = find_msi_entry(pdev, msi->vector, PCI_CAP_ID_MSI) ))
     {
         dprintk(XENLOG_WARNING, "vector %d has already mapped to MSI on "
                 "device %02x:%02x.%01x.\n", msi->vector, msi->bus,
                 PCI_SLOT(msi->devfn), PCI_FUNC(msi->devfn));
+        *desc = msi_desc;
         return 0;
     }
 
@@ -633,6 +638,7 @@ static int __pci_enable_msix(struct msi_
     u16 control;
     u8 slot = PCI_SLOT(msi->devfn);
     u8 func = PCI_FUNC(msi->devfn);
+    struct msi_desc *msi_desc;
 
     ASSERT(rw_is_locked(&pcidevs_lock));
     pdev = pci_get_pdev(msi->bus, msi->devfn);
@@ -645,11 +651,12 @@ static int __pci_enable_msix(struct msi_
     if (msi->entry_nr >= nr_entries)
         return -EINVAL;
 
-    if ( find_msi_entry(pdev, msi->vector, PCI_CAP_ID_MSIX) )
+    if ( (msi_desc = find_msi_entry(pdev, msi->vector, PCI_CAP_ID_MSIX)))
     {
         dprintk(XENLOG_WARNING, "vector %d has already mapped to MSIX on "
                 "device %02x:%02x.%01x.\n", msi->vector, msi->bus,
                 PCI_SLOT(msi->devfn), PCI_FUNC(msi->devfn));
+        *desc = msi_desc;
         return 0;
     }
 
diff -r 045f70d1acdb xen/arch/x86/physdev.c
--- a/xen/arch/x86/physdev.c    Sat Dec 13 17:44:20 2008 +0000
+++ b/xen/arch/x86/physdev.c    Wed Dec 17 10:10:36 2008 +0800
@@ -51,6 +51,9 @@ static int physdev_map_pirq(struct physd
         goto free_domain;
     }
 
+    read_lock(&pcidevs_lock);
+    spin_lock(&d->event_lock);
+
     /* Verify or get vector. */
     switch ( map->type )
     {
@@ -60,7 +63,7 @@ static int physdev_map_pirq(struct physd
                 dprintk(XENLOG_G_ERR, "dom%d: map invalid irq %d\n",
                         d->domain_id, map->index);
                 ret = -EINVAL;
-                goto free_domain;
+                goto vector_fail;
             }
             vector = IO_APIC_VECTOR(map->index);
             if ( !vector )
@@ -68,21 +71,27 @@ static int physdev_map_pirq(struct physd
                 dprintk(XENLOG_G_ERR, "dom%d: map irq with no vector %d\n",
                         d->domain_id, vector);
                 ret = -EINVAL;
-                goto free_domain;
+                goto vector_fail;
             }
             break;
 
         case MAP_PIRQ_TYPE_MSI:
             vector = map->index;
+
             if ( vector == -1 )
-                vector = assign_irq_vector(AUTO_ASSIGN);
+            {
+                if (map->pirq >= 0)
+                    vector = d->arch.pirq_vector[map->pirq];
+                else
+                    vector = assign_irq_vector(AUTO_ASSIGN);
+            }
 
             if ( vector < 0 || vector >= NR_VECTORS )
             {
                 dprintk(XENLOG_G_ERR, "dom%d: map irq with wrong vector %d\n",
                         d->domain_id, vector);
                 ret = -EINVAL;
-                goto free_domain;
+                goto vector_fail;
             }
 
             _msi.bus = map->bus;
@@ -97,12 +106,10 @@ static int physdev_map_pirq(struct physd
             dprintk(XENLOG_G_ERR, "dom%d: wrong map_pirq type %x\n",
                     d->domain_id, map->type);
             ret = -EINVAL;
-            goto free_domain;
-    }
-
-    read_lock(&pcidevs_lock);
+            goto vector_fail;
+    }
+
     /* Verify or get pirq. */
-    spin_lock(&d->event_lock);
     if ( map->pirq < 0 )
     {
         if ( d->arch.vector_pirq[vector] )
@@ -147,11 +154,12 @@ static int physdev_map_pirq(struct physd
         map->pirq = pirq;
 
 done:
+    if ( (ret != 0) && (map->type == MAP_PIRQ_TYPE_MSI) && (map->index == -1) )
+        free_irq_vector(vector);
+vector_fail:
     spin_unlock(&d->event_lock);
     read_unlock(&pcidevs_lock);
-    if ( (ret != 0) && (map->type == MAP_PIRQ_TYPE_MSI) && (map->index == -1) )
-        free_irq_vector(vector);
-free_domain:
+free_domain:    
     rcu_unlock_domain(d);
     return ret;
 }

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel