[Xen-devel] [PATCH v5 01/15] iommu: turn need_iommu back into a boolean.

[Paul Durrant <paul.durrant@xxxxxxxxxx>]

As noted in [1] the tri-state nature of need_iommu after commit 3e06b989 is
confusing.

Because arch_iommu_populate_page_table() removes pages from the page list
as it maps them it is actually safe to re-invoke multiple times without
the need for any specific indication it has been called before, so it
is actually safe to simply turn need_iommu back into a boolean with no
change to the population algorithm.

[1] https://lists.xenproject.org/archives/html/xen-devel/2018-07/msg01870.html

Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
---
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

v5:
 - Simpler patch replacing the more significatio reversion of 3e06b989 in
   v4.

v4:
 - New in v4.
---
 xen/drivers/passthrough/device_tree.c | 19 ++++++++-----------
 xen/drivers/passthrough/iommu.c       |  8 ++++----
 xen/drivers/passthrough/x86/iommu.c   |  2 --
 xen/include/xen/sched.h               |  4 ++--
 4 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/xen/drivers/passthrough/device_tree.c 
b/xen/drivers/passthrough/device_tree.c
index 421f003438..671c8db1d0 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -40,17 +40,14 @@ int iommu_assign_dt_device(struct domain *d, struct 
dt_device_node *dev)
     if ( !list_empty(&dev->domain_list) )
         goto fail;
 
-    if ( need_iommu(d) <= 0 )
-    {
-        /*
-         * The hwdom is forced to use IOMMU for protecting assigned
-         * device. Therefore the IOMMU data is already set up.
-         */
-        ASSERT(!is_hardware_domain(d));
-        rc = iommu_construct(d);
-        if ( rc )
-            goto fail;
-    }
+    /*
+     * The hwdom is forced to use IOMMU for protecting assigned
+     * device. Therefore the IOMMU data is already set up.
+     */
+    ASSERT(!is_hardware_domain(d) || need_iommu(d));
+    rc = iommu_construct(d);
+    if ( rc )
+        goto fail;
 
     /* The flag field doesn't matter to DT device. */
     rc = hd->platform_ops->assign_device(d, 0, dt_to_dev(dev), 0);
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 70d218f910..49974e5634 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -214,14 +214,14 @@ void iommu_teardown(struct domain *d)
 {
     const struct domain_iommu *hd = dom_iommu(d);
 
-    d->need_iommu = 0;
+    d->need_iommu = false;
     hd->platform_ops->teardown(d);
     tasklet_schedule(&iommu_pt_cleanup_tasklet);
 }
 
 int iommu_construct(struct domain *d)
 {
-    if ( need_iommu(d) > 0 )
+    if ( need_iommu(d) )
         return 0;
 
     if ( !iommu_use_hap_pt(d) )
@@ -233,7 +233,7 @@ int iommu_construct(struct domain *d)
             return rc;
     }
 
-    d->need_iommu = 1;
+    d->need_iommu = true;
     /*
      * There may be dirty cache lines when a device is assigned
      * and before need_iommu(d) becoming true, this will cause
@@ -493,7 +493,7 @@ static void iommu_dump_p2m_table(unsigned char key)
     ops = iommu_get_ops();
     for_each_domain(d)
     {
-        if ( is_hardware_domain(d) || need_iommu(d) <= 0 )
+        if ( is_hardware_domain(d) || !need_iommu(d) )
             continue;
 
         if ( iommu_use_hap_pt(d) )
diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index 68182afd91..d48d16b51b 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -45,8 +45,6 @@ int arch_iommu_populate_page_table(struct domain *d)
     struct page_info *page;
     int rc = 0, n = 0;
 
-    d->need_iommu = -1;
-
     this_cpu(iommu_dont_flush_iotlb) = 1;
     spin_lock(&d->page_alloc_lock);
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 851f11ecf7..5b9820e4e1 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -372,8 +372,8 @@ struct domain
 #ifdef CONFIG_HAS_PASSTHROUGH
     struct domain_iommu iommu;
 
-    /* Does this guest need iommu mappings (-1 meaning "being set up")? */
-    s8               need_iommu;
+    /* Does this guest need iommu mappings? */
+    bool             need_iommu;
 #endif
     /* is node-affinity automatically computed? */
     bool             auto_node_affinity;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel