Re: [PATCH 1/2] iommu: Statically set module owner

2021-03-25 Thread Will Deacon
On Fri, Mar 19, 2021 at 12:52:01PM +, Robin Murphy wrote:
> It happens that the 3 drivers which first supported being modular are
> also ones which play games with their pgsize_bitmap, so have non-const
> iommu_ops where dynamically setting the owner manages to work out OK.
> However, it's less than ideal to force that upon all drivers which want
> to be modular - like the new sprd-iommu driver which now has a potential
> bug in that regard - so let's just statically set the module owner and
> let ops remain const wherever possible.
> 
> Signed-off-by: Robin Murphy 
> ---
> 
> This is something I hadn't got round to sending earlier, so now rebased
> onto iommu/next to accommodate the new driver :)
> 
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
>  drivers/iommu/arm/arm-smmu/arm-smmu.c   | 1 +
>  drivers/iommu/sprd-iommu.c  | 1 +
>  drivers/iommu/virtio-iommu.c| 1 +
>  include/linux/iommu.h   | 9 +
>  5 files changed, 5 insertions(+), 8 deletions(-)

Acked-by: Will Deacon 

Will


Re: [PATCH 1/2] iommu: Statically set module owner

2021-03-19 Thread Christoph Hellwig
On Fri, Mar 19, 2021 at 12:52:01PM +, Robin Murphy wrote:
> It happens that the 3 drivers which first supported being modular are
> also ones which play games with their pgsize_bitmap, so have non-const
> iommu_ops where dynamically setting the owner manages to work out OK.
> However, it's less than ideal to force that upon all drivers which want
> to be modular - like the new sprd-iommu driver which now has a potential
> bug in that regard - so let's just statically set the module owner and
> let ops remain const wherever possible.
> 
> Signed-off-by: Robin Murphy 
> ---
> 
> This is something I hadn't got round to sending earlier, so now rebased
> onto iommu/next to accommodate the new driver :)

Ah, nice.  That __iommu_device_set_ops dance always confused me.

Reviewed-by: Christoph Hellwig 


[PATCH 1/2] iommu: Statically set module owner

2021-03-19 Thread Robin Murphy
It happens that the 3 drivers which first supported being modular are
also ones which play games with their pgsize_bitmap, so have non-const
iommu_ops where dynamically setting the owner manages to work out OK.
However, it's less than ideal to force that upon all drivers which want
to be modular - like the new sprd-iommu driver which now has a potential
bug in that regard - so let's just statically set the module owner and
let ops remain const wherever possible.

Signed-off-by: Robin Murphy 
---

This is something I hadn't got round to sending earlier, so now rebased
onto iommu/next to accommodate the new driver :)

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
 drivers/iommu/arm/arm-smmu/arm-smmu.c   | 1 +
 drivers/iommu/sprd-iommu.c  | 1 +
 drivers/iommu/virtio-iommu.c| 1 +
 include/linux/iommu.h   | 9 +
 5 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 8594b4a83043..b82000519af6 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2632,6 +2632,7 @@ static struct iommu_ops arm_smmu_ops = {
.sva_unbind = arm_smmu_sva_unbind,
.sva_get_pasid  = arm_smmu_sva_get_pasid,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
+   .owner  = THIS_MODULE,
 };
 
 /* Probing and initialisation functions */
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index d8c6bfde6a61..11ca963c4b93 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1638,6 +1638,7 @@ static struct iommu_ops arm_smmu_ops = {
.put_resv_regions   = generic_iommu_put_resv_regions,
.def_domain_type= arm_smmu_def_domain_type,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
+   .owner  = THIS_MODULE,
 };
 
 static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
index 7100ed17dcce..024a0cdd26a6 100644
--- a/drivers/iommu/sprd-iommu.c
+++ b/drivers/iommu/sprd-iommu.c
@@ -436,6 +436,7 @@ static const struct iommu_ops sprd_iommu_ops = {
.device_group   = sprd_iommu_device_group,
.of_xlate   = sprd_iommu_of_xlate,
.pgsize_bitmap  = ~0UL << SPRD_IOMMU_PAGE_SHIFT,
+   .owner  = THIS_MODULE,
 };
 
 static const struct of_device_id sprd_iommu_of_match[] = {
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 2bfdd5734844..594ed827e944 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -945,6 +945,7 @@ static struct iommu_ops viommu_ops = {
.get_resv_regions   = viommu_get_resv_regions,
.put_resv_regions   = generic_iommu_put_resv_regions,
.of_xlate   = viommu_of_xlate,
+   .owner  = THIS_MODULE,
 };
 
 static int viommu_init_vqs(struct viommu_dev *viommu)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5e7fe519430a..dce8c5e12ea0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -379,19 +379,12 @@ int  iommu_device_link(struct iommu_device   *iommu, 
struct device *link);
 void iommu_device_unlink(struct iommu_device *iommu, struct device *link);
 int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain);
 
-static inline void __iommu_device_set_ops(struct iommu_device *iommu,
+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)
 {
-- 
2.21.0.dirty