[Xen-devel] [PATCH v3 1/3] IOMMU: allow MSI message to IRTE propagation to fail

["Jan Beulich" <JBeulich@xxxxxxxx>]

With the need to allocate multiple contiguous IRTEs for multi-vector
MSI, the chance of failure here increases. While on the AMD side
there's no allocation of IRTEs at present at all (and hence no way for
this allocation to fail, which is going to change with a later patch in
this series), VT-d already ignores an eventual error here, which this
patch fixes.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
v3: Introduce _find_iommu_for_device() to take care of filtering out
    the case where an MSI is being set up for the IOMMU itself.

--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -254,13 +254,22 @@ static void hpet_msi_mask(struct irq_des
     ch->msi.msi_attrib.masked = 1;
 }
 
-static void hpet_msi_write(struct hpet_event_channel *ch, struct msi_msg *msg)
+static int hpet_msi_write(struct hpet_event_channel *ch, struct msi_msg *msg)
 {
     ch->msi.msg = *msg;
+
     if ( iommu_intremap )
-        iommu_update_ire_from_msi(&ch->msi, msg);
+    {
+        int rc = iommu_update_ire_from_msi(&ch->msi, msg);
+
+        if ( rc )
+            return rc;
+    }
+
     hpet_write32(msg->data, HPET_Tn_ROUTE(ch->idx));
     hpet_write32(msg->address_lo, HPET_Tn_ROUTE(ch->idx) + 4);
+
+    return 0;
 }
 
 static void __maybe_unused
@@ -318,12 +327,12 @@ static hw_irq_controller hpet_msi_type =
     .set_affinity   = hpet_msi_set_affinity,
 };
 
-static void __hpet_setup_msi_irq(struct irq_desc *desc)
+static int __hpet_setup_msi_irq(struct irq_desc *desc)
 {
     struct msi_msg msg;
 
     msi_compose_msg(desc, &msg);
-    hpet_msi_write(desc->action->dev_id, &msg);
+    return hpet_msi_write(desc->action->dev_id, &msg);
 }
 
 static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
@@ -347,6 +356,8 @@ static int __init hpet_setup_msi_irq(str
 
     desc->handler = &hpet_msi_type;
     ret = request_irq(ch->msi.irq, hpet_interrupt_handler, 0, "HPET", ch);
+    if ( ret >= 0 )
+        ret = __hpet_setup_msi_irq(desc);
     if ( ret < 0 )
     {
         if ( iommu_intremap )
@@ -354,7 +365,6 @@ static int __init hpet_setup_msi_irq(str
         return ret;
     }
 
-    __hpet_setup_msi_irq(desc);
     desc->msi_desc = &ch->msi;
 
     return 0;
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1938,7 +1938,14 @@ int map_domain_pirq(
         if ( desc->handler != &no_irq_type )
             dprintk(XENLOG_G_ERR, "dom%d: irq %d in use\n",
                     d->domain_id, irq);
-        setup_msi_handler(desc, msi_desc);
+
+        ret = setup_msi_irq(desc, msi_desc);
+        if ( ret )
+        {
+            spin_unlock_irqrestore(&desc->lock, flags);
+            pci_disable_msi(msi_desc);
+            goto done;
+        }
 
         if ( opt_irq_vector_map == OPT_IRQ_VECTOR_MAP_PERDEV
              && !desc->arch.used_vectors )
@@ -1954,7 +1961,6 @@ int map_domain_pirq(
         }
 
         set_domain_irq_pirq(d, irq, info);
-        setup_msi_irq(desc);
         spin_unlock_irqrestore(&desc->lock, flags);
     }
     else
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -214,14 +214,18 @@ static void read_msi_msg(struct msi_desc
         iommu_read_msi_from_ire(entry, msg);
 }
 
-static void write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
+static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 {
     entry->msg = *msg;
 
     if ( iommu_intremap )
     {
+        int rc;
+
         ASSERT(msg != &entry->msg);
-        iommu_update_ire_from_msi(entry, msg);
+        rc = iommu_update_ire_from_msi(entry, msg);
+        if ( rc )
+            return rc;
     }
 
     switch ( entry->msi_attrib.type )
@@ -264,6 +268,8 @@ static void write_msi_msg(struct msi_des
     default:
         BUG();
     }
+
+    return 0;
 }
 
 void set_msi_affinity(struct irq_desc *desc, const cpumask_t *mask)
@@ -464,19 +470,15 @@ static struct msi_desc* alloc_msi_entry(
     return entry;
 }
 
-void setup_msi_handler(struct irq_desc *desc, struct msi_desc *msidesc)
+int setup_msi_irq(struct irq_desc *desc, struct msi_desc *msidesc)
 {
+    struct msi_msg msg;
+
     desc->msi_desc = msidesc;
     desc->handler = msi_maskable_irq(msidesc) ? &pci_msi_maskable
                                               : &pci_msi_nonmaskable;
-}
-
-void setup_msi_irq(struct irq_desc *desc)
-{
-    struct msi_msg msg;
-
     msi_compose_msg(desc, &msg);
-    write_msi_msg(desc->msi_desc, &msg);
+    return write_msi_msg(msidesc, &msg);
 }
 
 int msi_free_irq(struct msi_desc *entry)
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -17,6 +17,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
  */
 
+#include <xen/err.h>
 #include <xen/sched.h>
 #include <xen/hvm/iommu.h>
 #include <asm/amd-iommu.h>
@@ -359,25 +360,35 @@ done:
     }
 }
 
-void amd_iommu_msi_msg_update_ire(
+static struct amd_iommu *_find_iommu_for_device(int seg, int bdf)
+{
+    struct amd_iommu *iommu = find_iommu_for_device(seg, bdf);
+
+    if ( iommu )
+        return iommu;
+
+    list_for_each_entry ( iommu, &amd_iommu_head, list )
+        if ( iommu->seg == seg && iommu->bdf == bdf )
+            return NULL;
+
+    AMD_IOMMU_DEBUG("No IOMMU for MSI dev = %04x:%02x:%02x.%u\n",
+                    seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
+    return ERR_PTR(-EINVAL);
+}
+
+int amd_iommu_msi_msg_update_ire(
     struct msi_desc *msi_desc, struct msi_msg *msg)
 {
     struct pci_dev *pdev = msi_desc->dev;
     int bdf, seg;
     struct amd_iommu *iommu;
 
-    if ( !iommu_intremap )
-        return;
-
     bdf = pdev ? PCI_BDF2(pdev->bus, pdev->devfn) : hpet_sbdf.bdf;
     seg = pdev ? pdev->seg : hpet_sbdf.seg;
 
-    iommu = find_iommu_for_device(seg, bdf);
-    if ( !iommu )
-    {
-        AMD_IOMMU_DEBUG("Fail to find iommu for MSI device id = %#x\n", bdf);
-        return;
-    }
+    iommu = _find_iommu_for_device(seg, bdf);
+    if ( IS_ERR_OR_NULL(iommu) )
+        return PTR_ERR(iommu);
 
     if ( msi_desc->remap_index >= 0 )
     {
@@ -395,7 +406,7 @@ void amd_iommu_msi_msg_update_ire(
     }
 
     if ( !msg )
-        return;
+        return 0;
 
     do {
         update_intremap_entry_from_msi_msg(iommu, bdf, &msi_desc->remap_index,
@@ -404,6 +415,8 @@ void amd_iommu_msi_msg_update_ire(
             break;
         bdf += pdev->phantom_stride;
     } while ( PCI_SLOT(bdf) == PCI_SLOT(pdev->devfn) );
+
+    return 0;
 }
 
 void amd_iommu_read_msi_from_ire(
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -548,18 +548,20 @@ void iommu_update_ire_from_apic(
     const struct iommu_ops *ops = iommu_get_ops();
     ops->update_ire_from_apic(apic, reg, value);
 }
-void iommu_update_ire_from_msi(
+
+int iommu_update_ire_from_msi(
     struct msi_desc *msi_desc, struct msi_msg *msg)
 {
     const struct iommu_ops *ops = iommu_get_ops();
-    ops->update_ire_from_msi(msi_desc, msg);
+    return iommu_intremap ? ops->update_ire_from_msi(msi_desc, msg) : 0;
 }
 
 void iommu_read_msi_from_ire(
     struct msi_desc *msi_desc, struct msi_msg *msg)
 {
     const struct iommu_ops *ops = iommu_get_ops();
-    ops->read_msi_from_ire(msi_desc, msg);
+    if ( iommu_intremap )
+        ops->read_msi_from_ire(msi_desc, msg);
 }
 
 unsigned int iommu_read_apic_from_ire(unsigned int apic, unsigned int reg)
--- a/xen/drivers/passthrough/vtd/extern.h
+++ b/xen/drivers/passthrough/vtd/extern.h
@@ -90,7 +90,7 @@ void io_apic_write_remap_rte(unsigned in
 struct msi_desc;
 struct msi_msg;
 void msi_msg_read_remap_rte(struct msi_desc *, struct msi_msg *);
-void msi_msg_write_remap_rte(struct msi_desc *, struct msi_msg *);
+int msi_msg_write_remap_rte(struct msi_desc *, struct msi_msg *);
 
 int intel_setup_hpet_msi(struct msi_desc *);
 
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -653,7 +653,7 @@ void msi_msg_read_remap_rte(
         remap_entry_to_msi_msg(drhd->iommu, msg);
 }
 
-void msi_msg_write_remap_rte(
+int msi_msg_write_remap_rte(
     struct msi_desc *msi_desc, struct msi_msg *msg)
 {
     struct pci_dev *pdev = msi_desc->dev;
@@ -661,8 +661,8 @@ void msi_msg_write_remap_rte(
 
     drhd = pdev ? acpi_find_matched_drhd_unit(pdev)
                 : hpet_to_drhd(msi_desc->hpet_id);
-    if ( drhd )
-        msi_msg_to_remap_entry(drhd->iommu, pdev, msi_desc, msg);
+    return drhd ? msi_msg_to_remap_entry(drhd->iommu, pdev, msi_desc, msg)
+                : -EINVAL;
 }
 
 int __init intel_setup_hpet_msi(struct msi_desc *msi_desc)
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -93,7 +93,7 @@ void *amd_iommu_alloc_intremap_table(voi
 int amd_iommu_free_intremap_table(u16 seg, struct ivrs_mappings *);
 void amd_iommu_ioapic_update_ire(
     unsigned int apic, unsigned int reg, unsigned int value);
-void amd_iommu_msi_msg_update_ire(
+int amd_iommu_msi_msg_update_ire(
     struct msi_desc *msi_desc, struct msi_msg *msg);
 void amd_iommu_read_msi_from_ire(
     struct msi_desc *msi_desc, struct msi_msg *msg);
--- a/xen/include/asm-x86/msi.h
+++ b/xen/include/asm-x86/msi.h
@@ -78,8 +78,7 @@ extern int pci_enable_msi(struct msi_inf
 extern void pci_disable_msi(struct msi_desc *desc);
 extern int pci_prepare_msix(u16 seg, u8 bus, u8 devfn, bool_t off);
 extern void pci_cleanup_msi(struct pci_dev *pdev);
-extern void setup_msi_handler(struct irq_desc *, struct msi_desc *);
-extern void setup_msi_irq(struct irq_desc *);
+extern int setup_msi_irq(struct irq_desc *, struct msi_desc *);
 extern void teardown_msi_irq(int irq);
 extern int msi_free_vector(struct msi_desc *entry);
 extern int pci_restore_msi_state(struct pci_dev *pdev);
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -106,7 +106,7 @@ struct iommu_ops {
                           u8 devfn, struct pci_dev *);
     int (*get_device_group_id)(u16 seg, u8 bus, u8 devfn);
     void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned 
int value);
-    void (*update_ire_from_msi)(struct msi_desc *msi_desc, struct msi_msg 
*msg);
+    int (*update_ire_from_msi)(struct msi_desc *msi_desc, struct msi_msg *msg);
     void (*read_msi_from_ire)(struct msi_desc *msi_desc, struct msi_msg *msg);
     unsigned int (*read_apic_from_ire)(unsigned int apic, unsigned int reg);
     int (*setup_hpet_msi)(struct msi_desc *);
@@ -120,7 +120,7 @@ struct iommu_ops {
 };
 
 void iommu_update_ire_from_apic(unsigned int apic, unsigned int reg, unsigned 
int value);
-void iommu_update_ire_from_msi(struct msi_desc *msi_desc, struct msi_msg *msg);
+int iommu_update_ire_from_msi(struct msi_desc *msi_desc, struct msi_msg *msg);
 void iommu_read_msi_from_ire(struct msi_desc *msi_desc, struct msi_msg *msg);
 unsigned int iommu_read_apic_from_ire(unsigned int apic, unsigned int reg);
 int iommu_setup_hpet_msi(struct msi_desc *);

Attachment: IOMMU-update-ire-from-msi-fail.patch
Description: Text document

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