Messages in this thread | ![/](/images/icornerl.gif) | | Date | Fri, 10 Mar 2023 22:04:18 +0000 | Subject | Re: [PATCH v3 1/6] ARM/dma-mapping: Add arm_iommu_release_device() | From | Robin Murphy <> |
| |
On 2023-03-06 02:57, Lu Baolu wrote: > It is like arm_iommu_detach_device() except it handles the special case > of being called under an iommu driver's release operation. In this case > the driver must have already detached the device from any attached > domain before calling this function. > > Replace arm_iommu_detach_device() with arm_iommu_release_device() in the > release path of the ipmmu-vmsa driver. > > The bonus is that it also removes a obstacle of arm_iommu_detach_device() > re-entering the iommu core during release_device. With this removed, the > iommu core code could be simplified a lot. > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> > --- > arch/arm/include/asm/dma-iommu.h | 1 + > arch/arm/mm/dma-mapping.c | 25 +++++++++++++++++++++++++ > drivers/iommu/ipmmu-vmsa.c | 15 +++++++++++++-- > 3 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h > index fe9ef6f79e9c..ea7198a17861 100644 > --- a/arch/arm/include/asm/dma-iommu.h > +++ b/arch/arm/include/asm/dma-iommu.h > @@ -31,6 +31,7 @@ void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping); > int arm_iommu_attach_device(struct device *dev, > struct dma_iommu_mapping *mapping); > void arm_iommu_detach_device(struct device *dev); > +void arm_iommu_release_device(struct device *dev); > > #endif /* __KERNEL__ */ > #endif > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > index 8bc01071474a..96fa27f4a164 100644 > --- a/arch/arm/mm/dma-mapping.c > +++ b/arch/arm/mm/dma-mapping.c > @@ -1682,6 +1682,31 @@ int arm_iommu_attach_device(struct device *dev, > } > EXPORT_SYMBOL_GPL(arm_iommu_attach_device); > > +/** > + * arm_iommu_release_device > + * @dev: valid struct device pointer > + * > + * This is like arm_iommu_detach_device() except it handles the special > + * case of being called under an iommu driver's release operation. In this > + * case the driver must have already detached the device from any attached > + * domain before calling this function. > + */ > +void arm_iommu_release_device(struct device *dev) > +{ > + struct dma_iommu_mapping *mapping; > + > + mapping = to_dma_iommu_mapping(dev); > + if (!mapping) { > + dev_warn(dev, "Not attached\n"); > + return; > + } > + > + kref_put(&mapping->kref, release_iommu_mapping); > + to_dma_iommu_mapping(dev) = NULL; > + set_dma_ops(dev, NULL); > +} > +EXPORT_SYMBOL_GPL(arm_iommu_release_device); > + > /** > * arm_iommu_detach_device > * @dev: valid struct device pointer > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c > index bdf1a4e5eae0..de9c74cf61a4 100644 > --- a/drivers/iommu/ipmmu-vmsa.c > +++ b/drivers/iommu/ipmmu-vmsa.c > @@ -30,7 +30,7 @@ > #define arm_iommu_create_mapping(...) NULL > #define arm_iommu_attach_device(...) -ENODEV > #define arm_iommu_release_mapping(...) do {} while (0) > -#define arm_iommu_detach_device(...) do {} while (0) > +#define arm_iommu_release_device(...) do {} while (0) > #endif > > #define IPMMU_CTX_MAX 16U > @@ -820,7 +820,18 @@ static void ipmmu_probe_finalize(struct device *dev) > > static void ipmmu_release_device(struct device *dev) > { > - arm_iommu_detach_device(dev); > + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > + struct ipmmu_vmsa_device *mmu = to_ipmmu(dev); > + unsigned int i; > + > + for (i = 0; i < fwspec->num_ids; ++i) { > + unsigned int utlb = fwspec->ids[i]; > + > + ipmmu_imuctr_write(mmu, utlb, 0); > + mmu->utlb_ctx[utlb] = IPMMU_CTX_INVALID; > + } > + > + arm_iommu_release_device(dev);
Just call the existing arm_iommu_release_mapping(). Look at where the BUS_NOTIFY_REMOVED_DEVICE point is in device_del(); this device is not coming back. Zeroing out pointers and testing for a condition which cannot be true by construction is simply a waste of time and code.
Thanks, Robin.
> } > > static struct iommu_group *ipmmu_find_group(struct device *dev)
| ![\](/images/icornerr.gif) |