[Xen-devel] [PATCH] x86/vMSI-X: Fix host crash when shutting down guests with MSI capable devices

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

c/s 74c6dc2d "x86/vMSI-X: defer intercept handler registration" caused MSI-X
table infrastructure not to always be initialised, but it missed one path
which needed an is-initialised check.

If a devices is passed through to a domain which is MSI capable but not MSI-X
capable, the call to msixtbl_init() is omitted, but a XEN_DOMCTL_unbind_pt_irq
hypercall still calls into msixtbl_pt_unregister().  This follows the linked
list pointer which is still NULL.

Introduce an is-initalised check to msixtbl_pt_unregister().

Furthermore, the purpose of the open-coded msixtbl_list.next check is rather
subtle.  Introduce an msixtbl_initialised() predicate instead, which makes its
purpose far more obvious.

Reported-by: Sander Eikelenboom <linux@xxxxxxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Sander Eikelenboom <linux@xxxxxxxxxxxxxx>

Sander - would you mind double checking this patch?
---
 xen/arch/x86/hvm/vmsi.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index e418b98..ef1dfff 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -166,6 +166,16 @@ struct msixtbl_entry
 
 static DEFINE_RCU_READ_LOCK(msixtbl_rcu_lock);
 
+/*
+ * MSI-X table infrastructure is dynamically initialised when an MSI-X capable
+ * device is passed through to a domain, rather than unconditionally for all
+ * domains.
+ */
+static bool msixtbl_initialised(const struct domain *d)
+{
+    return !!d->arch.hvm_domain.msixtbl_list.next;
+}
+
 static struct msixtbl_entry *msixtbl_find_entry(
     struct vcpu *v, unsigned long addr)
 {
@@ -519,7 +529,7 @@ void msixtbl_pt_unregister(struct domain *d, struct pirq 
*pirq)
     ASSERT(pcidevs_locked());
     ASSERT(spin_is_locked(&d->event_lock));
 
-    if ( !has_vlapic(d) )
+    if ( !msixtbl_initialised(d) )
         return;
 
     irq_desc = pirq_spin_lock_irq_desc(pirq, NULL);
@@ -552,7 +562,7 @@ void msixtbl_init(struct domain *d)
     struct hvm_io_handler *handler;
 
     if ( !has_hvm_container_domain(d) || !has_vlapic(d) ||
-         d->arch.hvm_domain.msixtbl_list.next )
+         msixtbl_initialised(d) )
         return;
 
     INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list);
@@ -569,7 +579,7 @@ void msixtbl_pt_cleanup(struct domain *d)
 {
     struct msixtbl_entry *entry, *temp;
 
-    if ( !d->arch.hvm_domain.msixtbl_list.next )
+    if ( !msixtbl_initialised(d) )
         return;
 
     spin_lock(&d->event_lock);
-- 
2.1.4


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