diff mbox series

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

Message ID 1643313352-29940-3-git-send-email-olekstysh@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add support for Renesas R-Car S4 IPMMU and other misc changes (remaining 2 patches) | expand

Commit Message

Oleksandr Tyshchenko Jan. 27, 2022, 7:55 p.m. UTC 
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(-)

Comments

Julien Grall Jan. 27, 2022, 8:03 p.m. UTC | #1 
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,
Oleksandr Tyshchenko Jan. 27, 2022, 8:45 p.m. UTC | #2 
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,
>
Rahul Singh Jan. 28, 2022, 10:11 a.m. UTC | #3 
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
> 
>
Volodymyr Babchuk Jan. 28, 2022, 6:08 p.m. UTC | #4 
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>
diff mbox series

Patch

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,