[PATCH V3 0/2] Add support for Renesas R-Car S4 IPMMU and other misc changes (remaining 2 patches)

[PATCH V3 0/2] Add support for Renesas R-Car S4 IPMMU and other misc changes (remaining 2 patches)

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Hello all.

You can find the V1-V2 patch series at [1]-[2].

The first 8 patches (prereq work + R-Car S4 IPMMU support) have been already committed.
These are remaining 2 patches which include misc changes.

[1] https://lore.kernel.org/all/1638035505-16931-1-git-send-email-olekstysh@gmail.com/
[2] https://lore.kernel.org/all/1640034957-19764-1-git-send-email-olekstysh@gmail.com/

Oleksandr Tyshchenko (2):
  iommu/ipmmu-vmsa: Use refcount for the micro-TLBs
  iommu/arm: Remove code duplication in all IOMMU drivers

 xen/drivers/passthrough/arm/iommu.c      |  7 +++++++
 xen/drivers/passthrough/arm/ipmmu-vmsa.c | 36 ++++++++++++++------------------
 xen/drivers/passthrough/arm/smmu-v3.c    | 17 +--------------
 xen/drivers/passthrough/arm/smmu.c       | 17 +--------------
 4 files changed, 25 insertions(+), 52 deletions(-)

-- 
2.7.4

[PATCH V3 1/2] iommu/ipmmu-vmsa: Use refcount for the micro-TLBs

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Reference-count the micro-TLBs as several bus masters can be
connected to the same micro-TLB (and drop TODO comment).
This wasn't an issue so far, since the platform devices
(this driver deals with) get assigned/deassigned together during
domain creation/destruction. But, in order to support PCI devices
(which are hot-pluggable) in the near future we will need to
take care of.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
Changes V1 -> V2:
   - add R-b
   - add ASSERT() in ipmmu_utlb_disable()

Changes V2 -> V3:
   - add comment to the code
---
 xen/drivers/passthrough/arm/ipmmu-vmsa.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
index 649b9f6..c912120 100644
--- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -127,6 +127,7 @@ struct ipmmu_vmsa_device {
     spinlock_t lock;    /* Protects ctx and domains[] */
     DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
     struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
+    unsigned int utlb_refcount[IPMMU_UTLB_MAX];
     const struct ipmmu_features *features;
 };
 
@@ -468,12 +469,17 @@ static int ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
     }
 
     /*
-     * TODO: Reference-count the micro-TLB as several bus masters can be
-     * connected to the same micro-TLB.
+     * Reference-count the micro-TLBs as several bus masters can be connected
+     * to the same micro-TLB. The platform devices get assigned/deassigned
+     * together during domain creation/destruction. The PCI devices which use
+     * the same micro-TLB must also be hot-(un)plugged together.
      */
-    ipmmu_imuasid_write(mmu, utlb, 0);
-    ipmmu_imuctr_write(mmu, utlb, imuctr |
-                       IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN);
+    if ( mmu->utlb_refcount[utlb]++ == 0 )
+    {
+        ipmmu_imuasid_write(mmu, utlb, 0);
+        ipmmu_imuctr_write(mmu, utlb, imuctr |
+                           IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN);
+    }
 
     return 0;
 }
@@ -484,7 +490,10 @@ static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
 {
     struct ipmmu_vmsa_device *mmu = domain->mmu;
 
-    ipmmu_imuctr_write(mmu, utlb, 0);
+    ASSERT(mmu->utlb_refcount[utlb] > 0);
+
+    if ( --mmu->utlb_refcount[utlb] == 0 )
+        ipmmu_imuctr_write(mmu, utlb, 0);
 }
 
 /* Domain/Context Management */
-- 
2.7.4

[PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

All IOMMU drivers on Arm perform almost the same generic actions in
hwdom_init callback. Move this code to common arch_iommu_hwdom_init()
in order to get rid of code duplication.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
Changes V1 -> V2:
   - add R-b

Changes V2 -> V3:
   - drop platform specific *_iommu_hwdom_init(), make .hwdom_init
     to directly point to the common arch_iommu_hwdom_init()
---
 xen/drivers/passthrough/arm/iommu.c      |  7 +++++++
 xen/drivers/passthrough/arm/ipmmu-vmsa.c | 15 +--------------
 xen/drivers/passthrough/arm/smmu-v3.c    | 17 +----------------
 xen/drivers/passthrough/arm/smmu.c       | 17 +----------------
 4 files changed, 10 insertions(+), 46 deletions(-)

diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
index ee653a9..fc45318 100644
--- a/xen/drivers/passthrough/arm/iommu.c
+++ b/xen/drivers/passthrough/arm/iommu.c
@@ -134,6 +134,13 @@ void arch_iommu_domain_destroy(struct domain *d)
 
 void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
 {
+    /* Set to false options not supported on ARM. */
+    if ( iommu_hwdom_inclusive )
+        printk(XENLOG_WARNING "map-inclusive dom0-iommu option is not supported on ARM\n");
+    iommu_hwdom_inclusive = false;
+    if ( iommu_hwdom_reserved == 1 )
+        printk(XENLOG_WARNING "map-reserved dom0-iommu option is not supported on ARM\n");
+    iommu_hwdom_reserved = 0;
 }
 
 /*
diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
index c912120..d2572bc 100644
--- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -1329,19 +1329,6 @@ static int ipmmu_iommu_domain_init(struct domain *d)
     return 0;
 }
 
-static void __hwdom_init ipmmu_iommu_hwdom_init(struct domain *d)
-{
-    /* Set to false options not supported on ARM. */
-    if ( iommu_hwdom_inclusive )
-        printk(XENLOG_WARNING "ipmmu: map-inclusive dom0-iommu option is not supported on ARM\n");
-    iommu_hwdom_inclusive = false;
-    if ( iommu_hwdom_reserved == 1 )
-        printk(XENLOG_WARNING "ipmmu: map-reserved dom0-iommu option is not supported on ARM\n");
-    iommu_hwdom_reserved = 0;
-
-    arch_iommu_hwdom_init(d);
-}
-
 static void ipmmu_iommu_domain_teardown(struct domain *d)
 {
     struct ipmmu_vmsa_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
@@ -1369,7 +1356,7 @@ static void ipmmu_iommu_domain_teardown(struct domain *d)
 static const struct iommu_ops ipmmu_iommu_ops =
 {
     .init            = ipmmu_iommu_domain_init,
-    .hwdom_init      = ipmmu_iommu_hwdom_init,
+    .hwdom_init      = arch_iommu_hwdom_init,
     .teardown        = ipmmu_iommu_domain_teardown,
     .iotlb_flush     = ipmmu_iotlb_flush,
     .iotlb_flush_all = ipmmu_iotlb_flush_all,
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
index d115df7..71b022f 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -3402,21 +3402,6 @@ static int arm_smmu_iommu_xen_domain_init(struct domain *d)
 
 }
 
-static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
-{
-	/* Set to false options not supported on ARM. */
-	if (iommu_hwdom_inclusive)
-		printk(XENLOG_WARNING
-		"map-inclusive dom0-iommu option is not supported on ARM\n");
-	iommu_hwdom_inclusive = false;
-	if (iommu_hwdom_reserved == 1)
-		printk(XENLOG_WARNING
-		"map-reserved dom0-iommu option is not supported on ARM\n");
-	iommu_hwdom_reserved = 0;
-
-	arch_iommu_hwdom_init(d);
-}
-
 static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
 {
 	struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
@@ -3427,7 +3412,7 @@ static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
 
 static const struct iommu_ops arm_smmu_iommu_ops = {
 	.init		= arm_smmu_iommu_xen_domain_init,
-	.hwdom_init		= arm_smmu_iommu_hwdom_init,
+	.hwdom_init		= arch_iommu_hwdom_init,
 	.teardown		= arm_smmu_iommu_xen_domain_teardown,
 	.iotlb_flush		= arm_smmu_iotlb_flush,
 	.iotlb_flush_all	= arm_smmu_iotlb_flush_all,
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index c9dfc4c..b186c28 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2849,21 +2849,6 @@ static int arm_smmu_iommu_domain_init(struct domain *d)
 	return 0;
 }
 
-static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
-{
-	/* Set to false options not supported on ARM. */
-	if ( iommu_hwdom_inclusive )
-		printk(XENLOG_WARNING
-		"map-inclusive dom0-iommu option is not supported on ARM\n");
-	iommu_hwdom_inclusive = false;
-	if ( iommu_hwdom_reserved == 1 )
-		printk(XENLOG_WARNING
-		"map-reserved dom0-iommu option is not supported on ARM\n");
-	iommu_hwdom_reserved = 0;
-
-	arch_iommu_hwdom_init(d);
-}
-
 static void arm_smmu_iommu_domain_teardown(struct domain *d)
 {
 	struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
@@ -2874,7 +2859,7 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
 
 static const struct iommu_ops arm_smmu_iommu_ops = {
     .init = arm_smmu_iommu_domain_init,
-    .hwdom_init = arm_smmu_iommu_hwdom_init,
+    .hwdom_init = arch_iommu_hwdom_init,
     .add_device = arm_smmu_dt_add_device_generic,
     .teardown = arm_smmu_iommu_domain_teardown,
     .iotlb_flush = arm_smmu_iotlb_flush,
-- 
2.7.4

Re: [PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers

[Julien Grall]

Hi,

On 27/01/2022 19:55, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> All IOMMU drivers on Arm perform almost the same generic actions in
> hwdom_init callback. Move this code to common arch_iommu_hwdom_init()
> in order to get rid of code duplication.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

IMO, the reviewed-by tags should have been dropped with the changes you 
made. So I would like both reviewer to confirm they are happy with the 
change.

This also technically needs an ack from Bertrand/Rahul.

> ---
> Changes V1 -> V2:
>     - add R-b
> 
> Changes V2 -> V3:
>     - drop platform specific *_iommu_hwdom_init(), make .hwdom_init
>       to directly point to the common arch_iommu_hwdom_init()
> ---
>   xen/drivers/passthrough/arm/iommu.c      |  7 +++++++
>   xen/drivers/passthrough/arm/ipmmu-vmsa.c | 15 +--------------
>   xen/drivers/passthrough/arm/smmu-v3.c    | 17 +----------------
>   xen/drivers/passthrough/arm/smmu.c       | 17 +----------------
>   4 files changed, 10 insertions(+), 46 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
> index ee653a9..fc45318 100644
> --- a/xen/drivers/passthrough/arm/iommu.c
> +++ b/xen/drivers/passthrough/arm/iommu.c
> @@ -134,6 +134,13 @@ void arch_iommu_domain_destroy(struct domain *d)
>   
>   void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
>   {
> +    /* Set to false options not supported on ARM. */
> +    if ( iommu_hwdom_inclusive )
> +        printk(XENLOG_WARNING "map-inclusive dom0-iommu option is not supported on ARM\n");
> +    iommu_hwdom_inclusive = false;
> +    if ( iommu_hwdom_reserved == 1 )
> +        printk(XENLOG_WARNING "map-reserved dom0-iommu option is not supported on ARM\n");
> +    iommu_hwdom_reserved = 0;
>   }
>   
>   /*
> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> index c912120..d2572bc 100644
> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> @@ -1329,19 +1329,6 @@ static int ipmmu_iommu_domain_init(struct domain *d)
>       return 0;
>   }
>   
> -static void __hwdom_init ipmmu_iommu_hwdom_init(struct domain *d)
> -{
> -    /* Set to false options not supported on ARM. */
> -    if ( iommu_hwdom_inclusive )
> -        printk(XENLOG_WARNING "ipmmu: map-inclusive dom0-iommu option is not supported on ARM\n");
> -    iommu_hwdom_inclusive = false;
> -    if ( iommu_hwdom_reserved == 1 )
> -        printk(XENLOG_WARNING "ipmmu: map-reserved dom0-iommu option is not supported on ARM\n");
> -    iommu_hwdom_reserved = 0;
> -
> -    arch_iommu_hwdom_init(d);
> -}
> -
>   static void ipmmu_iommu_domain_teardown(struct domain *d)
>   {
>       struct ipmmu_vmsa_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> @@ -1369,7 +1356,7 @@ static void ipmmu_iommu_domain_teardown(struct domain *d)
>   static const struct iommu_ops ipmmu_iommu_ops =
>   {
>       .init            = ipmmu_iommu_domain_init,
> -    .hwdom_init      = ipmmu_iommu_hwdom_init,
> +    .hwdom_init      = arch_iommu_hwdom_init,
>       .teardown        = ipmmu_iommu_domain_teardown,
>       .iotlb_flush     = ipmmu_iotlb_flush,
>       .iotlb_flush_all = ipmmu_iotlb_flush_all,
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
> index d115df7..71b022f 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -3402,21 +3402,6 @@ static int arm_smmu_iommu_xen_domain_init(struct domain *d)
>   
>   }
>   
> -static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
> -{
> -	/* Set to false options not supported on ARM. */
> -	if (iommu_hwdom_inclusive)
> -		printk(XENLOG_WARNING
> -		"map-inclusive dom0-iommu option is not supported on ARM\n");
> -	iommu_hwdom_inclusive = false;
> -	if (iommu_hwdom_reserved == 1)
> -		printk(XENLOG_WARNING
> -		"map-reserved dom0-iommu option is not supported on ARM\n");
> -	iommu_hwdom_reserved = 0;
> -
> -	arch_iommu_hwdom_init(d);
> -}
> -
>   static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
>   {
>   	struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> @@ -3427,7 +3412,7 @@ static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
>   
>   static const struct iommu_ops arm_smmu_iommu_ops = {
>   	.init		= arm_smmu_iommu_xen_domain_init,
> -	.hwdom_init		= arm_smmu_iommu_hwdom_init,
> +	.hwdom_init		= arch_iommu_hwdom_init,
>   	.teardown		= arm_smmu_iommu_xen_domain_teardown,
>   	.iotlb_flush		= arm_smmu_iotlb_flush,
>   	.iotlb_flush_all	= arm_smmu_iotlb_flush_all,
> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index c9dfc4c..b186c28 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2849,21 +2849,6 @@ static int arm_smmu_iommu_domain_init(struct domain *d)
>   	return 0;
>   }
>   
> -static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
> -{
> -	/* Set to false options not supported on ARM. */
> -	if ( iommu_hwdom_inclusive )
> -		printk(XENLOG_WARNING
> -		"map-inclusive dom0-iommu option is not supported on ARM\n");
> -	iommu_hwdom_inclusive = false;
> -	if ( iommu_hwdom_reserved == 1 )
> -		printk(XENLOG_WARNING
> -		"map-reserved dom0-iommu option is not supported on ARM\n");
> -	iommu_hwdom_reserved = 0;
> -
> -	arch_iommu_hwdom_init(d);
> -}
> -
>   static void arm_smmu_iommu_domain_teardown(struct domain *d)
>   {
>   	struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> @@ -2874,7 +2859,7 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
>   
>   static const struct iommu_ops arm_smmu_iommu_ops = {
>       .init = arm_smmu_iommu_domain_init,
> -    .hwdom_init = arm_smmu_iommu_hwdom_init,
> +    .hwdom_init = arch_iommu_hwdom_init,
>       .add_device = arm_smmu_dt_add_device_generic,
>       .teardown = arm_smmu_iommu_domain_teardown,
>       .iotlb_flush = arm_smmu_iotlb_flush,

-- 
Julien Grall

Re: [PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers

[Oleksandr]

On 27.01.22 22:03, Julien Grall wrote:
> Hi,


Hi Julien


>
> On 27/01/2022 19:55, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> All IOMMU drivers on Arm perform almost the same generic actions in
>> hwdom_init callback. Move this code to common arch_iommu_hwdom_init()
>> in order to get rid of code duplication.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>
> IMO, the reviewed-by tags should have been dropped with the changes 
> you made. 

Yes, you are right.  I apologize, my fault.


> So I would like both reviewer to confirm they are happy with the change.
>
> This also technically needs an ack from Bertrand/Rahul.

Yes, for the SMMU V3.


>
>> ---
>> Changes V1 -> V2:
>>     - add R-b
>>
>> Changes V2 -> V3:
>>     - drop platform specific *_iommu_hwdom_init(), make .hwdom_init
>>       to directly point to the common arch_iommu_hwdom_init()
>> ---
>>   xen/drivers/passthrough/arm/iommu.c      |  7 +++++++
>>   xen/drivers/passthrough/arm/ipmmu-vmsa.c | 15 +--------------
>>   xen/drivers/passthrough/arm/smmu-v3.c    | 17 +----------------
>>   xen/drivers/passthrough/arm/smmu.c       | 17 +----------------
>>   4 files changed, 10 insertions(+), 46 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/arm/iommu.c 
>> b/xen/drivers/passthrough/arm/iommu.c
>> index ee653a9..fc45318 100644
>> --- a/xen/drivers/passthrough/arm/iommu.c
>> +++ b/xen/drivers/passthrough/arm/iommu.c
>> @@ -134,6 +134,13 @@ void arch_iommu_domain_destroy(struct domain *d)
>>     void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
>>   {
>> +    /* Set to false options not supported on ARM. */
>> +    if ( iommu_hwdom_inclusive )
>> +        printk(XENLOG_WARNING "map-inclusive dom0-iommu option is 
>> not supported on ARM\n");
>> +    iommu_hwdom_inclusive = false;
>> +    if ( iommu_hwdom_reserved == 1 )
>> +        printk(XENLOG_WARNING "map-reserved dom0-iommu option is not 
>> supported on ARM\n");
>> +    iommu_hwdom_reserved = 0;
>>   }
>>     /*
>> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c 
>> b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
>> index c912120..d2572bc 100644
>> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
>> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
>> @@ -1329,19 +1329,6 @@ static int ipmmu_iommu_domain_init(struct 
>> domain *d)
>>       return 0;
>>   }
>>   -static void __hwdom_init ipmmu_iommu_hwdom_init(struct domain *d)
>> -{
>> -    /* Set to false options not supported on ARM. */
>> -    if ( iommu_hwdom_inclusive )
>> -        printk(XENLOG_WARNING "ipmmu: map-inclusive dom0-iommu 
>> option is not supported on ARM\n");
>> -    iommu_hwdom_inclusive = false;
>> -    if ( iommu_hwdom_reserved == 1 )
>> -        printk(XENLOG_WARNING "ipmmu: map-reserved dom0-iommu option 
>> is not supported on ARM\n");
>> -    iommu_hwdom_reserved = 0;
>> -
>> -    arch_iommu_hwdom_init(d);
>> -}
>> -
>>   static void ipmmu_iommu_domain_teardown(struct domain *d)
>>   {
>>       struct ipmmu_vmsa_xen_domain *xen_domain = 
>> dom_iommu(d)->arch.priv;
>> @@ -1369,7 +1356,7 @@ static void ipmmu_iommu_domain_teardown(struct 
>> domain *d)
>>   static const struct iommu_ops ipmmu_iommu_ops =
>>   {
>>       .init            = ipmmu_iommu_domain_init,
>> -    .hwdom_init      = ipmmu_iommu_hwdom_init,
>> +    .hwdom_init      = arch_iommu_hwdom_init,
>>       .teardown        = ipmmu_iommu_domain_teardown,
>>       .iotlb_flush     = ipmmu_iotlb_flush,
>>       .iotlb_flush_all = ipmmu_iotlb_flush_all,
>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
>> b/xen/drivers/passthrough/arm/smmu-v3.c
>> index d115df7..71b022f 100644
>> --- a/xen/drivers/passthrough/arm/smmu-v3.c
>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
>> @@ -3402,21 +3402,6 @@ static int 
>> arm_smmu_iommu_xen_domain_init(struct domain *d)
>>     }
>>   -static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
>> -{
>> -    /* Set to false options not supported on ARM. */
>> -    if (iommu_hwdom_inclusive)
>> -        printk(XENLOG_WARNING
>> -        "map-inclusive dom0-iommu option is not supported on ARM\n");
>> -    iommu_hwdom_inclusive = false;
>> -    if (iommu_hwdom_reserved == 1)
>> -        printk(XENLOG_WARNING
>> -        "map-reserved dom0-iommu option is not supported on ARM\n");
>> -    iommu_hwdom_reserved = 0;
>> -
>> -    arch_iommu_hwdom_init(d);
>> -}
>> -
>>   static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
>>   {
>>       struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
>> @@ -3427,7 +3412,7 @@ static void 
>> arm_smmu_iommu_xen_domain_teardown(struct domain *d)
>>     static const struct iommu_ops arm_smmu_iommu_ops = {
>>       .init        = arm_smmu_iommu_xen_domain_init,
>> -    .hwdom_init        = arm_smmu_iommu_hwdom_init,
>> +    .hwdom_init        = arch_iommu_hwdom_init,
>>       .teardown        = arm_smmu_iommu_xen_domain_teardown,
>>       .iotlb_flush        = arm_smmu_iotlb_flush,
>>       .iotlb_flush_all    = arm_smmu_iotlb_flush_all,
>> diff --git a/xen/drivers/passthrough/arm/smmu.c 
>> b/xen/drivers/passthrough/arm/smmu.c
>> index c9dfc4c..b186c28 100644
>> --- a/xen/drivers/passthrough/arm/smmu.c
>> +++ b/xen/drivers/passthrough/arm/smmu.c
>> @@ -2849,21 +2849,6 @@ static int arm_smmu_iommu_domain_init(struct 
>> domain *d)
>>       return 0;
>>   }
>>   -static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
>> -{
>> -    /* Set to false options not supported on ARM. */
>> -    if ( iommu_hwdom_inclusive )
>> -        printk(XENLOG_WARNING
>> -        "map-inclusive dom0-iommu option is not supported on ARM\n");
>> -    iommu_hwdom_inclusive = false;
>> -    if ( iommu_hwdom_reserved == 1 )
>> -        printk(XENLOG_WARNING
>> -        "map-reserved dom0-iommu option is not supported on ARM\n");
>> -    iommu_hwdom_reserved = 0;
>> -
>> -    arch_iommu_hwdom_init(d);
>> -}
>> -
>>   static void arm_smmu_iommu_domain_teardown(struct domain *d)
>>   {
>>       struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
>> @@ -2874,7 +2859,7 @@ static void 
>> arm_smmu_iommu_domain_teardown(struct domain *d)
>>     static const struct iommu_ops arm_smmu_iommu_ops = {
>>       .init = arm_smmu_iommu_domain_init,
>> -    .hwdom_init = arm_smmu_iommu_hwdom_init,
>> +    .hwdom_init = arch_iommu_hwdom_init,
>>       .add_device = arm_smmu_dt_add_device_generic,
>>       .teardown = arm_smmu_iommu_domain_teardown,
>>       .iotlb_flush = arm_smmu_iotlb_flush,
>
-- 
Regards,

Oleksandr Tyshchenko

Re: [PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers

[Rahul Singh]

Hi Oleksandr,

> On 27 Jan 2022, at 7:55 pm, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
> 
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> All IOMMU drivers on Arm perform almost the same generic actions in
> hwdom_init callback. Move this code to common arch_iommu_hwdom_init()
> in order to get rid of code duplication.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Acked-by:: Rahul Singh <rahul.singh@arm.com>

Regards,
Rahul
> ---
> Changes V1 -> V2:
>   - add R-b
> 
> Changes V2 -> V3:
>   - drop platform specific *_iommu_hwdom_init(), make .hwdom_init
>     to directly point to the common arch_iommu_hwdom_init()
> ---
> xen/drivers/passthrough/arm/iommu.c      |  7 +++++++
> xen/drivers/passthrough/arm/ipmmu-vmsa.c | 15 +--------------
> xen/drivers/passthrough/arm/smmu-v3.c    | 17 +----------------
> xen/drivers/passthrough/arm/smmu.c       | 17 +----------------
> 4 files changed, 10 insertions(+), 46 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
> index ee653a9..fc45318 100644
> --- a/xen/drivers/passthrough/arm/iommu.c
> +++ b/xen/drivers/passthrough/arm/iommu.c
> @@ -134,6 +134,13 @@ void arch_iommu_domain_destroy(struct domain *d)
> 
> void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> {
> +    /* Set to false options not supported on ARM. */
> +    if ( iommu_hwdom_inclusive )
> +        printk(XENLOG_WARNING "map-inclusive dom0-iommu option is not supported on ARM\n");
> +    iommu_hwdom_inclusive = false;
> +    if ( iommu_hwdom_reserved == 1 )
> +        printk(XENLOG_WARNING "map-reserved dom0-iommu option is not supported on ARM\n");
> +    iommu_hwdom_reserved = 0;
> }
> 
> /*
> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> index c912120..d2572bc 100644
> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> @@ -1329,19 +1329,6 @@ static int ipmmu_iommu_domain_init(struct domain *d)
>     return 0;
> }
> 
> -static void __hwdom_init ipmmu_iommu_hwdom_init(struct domain *d)
> -{
> -    /* Set to false options not supported on ARM. */
> -    if ( iommu_hwdom_inclusive )
> -        printk(XENLOG_WARNING "ipmmu: map-inclusive dom0-iommu option is not supported on ARM\n");
> -    iommu_hwdom_inclusive = false;
> -    if ( iommu_hwdom_reserved == 1 )
> -        printk(XENLOG_WARNING "ipmmu: map-reserved dom0-iommu option is not supported on ARM\n");
> -    iommu_hwdom_reserved = 0;
> -
> -    arch_iommu_hwdom_init(d);
> -}
> -
> static void ipmmu_iommu_domain_teardown(struct domain *d)
> {
>     struct ipmmu_vmsa_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> @@ -1369,7 +1356,7 @@ static void ipmmu_iommu_domain_teardown(struct domain *d)
> static const struct iommu_ops ipmmu_iommu_ops =
> {
>     .init            = ipmmu_iommu_domain_init,
> -    .hwdom_init      = ipmmu_iommu_hwdom_init,
> +    .hwdom_init      = arch_iommu_hwdom_init,
>     .teardown        = ipmmu_iommu_domain_teardown,
>     .iotlb_flush     = ipmmu_iotlb_flush,
>     .iotlb_flush_all = ipmmu_iotlb_flush_all,
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
> index d115df7..71b022f 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -3402,21 +3402,6 @@ static int arm_smmu_iommu_xen_domain_init(struct domain *d)
> 
> }
> 
> -static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
> -{
> -	/* Set to false options not supported on ARM. */
> -	if (iommu_hwdom_inclusive)
> -		printk(XENLOG_WARNING
> -		"map-inclusive dom0-iommu option is not supported on ARM\n");
> -	iommu_hwdom_inclusive = false;
> -	if (iommu_hwdom_reserved == 1)
> -		printk(XENLOG_WARNING
> -		"map-reserved dom0-iommu option is not supported on ARM\n");
> -	iommu_hwdom_reserved = 0;
> -
> -	arch_iommu_hwdom_init(d);
> -}
> -
> static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
> {
> 	struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> @@ -3427,7 +3412,7 @@ static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
> 
> static const struct iommu_ops arm_smmu_iommu_ops = {
> 	.init		= arm_smmu_iommu_xen_domain_init,
> -	.hwdom_init		= arm_smmu_iommu_hwdom_init,
> +	.hwdom_init		= arch_iommu_hwdom_init,
> 	.teardown		= arm_smmu_iommu_xen_domain_teardown,
> 	.iotlb_flush		= arm_smmu_iotlb_flush,
> 	.iotlb_flush_all	= arm_smmu_iotlb_flush_all,
> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index c9dfc4c..b186c28 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2849,21 +2849,6 @@ static int arm_smmu_iommu_domain_init(struct domain *d)
> 	return 0;
> }
> 
> -static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
> -{
> -	/* Set to false options not supported on ARM. */
> -	if ( iommu_hwdom_inclusive )
> -		printk(XENLOG_WARNING
> -		"map-inclusive dom0-iommu option is not supported on ARM\n");
> -	iommu_hwdom_inclusive = false;
> -	if ( iommu_hwdom_reserved == 1 )
> -		printk(XENLOG_WARNING
> -		"map-reserved dom0-iommu option is not supported on ARM\n");
> -	iommu_hwdom_reserved = 0;
> -
> -	arch_iommu_hwdom_init(d);
> -}
> -
> static void arm_smmu_iommu_domain_teardown(struct domain *d)
> {
> 	struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> @@ -2874,7 +2859,7 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
> 
> static const struct iommu_ops arm_smmu_iommu_ops = {
>     .init = arm_smmu_iommu_domain_init,
> -    .hwdom_init = arm_smmu_iommu_hwdom_init,
> +    .hwdom_init = arch_iommu_hwdom_init,
>     .add_device = arm_smmu_dt_add_device_generic,
>     .teardown = arm_smmu_iommu_domain_teardown,
>     .iotlb_flush = arm_smmu_iotlb_flush,
> -- 
> 2.7.4
> 
> 

Re: [PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers

[Volodymyr Babchuk]

Hi Julien,

Julien Grall <julien@xen.org> writes:

> Hi,
>
> On 27/01/2022 19:55, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> All IOMMU drivers on Arm perform almost the same generic actions in
>> hwdom_init callback. Move this code to common arch_iommu_hwdom_init()
>> in order to get rid of code duplication.
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>
> IMO, the reviewed-by tags should have been dropped with the changes
> you made. So I would like both reviewer to confirm they are happy with
> the change.

Yep, I'm still fine with this:

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>


-- 
Volodymyr Babchuk at EPAM

Re: [PATCH V3 0/2] Add support for Renesas R-Car S4 IPMMU and other misc changes (remaining 2 patches)

[Julien Grall]

Hi,

On 27/01/2022 19:55, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> Hello all.
> 
> You can find the V1-V2 patch series at [1]-[2].
> 
> The first 8 patches (prereq work + R-Car S4 IPMMU support) have been already committed.
> These are remaining 2 patches which include misc changes.
> 
> [1] https://lore.kernel.org/all/1638035505-16931-1-git-send-email-olekstysh@gmail.com/
> [2] https://lore.kernel.org/all/1640034957-19764-1-git-send-email-olekstysh@gmail.com/
> 
> Oleksandr Tyshchenko (2):
>    iommu/ipmmu-vmsa: Use refcount for the micro-TLBs
>    iommu/arm: Remove code duplication in all IOMMU drivers

I have committed the two patches.

Cheers,

> 
>   xen/drivers/passthrough/arm/iommu.c      |  7 +++++++
>   xen/drivers/passthrough/arm/ipmmu-vmsa.c | 36 ++++++++++++++------------------
>   xen/drivers/passthrough/arm/smmu-v3.c    | 17 +--------------
>   xen/drivers/passthrough/arm/smmu.c       | 17 +--------------
>   4 files changed, 25 insertions(+), 52 deletions(-)
> 

-- 
Julien Grall