[Xen-changelog] [xen master] x86: allow guest to set/clear MSI-X mask bit (try 2)

[patchbot@xxxxxxx]

commit a35137373aa9042424565e5ee76dc0a3bb7642ae
Author:     Joby Poriyath <joby.poriyath@xxxxxxxxxx>
AuthorDate: Mon Sep 9 10:43:11 2013 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon Sep 9 10:43:11 2013 +0200

    x86: allow guest to set/clear MSI-X mask bit (try 2)
    
    Guest needs the ability to enable and disable MSI-X interrupts
    by setting the MSI-X control bit, for a passed-through device.
    Guest is allowed to write MSI-X mask bit only if Xen *thinks*
    that mask is clear (interrupts enabled). If the mask is set by
    Xen (interrupts disabled), writes to mask bit by the guest is
    ignored.
    
    Currently, a write to MSI-X mask bit by the guest is silently
    ignored.
    
    A likely scenario is where we have a 82599 SR-IOV nic passed
    through to a guest. From the guest if you do
    
      ifconfig <ETH_DEV> down
      ifconfig <ETH_DEV> up
    
    the interrupts remain masked. On VF reset, the mask bit is set
    by the controller. At this point, Xen is not aware that mask is set.
    However, interrupts are enabled by VF driver by clearing the mask
    bit by writing directly to BAR3 region containing the MSI-X table.
    
    From dom0, we can verify that
    interrupts are being masked using 'xl debug-keys M'.
    
    Initially, guest was allowed to modify MSI-X bit.
    Later this behaviour was changed.
    See changeset 74c213c506afcd74a8556dd092995fd4dc38b225.
    
    Signed-off-by: Joby Poriyath <joby.poriyath@xxxxxxxxxx>
---
 xen/arch/x86/hvm/vmsi.c |   75 +++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 0d5ef1b..4826b4a 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -187,6 +187,19 @@ static struct msixtbl_entry *msixtbl_find_entry(
     return NULL;
 }
 
+static struct msi_desc *virt_to_msi_desc(struct pci_dev *dev, void *virt)
+{
+    struct msi_desc *desc;
+
+    list_for_each_entry( desc, &dev->msi_list, list )
+        if ( desc->msi_attrib.type == PCI_CAP_ID_MSIX  &&
+             virt >= desc->mask_base &&
+             virt < desc->mask_base + PCI_MSIX_ENTRY_SIZE ) 
+            return desc;
+
+    return NULL;
+}
+
 static void __iomem *msixtbl_addr_to_virt(
     struct msixtbl_entry *entry, unsigned long addr)
 {
@@ -247,13 +260,16 @@ out:
 }
 
 static int msixtbl_write(struct vcpu *v, unsigned long address,
-                        unsigned long len, unsigned long val)
+                         unsigned long len, unsigned long val)
 {
     unsigned long offset;
     struct msixtbl_entry *entry;
+    const struct msi_desc *msi_desc;
     void *virt;
     unsigned int nr_entry, index;
     int r = X86EMUL_UNHANDLEABLE;
+    unsigned long flags, orig;
+    struct irq_desc *desc;
 
     if ( len != 4 || (address & 3) )
         return r;
@@ -283,22 +299,57 @@ static int msixtbl_write(struct vcpu *v, unsigned long 
address,
     if ( !virt )
         goto out;
 
-    /* Do not allow the mask bit to be changed. */
-#if 0 /* XXX
-       * As the mask bit is the only defined bit in the word, and as the
-       * host MSI-X code doesn't preserve the other bits anyway, doing
-       * this is pointless. So for now just discard the write (also
-       * saving us from having to determine the matching irq_desc).
-       */
+    msi_desc = virt_to_msi_desc(entry->pdev, virt);
+    if ( !msi_desc || msi_desc->irq < 0 )
+        goto out;
+    
+    desc = irq_to_desc(msi_desc->irq);
+    if ( !desc )
+        goto out;
+
     spin_lock_irqsave(&desc->lock, flags);
+
+    if ( !desc->msi_desc )
+        goto unlock;
+
+    ASSERT(msi_desc == desc->msi_desc);
+   
     orig = readl(virt);
-    val &= ~PCI_MSIX_VECTOR_BITMASK;
-    val |= orig & PCI_MSIX_VECTOR_BITMASK;
+
+    /*
+     * Do not allow guest to modify MSI-X control bit if it is masked 
+     * by Xen. We'll only handle the case where Xen thinks that
+     * bit is unmasked, but hardware has silently masked the bit
+     * (in case of SR-IOV VF reset, etc). On the other hand, if Xen 
+     * thinks that the bit is masked, but it's really not, 
+     * we log a warning.
+     */
+    if ( msi_desc->msi_attrib.masked )
+    {
+        if ( !(orig & PCI_MSIX_VECTOR_BITMASK) )
+            printk(XENLOG_WARNING "MSI-X control bit is unmasked when"
+                   " it is expected to be masked [%04x:%02x:%02x.%u]\n", 
+                   entry->pdev->seg, entry->pdev->bus,
+                   PCI_SLOT(entry->pdev->devfn), 
+                   PCI_FUNC(entry->pdev->devfn));
+
+        goto unlock;
+    }
+
+    /*
+     * The mask bit is the only defined bit in the word. But we 
+     * ought to preserve the reserved bits. Clearing the reserved 
+     * bits can result in undefined behaviour (see PCI Local Bus
+     * Specification revision 2.3).
+     */
+    val &= PCI_MSIX_VECTOR_BITMASK;
+    val |= (orig & ~PCI_MSIX_VECTOR_BITMASK);
     writel(val, virt);
-    spin_unlock_irqrestore(&desc->lock, flags);
-#endif
 
+unlock:
+    spin_unlock_irqrestore(&desc->lock, flags);
     r = X86EMUL_OKAY;
+
 out:
     rcu_read_unlock(&msixtbl_rcu_lock);
     return r;
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog