[Xen-changelog] [xen stable-4.6] x86/vMSI-X: avoid missing first unmask of vectors

[patchbot@xxxxxxx]

commit 3b412a9afd467fcfa434052b68a6eac607705968
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Mon May 9 12:56:29 2016 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon May 9 12:56:29 2016 +0200

    x86/vMSI-X: avoid missing first unmask of vectors
    
    Recent changes to Linux result in there just being a single unmask
    operation prior to expecting the first interrupts to arrive. However,
    we've had a chicken-and-egg problem here: Qemu invokes
    xc_domain_update_msi_irq(), ultimately leading to
    msixtbl_pt_register(), upon seeing that first unmask operation. Yet
    for msixtbl_range() to return true (in order to msixtbl_write() to get
    invoked at all) msixtbl_pt_register() must have completed.
    
    Deal with this by snooping suitable writes in msixtbl_range() and
    triggering the invocation of msix_write_completion() from
    msixtbl_pt_register() when that happens in the context of a still in
    progress vector control field write.
    
    Note that the seemingly unrelated deletion of the redundant
    irq_desc->msi_desc check in msixtbl_pt_register() is to make clear to
    any compiler version used that the "msi_desc" local variable isn't
    being used uninitialized. (Doing the same in msixtbl_pt_unregister() is
    just for consistency reasons.)
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
    
    x86/vMSI-X: write snoops should ignore hvm_mmio_internal() requests
    
    Those aren't actual I/O requests (and hence are of no interest here
    anyway). Since they don't get copied into struct vcpu, looking at that
    copy reads whatever was left there. Use the state of the request to
    determine its validity.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
    
    x86/vMSI-X: add further checks to snoop logic
    
    msixtbl_range(), as any other MMIO ->check() handlers, may get called
    with other than the base address of an access - avoid the snoop logic
    considering those.
    
    Also avoid considering vCPU-s not blocked in the hypervisor in
    msixtbl_pt_register(), just to be on the safe side.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
    master commit: 3a6222bd57209d4c2f098d61ace042370a9b0a96
    master date: 2016-04-26 11:52:22 +0200
    master commit: 9772c480a71ad38cc2c342e4c2e78c2475de7268
    master date: 2016-04-26 16:53:36 +0200
    master commit: de8627767968d84385648b399317b1b55323bc15
    master date: 2016-04-28 15:10:22 +0200
---
 xen/arch/x86/hvm/vmsi.c        | 43 +++++++++++++++++++++++++++++++++++-------
 xen/include/asm-x86/hvm/vcpu.h |  1 +
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 4372c50..fd4f3a2 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -341,7 +341,23 @@ static int msixtbl_range(struct vcpu *v, unsigned long 
addr)
     desc = msixtbl_addr_to_desc(msixtbl_find_entry(v, addr), addr);
     rcu_read_unlock(&msixtbl_rcu_lock);
 
-    return !!desc;
+    if ( desc )
+        return 1;
+
+    if ( (addr & (PCI_MSIX_ENTRY_SIZE - 1)) ==
+         PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET )
+    {
+        const ioreq_t *r = &v->arch.hvm_vcpu.hvm_io.io_req;
+
+        if ( r->state != STATE_IOREQ_READY || r->addr != addr )
+            return 0;
+        ASSERT(r->type == IOREQ_TYPE_COPY);
+        if ( r->dir == IOREQ_WRITE && r->size == 4 && !r->data_is_ptr
+             && !(r->data & PCI_MSIX_VECTOR_BITMASK) )
+            v->arch.hvm_vcpu.hvm_io.msix_snoop_address = addr;
+    }
+
+    return 0;
 }
 
 static const struct hvm_mmio_ops msixtbl_mmio_ops = {
@@ -407,9 +423,6 @@ int msixtbl_pt_register(struct domain *d, struct pirq 
*pirq, uint64_t gtable)
         return r;
     }
 
-    if ( !irq_desc->msi_desc )
-        goto out;
-
     msi_desc = irq_desc->msi_desc;
     if ( !msi_desc )
         goto out;
@@ -434,6 +447,23 @@ found:
 out:
     spin_unlock_irq(&irq_desc->lock);
     xfree(new_entry);
+
+    if ( !r )
+    {
+        struct vcpu *v;
+
+        for_each_vcpu ( d, v )
+        {
+            if ( (v->pause_flags & VPF_blocked_in_xen) &&
+                 v->arch.hvm_vcpu.hvm_io.msix_snoop_address ==
+                 (gtable + msi_desc->msi_attrib.entry_nr *
+                           PCI_MSIX_ENTRY_SIZE +
+                  PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET) )
+                v->arch.hvm_vcpu.hvm_io.msix_unmask_address =
+                    v->arch.hvm_vcpu.hvm_io.msix_snoop_address;
+        }
+    }
+
     return r;
 }
 
@@ -451,9 +481,6 @@ void msixtbl_pt_unregister(struct domain *d, struct pirq 
*pirq)
     if ( !irq_desc )
         return;
 
-    if ( !irq_desc->msi_desc )
-        goto out;
-
     msi_desc = irq_desc->msi_desc;
     if ( !msi_desc )
         goto out;
@@ -510,6 +537,8 @@ void msix_write_completion(struct vcpu *v)
 {
     unsigned long ctrl_address = v->arch.hvm_vcpu.hvm_io.msix_unmask_address;
 
+    v->arch.hvm_vcpu.hvm_io.msix_snoop_address = 0;
+
     if ( !ctrl_address )
         return;
 
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index f553814..9e26416 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -85,6 +85,7 @@ struct hvm_vcpu_io {
     bool_t mmio_retry;
 
     unsigned long msix_unmask_address;
+    unsigned long msix_snoop_address;
 
     const struct g2m_ioport *g2m_ioport;
 };
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.6

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