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@suse.com>
--- 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);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
On 15/10/2012 15:45, "Jan Beulich" <JBeulich@suse.com> wrote:> 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@suse.com>Yes, I like this sort of cleanup. Acked-by: Keir Fraser <keir@xen.org>> --- 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); > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel