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)
{
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
