On Tue, Mar 10, 2020 at 10:12:24AM +0100, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@xxxxxxx> > > Make use of dev_iommu_priv_set/get() functions and simplify the code > where possible with this change. > > Tested-by: Will Deacon <will@xxxxxxxxxx> # arm-smmu > Signed-off-by: Joerg Roedel <jroedel@xxxxxxx> > --- [...] > @@ -1467,7 +1470,7 @@ static void arm_smmu_remove_device(struct device *dev) > if (!fwspec || fwspec->ops != &arm_smmu_ops) > return; > > - cfg = fwspec->iommu_priv; > + cfg = dev_iommu_priv_get(dev); > smmu = cfg->smmu; > > ret = arm_smmu_rpm_get(smmu); > @@ -1475,23 +1478,22 @@ static void arm_smmu_remove_device(struct device *dev) > return; > > iommu_device_unlink(&smmu->iommu, dev); > - arm_smmu_master_free_smes(fwspec); > + arm_smmu_master_free_smes(dev); > > arm_smmu_rpm_put(smmu); > > iommu_group_remove_device(dev); > - kfree(fwspec->iommu_priv); > iommu_fwspec_free(dev); > + kfree(cfg); nit: cfg is allocated after fwspec so it might be cleaner to free cfg before fwspec. But more importantly, should we clear the private data here and in the other drivers, by calling dev_iommu_priv_set(dev, NULL) from remove_device()? We are leaving stale pointers in dev->iommu and I think some of the drivers could end up reusing them. Thanks, Jean
