[Xen-devel] [PATCH v5] x86: properly handle MSI-X unmask operation from guests

["Wu, Feng" <feng.wu@xxxxxxxxx>]

patch revision history
----------------------
v1: Initial patch to handle this issue involving changing the hypercall 
interface
v2:Totally handled inside hypervisor.
v3:Change some logics of handling msi-x pending unmask operations.
v4:Some changes related to coding style according to Andrew Cooper's comments
v5:Some changes according to Jan's comments, including
        a) remove "valid" field, use "ctrl_address" itself to judge if there is 
a valid
      pending msix unmask operation
    b) Call msix_post_handler() when (p->state == STATE_IOREQ_NONE), which
      means it gets executed only upon completion of I/O

From de586050612f2fbe6fbb46065c2a84069c52009e Mon Sep 17 00:00:00 2001
From: Feng Wu <feng.wu@xxxxxxxxx>
Date: Wed, 13 Nov 2013 21:43:48 -0500
Subject: [PATCH v5] x86: properly handle MSI-X unmask operation from guests

For a pass-through device with MSI-x capability, when guest tries
to unmask the MSI-x interrupt for the passed through device, xen
doesn't clear the mask bit for MSI-x in hardware in the following
scenario, which will cause network disconnection:

1. Guest masks the MSI-x interrupt
2. Guest updates the address and data for it
3. Guest unmasks the MSI-x interrupt (This is the problematic step)

In the step #3 above, Xen doesn't handle it well. When guest tries
to unmask MSI-X interrupt, it traps to Xen, Xen just returns to Qemu
if it notices that address or data has been modified by guest before,
then Qemu will update Xen with the latest value of address/data by
hypercall. However, in this whole process, the MSI-X interrupt unmask
operation is missing, which means Xen doesn't clear the mask bit in
hardware for the MSI-X interrupt, so it remains disabled, that is why
it loses the network connection.

This patch fixes this issue.

Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
---
 xen/arch/x86/hvm/io.c        |    4 ++++
 xen/arch/x86/hvm/vmsi.c      |   15 +++++++++++++++
 xen/include/asm-x86/domain.h |    7 +++++++
 xen/include/xen/pci.h        |    1 +
 4 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index deb7b92..bd751d1 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -298,7 +298,11 @@ void hvm_io_assist(ioreq_t *p)
     }

     if ( p->state == STATE_IOREQ_NONE )
+    {
+        if ( !msix_post_handler(curr) )
+            gdprintk(XENLOG_WARNING, "msix_post_handler error\n");
         vcpu_end_shutdown_deferral(curr);
+    }
 }

 static int dpci_ioport_read(uint32_t mport, ioreq_t *p)
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 4826b4a..7178de9 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -293,7 +293,10 @@ static int msixtbl_write(struct vcpu *v, unsigned long 
address,

     /* exit to device model if address/data has been modified */
     if ( test_and_clear_bit(nr_entry, &entry->table_flags) )
+    {
+        v->arch.pending_msix_unmask.ctrl_address = address;
         goto out;
+    }

     virt = msixtbl_addr_to_virt(entry, address);
     if ( !virt )
@@ -528,3 +531,15 @@ void msixtbl_pt_cleanup(struct domain *d)
     spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock);
     local_irq_restore(flags);
 }
+
+bool_t msix_post_handler(struct vcpu *v)
+{
+    unsigned long ctrl_address = v->arch.pending_msix_unmask.ctrl_address;
+
+    if ( ctrl_address == 0 )
+        return 1;
+
+    v->arch.pending_msix_unmask.ctrl_address = 0;
+    return msixtbl_write(v, ctrl_address, 4, 0) == X86EMUL_OKAY;
+}
+
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 9d39061..8292fdb 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -375,6 +375,11 @@ struct pv_vcpu
     spinlock_t shadow_ldt_lock;
 };

+struct pending_msix_unmask_info
+{
+    unsigned long ctrl_address;
+};
+
 struct arch_vcpu
 {
     /*
@@ -439,6 +444,8 @@ struct arch_vcpu

     /* A secondary copy of the vcpu time info. */
     XEN_GUEST_HANDLE(vcpu_time_info_t) time_info_guest;
+
+    struct pending_msix_unmask_info pending_msix_unmask;
 } __cacheline_aligned;

 /* Shorthands to improve code legibility. */
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index cadb525..15050c8 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -147,5 +147,6 @@ struct pirq;
 int msixtbl_pt_register(struct domain *, struct pirq *, uint64_t gtable);
 void msixtbl_pt_unregister(struct domain *, struct pirq *);
 void msixtbl_pt_cleanup(struct domain *d);
+bool_t msix_post_handler(struct vcpu *v);

 #endif /* __XEN_PCI_H__ */
--
1.7.1

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