[Xen-devel] Ping: [PATCH] VT-d: drop bogus checks

["Jan Beulich" <JBeulich@xxxxxxxx>]

There were a number of cases where an "iommu" retrieved got passed to
another function before being NULL-checked. While this by itself was
not a problem as the called function did the checks, it is confusing to
the reader and redundant in several cases (particularly with NULL-
checking the return value of iommu_ir_ctrl()). Drop the redundant
checks (also ones where the sole caller of a function did the checking
already), and at once make the three similar functions proper inline
instead of extern ones (they were prototyped in the wrong header file
anyway, so would have needed touching sooner or later).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -217,13 +217,6 @@ static int remap_entry_to_ioapic_rte(
     unsigned long flags;
     struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
 
-    if ( ir_ctrl == NULL )
-    {
-        dprintk(XENLOG_ERR VTDPREFIX,
-                "remap_entry_to_ioapic_rte: ir_ctl is not ready\n");
-        return -EFAULT;
-    }
-
     if ( index < 0 || index > IREMAP_ENTRY_NR - 1 )
     {
         dprintk(XENLOG_ERR VTDPREFIX,
@@ -358,8 +351,7 @@ unsigned int io_apic_read_remap_rte(
     struct iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
     struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
 
-    if ( !iommu || !ir_ctrl || ir_ctrl->iremap_maddr == 0 ||
-        (ir_ctrl->iremap_num == 0) ||
+    if ( !ir_ctrl || !ir_ctrl->iremap_maddr || !ir_ctrl->iremap_num ||
         ( (index = apic_pin_2_ir_idx[apic][ioapic_pin]) < 0 ) )
         return __io_apic_read(apic, reg);
 
@@ -385,7 +377,7 @@ void io_apic_write_remap_rte(
     struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
     int saved_mask;
 
-    if ( !iommu || !ir_ctrl || ir_ctrl->iremap_maddr == 0 )
+    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
     {
         __io_apic_write(apic, reg, value);
         return;
@@ -475,13 +467,6 @@ static int remap_entry_to_msi_msg(
     unsigned long flags;
     struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
 
-    if ( ir_ctrl == NULL )
-    {
-        dprintk(XENLOG_ERR VTDPREFIX,
-                "remap_entry_to_msi_msg: ir_ctl == NULL");
-        return -EFAULT;
-    }
-
     remap_rte = (struct msi_msg_remap_entry *) msg;
     index = (remap_rte->address_lo.index_15 << 15) |
              remap_rte->address_lo.index_0_14;
@@ -644,7 +629,7 @@ void msi_msg_read_remap_rte(
     iommu = drhd->iommu;
 
     ir_ctrl = iommu_ir_ctrl(iommu);
-    if ( !iommu || !ir_ctrl || ir_ctrl->iremap_maddr == 0 )
+    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
         return;
 
     remap_entry_to_msi_msg(iommu, msg);
@@ -663,7 +648,7 @@ void msi_msg_write_remap_rte(
     iommu = drhd->iommu;
 
     ir_ctrl = iommu_ir_ctrl(iommu);
-    if ( !iommu || !ir_ctrl || ir_ctrl->iremap_maddr == 0 )
+    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
         return;
 
     msi_msg_to_remap_entry(iommu, pdev, msi_desc, msg);
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -153,21 +153,6 @@ static void __init free_intel_iommu(stru
     xfree(intel);
 }
 
-struct qi_ctrl *iommu_qi_ctrl(struct iommu *iommu)
-{
-    return iommu ? &iommu->intel->qi_ctrl : NULL;
-}
-
-struct ir_ctrl *iommu_ir_ctrl(struct iommu *iommu)
-{
-    return iommu ? &iommu->intel->ir_ctrl : NULL;
-}
-
-struct iommu_flush *iommu_get_flush(struct iommu *iommu)
-{
-    return iommu ? &iommu->intel->flush : NULL;
-}
-
 static int iommus_incoherent;
 static void __iommu_flush_cache(void *addr, unsigned int size)
 {
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -20,7 +20,7 @@
 #ifndef _INTEL_IOMMU_H_
 #define _INTEL_IOMMU_H_
 
-#include <xen/types.h>
+#include <xen/iommu.h>
 
 /*
  * Intel IOMMU register specification per version 1.0 public spec.
@@ -510,6 +510,21 @@ struct intel_iommu {
     struct acpi_drhd_unit *drhd;
 };
 
+static inline struct qi_ctrl *iommu_qi_ctrl(struct iommu *iommu)
+{
+    return iommu ? &iommu->intel->qi_ctrl : NULL;
+}
+
+static inline struct ir_ctrl *iommu_ir_ctrl(struct iommu *iommu)
+{
+    return iommu ? &iommu->intel->ir_ctrl : NULL;
+}
+
+static inline struct iommu_flush *iommu_get_flush(struct iommu *iommu)
+{
+    return iommu ? &iommu->intel->flush : NULL;
+}
+
 #define INTEL_IOMMU_DEBUG(fmt, args...) \
     do  \
     {   \
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -267,8 +267,7 @@ static void dump_iommu_info(unsigned cha
         {
             iommu = ioapic_to_iommu(mp_ioapics[apic].mpc_apicid);
             ir_ctrl = iommu_ir_ctrl(iommu);
-            if ( !iommu || !ir_ctrl || ir_ctrl->iremap_maddr == 0 ||
-                ir_ctrl->iremap_num == 0 )
+            if ( !ir_ctrl || !ir_ctrl->iremap_maddr || !ir_ctrl->iremap_num )
                 continue;
 
             printk( "\nRedirection table of IOAPIC %x:\n", apic);
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -106,9 +106,6 @@ struct msi_desc;
 struct msi_msg;
 void msi_msg_read_remap_rte(struct msi_desc *msi_desc, struct msi_msg *msg);
 void msi_msg_write_remap_rte(struct msi_desc *msi_desc, struct msi_msg *msg);
-struct qi_ctrl *iommu_qi_ctrl(struct iommu *iommu);
-struct ir_ctrl *iommu_ir_ctrl(struct iommu *iommu);
-struct iommu_flush *iommu_get_flush(struct iommu *iommu);
 void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq);
 struct hvm_irq_dpci *domain_get_irq_dpci(const struct domain *);
 void free_hvm_irq_dpci(struct hvm_irq_dpci *dpci);

Attachment: VT-d-drop-bogus-checks.patch
Description: Text document

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