[PATCH] x86/physdev: Call xsm_unmap_domain_irq earlier

[Jason Andryuk <jandryuk@xxxxxxxxx>]

Pull the XSM check up out of unmap_domain_pirq into physdev_map_pirq.

xsm_unmap_domain_irq was seen denying unmap_domain_pirq when called from
complete_domain_destroy as an RCU callback.  The source context was an
unexpected, random domain.  Since this is a xen-internal operation,
going through the XSM hook is inapproriate.

Move the XSM hook up into physdev_unmap_pirq, which is the
guest-accessible path.  This requires moving some of the sanity check
upwards as well since the hook needs the additional data to make its
decision.  Since complete_domain_destroy still calls unmap_domain_pirq,
replace the moved runtime checking with assert.  Only valid pirqs should
make their way into unmap_domain_pirq from complete_domain_destroy.

This is mostly code movement, but one style change is to pull `irq =
info->arch.irq` out of the if condition.

Label done is now unused and removed.

Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
---
unmap_domain_pirq is also called in vioapic_hwdom_map_gsi and
vpci_msi_disable.  vioapic_hwdom_map_gsi is a cleanup path after going
through map_domain_pirq, and I don't think the vpci code is directly
guest-accessible.  So I think those are okay, but I not familiar with
that code.  Hence, I am highlighting it.

 xen/arch/x86/irq.c     | 31 +++++++-----------------------
 xen/arch/x86/physdev.c | 43 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 285ac399fb..ddd3194fba 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2310,41 +2310,25 @@ int unmap_domain_pirq(struct domain *d, int pirq)
     struct pirq *info;
     struct msi_desc *msi_desc = NULL;
 
-    if ( (pirq < 0) || (pirq >= d->nr_pirqs) )
-        return -EINVAL;
-
+    ASSERT(pirq >= 0);
+    ASSERT(pirq < d->nr_pirqs);
     ASSERT(pcidevs_locked());
     ASSERT(spin_is_locked(&d->event_lock));
 
     info = pirq_info(d, pirq);
-    if ( !info || (irq = info->arch.irq) <= 0 )
-    {
-        dprintk(XENLOG_G_ERR, "dom%d: pirq %d not mapped\n",
-                d->domain_id, pirq);
-        ret = -EINVAL;
-        goto done;
-    }
+    ASSERT(info);
+
+    irq = info->arch.irq;
+    ASSERT(irq > 0);
 
     desc = irq_to_desc(irq);
     msi_desc = desc->msi_desc;
     if ( msi_desc && msi_desc->msi_attrib.type == PCI_CAP_ID_MSI )
     {
-        if ( msi_desc->msi_attrib.entry_nr )
-        {
-            printk(XENLOG_G_ERR
-                   "dom%d: trying to unmap secondary MSI pirq %d\n",
-                   d->domain_id, pirq);
-            ret = -EBUSY;
-            goto done;
-        }
+        ASSERT(msi_desc->msi_attrib.entry_nr == 0);
         nr = msi_desc->msi.nvec;
     }
 
-    ret = xsm_unmap_domain_irq(XSM_HOOK, d, irq,
-                               msi_desc ? msi_desc->dev : NULL);
-    if ( ret )
-        goto done;
-
     forced_unbind = pirq_guest_force_unbind(d, info);
     if ( forced_unbind )
         dprintk(XENLOG_G_WARNING, "dom%d: forcing unbind of pirq %d\n",
@@ -2405,7 +2389,6 @@ int unmap_domain_pirq(struct domain *d, int pirq)
     if (msi_desc)
         msi_free_irq(msi_desc);
 
- done:
     return ret;
 }
 
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 2ddcf44f33..a5ed257dca 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -140,8 +140,11 @@ int physdev_map_pirq(domid_t domid, int type, int *index, 
int *pirq_p,
 
 int physdev_unmap_pirq(domid_t domid, int pirq)
 {
+    struct msi_desc *msi_desc;
+    struct irq_desc *desc;
+    struct pirq *info;
     struct domain *d;
-    int ret = 0;
+    int irq, ret = 0;
 
     d = rcu_lock_domain_by_any_id(domid);
     if ( d == NULL )
@@ -162,9 +165,47 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
             goto free_domain;
     }
 
+    if ( (pirq < 0) || (pirq >= d->nr_pirqs) ) {
+        ret = -EINVAL;
+        goto free_domain;
+    }
+
     pcidevs_lock();
     spin_lock(&d->event_lock);
+
+    info = pirq_info(d, pirq);
+    irq = info ? info->arch.irq : 0;
+    if ( !info || irq <= 0 )
+    {
+        dprintk(XENLOG_G_ERR, "dom%d: pirq %d not mapped\n",
+                d->domain_id, pirq);
+        ret = -EINVAL;
+        goto unlock;
+    }
+
+    desc = irq_to_desc(irq);
+    msi_desc = desc->msi_desc;
+    if ( msi_desc && msi_desc->msi_attrib.type == PCI_CAP_ID_MSI )
+    {
+        if ( msi_desc->msi_attrib.entry_nr )
+        {
+            printk(XENLOG_G_ERR
+                   "dom%d: trying to unmap secondary MSI pirq %d\n",
+                   d->domain_id, pirq);
+            ret = -EBUSY;
+            goto unlock;
+        }
+    }
+
+    ret = xsm_unmap_domain_irq(XSM_HOOK, d, irq,
+                               msi_desc ? msi_desc->dev : NULL);
+    if ( ret )
+        goto unlock;
+
     ret = unmap_domain_pirq(d, pirq);
+
+ unlock:
+
     spin_unlock(&d->event_lock);
     pcidevs_unlock();
 
-- 
2.35.1