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

[Sander Eikelenboom <linux@xxxxxxxxxxxxxx>]

On July 21, 2016 12:18:37 PM GMT+02:00, Andrew Cooper 
<andrew.cooper3@xxxxxxxxxx> wrote:
>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?
>---

Sure, will report back tomorrow.

--
Sander

> 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);


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