>>> On 08.05.15 at 11:07, <feng...@intel.com> wrote: > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -2044,6 +2044,7 @@ static int init_vtd_hw(void) > if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL ) > { > iommu_intremap = 0; > + iommu_intpost = 0; > dprintk(XENLOG_ERR VTDPREFIX, > "ioapic_to_iommu: ioapic %#x (id: %#x) is NULL! " > "Will not try to enable Interrupt Remapping.\n", > @@ -2060,6 +2061,7 @@ static int init_vtd_hw(void) > if ( enable_intremap(iommu, 0) != 0 ) > { > iommu_intremap = 0; > + iommu_intpost = 0; > dprintk(XENLOG_WARNING VTDPREFIX, > "Interrupt Remapping not enabled\n"); >
Instead of sprinkling these around, wouldn't it be possible to clear the variable more centrally upon !iommu_intremap (assuming the clearing in generic IOMMU code doesn't already suffice)? > @@ -2160,7 +2162,13 @@ int __init intel_vtd_setup(void) > iommu_qinval = 0; > > if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) ) > + { > iommu_intremap = 0; > + iommu_intpost = 0; > + } Just like in the previous patch, I think this would better be folded into the next if(): > + if ( iommu_intpost && !cap_intr_post(iommu->cap) ) > + iommu_intpost = 0; Please try to avoid copying odd code: There's no point (afaics) for the if to check iommu_intpost. Together with the above I think what you want is if ( !iommu_intremap || !cap_intr_post(iommu->cap) ) iommu_intpost = 0; Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel