[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

To: Julien Grall <julien@xxxxxxx>
From: Oleksandr <olekstysh@xxxxxxxxx>
Date: Thu, 27 Jan 2022 22:45:16 +0200
Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>, Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>, Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
Delivery-date: Thu, 27 Jan 2022 20:45:27 +0000
List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


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@xxxxxxxx>

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@xxxxxxxx>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>

IMO, the reviewed-by tags should have been dropped with the changesyou 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.cb/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 isnot supported on ARM\n");

+    iommu_hwdom_inclusive = false;
+    if ( iommu_hwdom_reserved == 1 )

+ printk(XENLOG_WARNING "map-reserved dom0-iommu option is notsupported on ARM\n");

+    iommu_hwdom_reserved = 0;
  }
    /*

diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.cb/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(structdomain *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-iommuoption is not supported on ARM\n");

-    iommu_hwdom_inclusive = false;
-    if ( iommu_hwdom_reserved == 1 )

- printk(XENLOG_WARNING "ipmmu: map-reserved dom0-iommu optionis 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(structdomain *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.cb/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 intarm_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 voidarm_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.cb/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(structdomain *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 voidarm_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

References:
- [PATCH V3 0/2] Add support for Renesas R-Car S4 IPMMU and other misc changes (remaining 2 patches)
  - From: Oleksandr Tyshchenko
- [PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers
  - From: Oleksandr Tyshchenko
- Re: [PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers
  - From: Julien Grall

Prev by Date: Re: [XEN v1] xen/arm: io: Check ESR_EL2.ISV != 0 before searching for a MMIO handler
Next by Date: Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver
Previous by thread: Re: [PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers
Next by thread: Re: [PATCH V3 2/2] iommu/arm: Remove code duplication in all IOMMU drivers
Index(es):
- Date
- Thread

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.