On Thu, Jan 09, 2020 at 02:16:03PM +0000, Will Deacon wrote:
> Hi Greg,
>
> On Thu, Dec 19, 2019 at 03:44:37PM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Dec 19, 2019 at 12:03:41PM +0000, Will Deacon wrote:
> > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > > index f2223cbb5fd5..e9f94d3f7a04 100644
> > > --- a/include/linux/iommu.h
> > > +++ b/include/linux/iommu.h
> > > @@ -246,9 +246,10 @@ struct iommu_iotlb_gather {
> > > * @sva_get_pasid: Get PASID associated to a SVA handle
> > > * @page_response: handle page request response
> > > * @cache_invalidate: invalidate translation caches
> > > - * @pgsize_bitmap: bitmap of all possible supported page sizes
> > > * @sva_bind_gpasid: bind guest pasid and mm
> > > * @sva_unbind_gpasid: unbind guest pasid and mm
> > > + * @pgsize_bitmap: bitmap of all possible supported page sizes
> > > + * @owner: Driver module providing these ops
> > > */
> > > struct iommu_ops {
> > > bool (*capable)(enum iommu_cap);
> > > @@ -318,6 +319,7 @@ struct iommu_ops {
> > > int (*sva_unbind_gpasid)(struct device *dev, int pasid);
> > >
> > > unsigned long pgsize_bitmap;
> > > + struct module *owner;
> >
> > Everyone is always going to forget to set this field. I don't think you
> > even set it for all of the different iommu_ops possible in this series,
> > right?
>
> I only initialised the field for those drivers which can actually be built
> as a module, but I take your point about this being error-prone.
>
> > The "trick" we did to keep people from having to remember this is to do
> > what we did for the bus registering functions.
> >
> > Look at pci_register_driver in pci.h:
> > #define pci_register_driver(driver) \
> > __pci_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
> >
> > Then we set the .owner field in the "real" __pci_register_driver() call.
> >
> > Same thing for USB and lots, if not all, other driver register
> > functions.
> >
> > You can do the same thing here, and I would recommend it.
>
> Yes, that makes sense, cheers. Diff below. I'll send it to Joerg along
> with some other SMMU patches that have come in since the holiday.
>
> Will
>
> --->8
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 03dc97842875..e82997a705a8 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2733,7 +2733,6 @@ static struct iommu_ops arm_smmu_ops = {
> .get_resv_regions = arm_smmu_get_resv_regions,
> .put_resv_regions = arm_smmu_put_resv_regions,
> .pgsize_bitmap = -1UL, /* Restricted during device attach */
> - .owner = THIS_MODULE,
> };
>
> /* Probing and initialisation functions */
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 5ef1f2e100d7..93d332423f6f 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1623,7 +1623,6 @@ static struct iommu_ops arm_smmu_ops = {
> .get_resv_regions = arm_smmu_get_resv_regions,
> .put_resv_regions = arm_smmu_put_resv_regions,
> .pgsize_bitmap = -1UL, /* Restricted during device attach */
> - .owner = THIS_MODULE,
> };
>
> static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e9f94d3f7a04..90007c92ad2d 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -388,12 +388,19 @@ void iommu_device_sysfs_remove(struct iommu_device
> *iommu);
> int iommu_device_link(struct iommu_device *iommu, struct device *link);
> void iommu_device_unlink(struct iommu_device *iommu, struct device *link);
>
> -static inline void iommu_device_set_ops(struct iommu_device *iommu,
> - const struct iommu_ops *ops)
> +static inline void __iommu_device_set_ops(struct iommu_device *iommu,
> + const struct iommu_ops *ops)
> {
> iommu->ops = ops;
> }
>
> +#define iommu_device_set_ops(iommu, ops) \
> +do { \
> + struct iommu_ops *__ops = (struct iommu_ops *)(ops); \
> + __ops->owner = THIS_MODULE; \
> + __iommu_device_set_ops(iommu, __ops); \
> +} while (0)
> +
> static inline void iommu_device_set_fwnode(struct iommu_device *iommu,
> struct fwnode_handle *fwnode)
> {
Looks good:
Reviewed-by: Greg Kroah-Hartman <[email protected]>
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
