Re: [PATCH V2 10/10] iommu/arm: Remove code duplication in all IOMMU drivers
On 27.01.22 13:54, Julien Grall wrote:
Hi,
Hi Julien
On 20/12/2021 21:15, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko
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
Reviewed-by: Volodymyr Babchuk
Reviewed-by: Yoshihiro Shimoda
---
Changes V1 -> V2:
- add R-b
---
xen/drivers/passthrough/arm/iommu.c | 7 +++
xen/drivers/passthrough/arm/ipmmu-vmsa.c | 8
xen/drivers/passthrough/arm/smmu-v3.c | 10 --
xen/drivers/passthrough/arm/smmu.c | 10 --
4 files changed, 7 insertions(+), 28 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 1224ea4..64d8ddc 100644
--- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -1325,14 +1325,6 @@ static int ipmmu_iommu_domain_init(struct
domain *d)
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);
}
With this change, ipmmu_iommu_domain_init() is just a call to
arch_iommu_hwdom_init(). The two functions have the same prototype, so
could we simply drop the former and update the .hwdom_init to directly
point to the latter?
Yes, good idea, will update. Thank you.
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c
b/xen/drivers/passthrough/arm/smmu-v3.c
index d115df7..ca8b5c7 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -3404,16 +3404,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);
}
Same here.
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index c9dfc4c..ec18df7 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2851,16 +2851,6 @@ static int arm_smmu_iommu_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);
}
Same here.
Cheers,
--
Regards,
Oleksandr Tyshchenko
Re: [PATCH V2 10/10] iommu/arm: Remove code duplication in all IOMMU drivers
Hi,
On 20/12/2021 21:15, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko
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
Reviewed-by: Volodymyr Babchuk
Reviewed-by: Yoshihiro Shimoda
---
Changes V1 -> V2:
- add R-b
---
xen/drivers/passthrough/arm/iommu.c | 7 +++
xen/drivers/passthrough/arm/ipmmu-vmsa.c | 8
xen/drivers/passthrough/arm/smmu-v3.c| 10 --
xen/drivers/passthrough/arm/smmu.c | 10 --
4 files changed, 7 insertions(+), 28 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 1224ea4..64d8ddc 100644
--- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -1325,14 +1325,6 @@ static int ipmmu_iommu_domain_init(struct domain *d)
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);
}
With this change, ipmmu_iommu_domain_init() is just a call to
arch_iommu_hwdom_init(). The two functions have the same prototype, so
could we simply drop the former and update the .hwdom_init to directly
point to the latter?
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
index d115df7..ca8b5c7 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -3404,16 +3404,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);
}
Same here.
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index c9dfc4c..ec18df7 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2851,16 +2851,6 @@ static int arm_smmu_iommu_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);
}
Same here.
Cheers,
--
Julien Grall
