On 10/02/17 16:11, Joerg Roedel wrote: > On Fri, Feb 10, 2017 at 04:03:07PM +0000, Robin Murphy wrote: >> Yeah, on reflection explicit initialisation is certainly easier to read >> than a bunch of arguments handled implicitly by register(), but then >> from that angle, even more clear would be to simply have the drivers >> write the relevant struct members directly - I'd be quite happy with >> that, and we then don't have to add another setter to iommu.h for every >> new struct member (and risk it looking like Java code...) > > Yeah, that was my first approach. But there is the Intel VT-d anomaly, > where a part of the driver can be built-in (dmar.c) with > CONFIG_IOMMU_API=N. In this case 'struct iommu_device' is empty, and > trying to access the members directly doesn't compile anymore. > > I have to look if this anomaly could be removed, then it is probably the > best to set the struct members directly without wrapper functions.
Ah, I hadn't managed to spot that - I assume there probably is some valid edge case for wanting x2APIC functionality without DMA remapping which prevents us from just adding the dependency. Looking at the code, though, that situation does seem to rely on the call never actually executing at runtime - not only is it conditional on a static variable which is only ever set by non-present code, it would fail the probe if it were called - so I think it would be perfectly reasonable to just address that particular problem as below (untested, but if it lets us get rid of the dummy !IOMMU_API definitions of the registration functions I'd say we've done the right thing). Robin. ----->8----- diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c index 8ccbd7023194..161641caff79 100644 --- a/drivers/iommu/dmar.c +++ b/drivers/iommu/dmar.c @@ -1077,6 +1077,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd) raw_spin_lock_init(&iommu->register_lock); +#ifdef CONFIG_IOMMU_API if (intel_iommu_enabled) { iommu->iommu_dev = iommu_device_create(NULL, iommu, intel_iommu_groups, @@ -1087,6 +1088,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd) goto err_unmap; } } +#endif drhd->iommu = iommu; _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu