All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Andryuk <jandryuk@gmail.com>
To: xen-devel@lists.xenproject.org
Cc: "Jason Andryuk" <jandryuk@gmail.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Daniel P. Smith" <dpsmith.dev@gmail.com>
Subject: [PATCH] x86/physdev: Call xsm_unmap_domain_irq earlier
Date: Fri, 25 Mar 2022 10:18:26 -0400	[thread overview]
Message-ID: <20220325141826.16245-1-jandryuk@gmail.com> (raw)

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@gmail.com>
---
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



             reply	other threads:[~2022-03-25 14:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-25 14:18 Jason Andryuk [this message]
2022-03-28 11:40 ` [PATCH] x86/physdev: Call xsm_unmap_domain_irq earlier Roger Pau Monné
2022-03-28 12:44   ` Jan Beulich
2022-03-29 12:40     ` Jason Andryuk
2022-03-28 15:36 ` Daniel P. Smith

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220325141826.16245-1-jandryuk@gmail.com \
    --to=jandryuk@gmail.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=dpsmith.dev@gmail.com \
    --cc=jbeulich@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.