[Xen-devel] [PATCH v2 3/4] xen/dom0: Drop iommu_hwdom_inclusive entirely

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

This option is unique to x86 PV dom0's, but it is not sensible to have a
catch-all which blindly maps all non-RAM regions into the IOMMU.

The map-reserved option remains, and covers all the buggy firmware issues that
I am aware of.  The two common cases are legacy USB keyboard emulation, and
the BMC mailbox used by vendor firmware in NICs/HBAs to report information
back to the iLO/iDRAC/etc for remote remote management purposes.

A specific advantage of removing this option is that x86 dom0's IOMMU setup is
now consistent between PV and PVH.

This removal is not expected to have any impact, due to map-reserved
remaining.  In the unlikely case that it does cause an issue, we should
introduce other map-$SPECIFIC options rather than re-introducing this
catch-all.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>

v2:
 * New
---
 docs/misc/xen-command-line.markdown   | 19 +++----------------
 xen/drivers/passthrough/arm/smmu.c    |  4 ----
 xen/drivers/passthrough/iommu.c       |  3 ---
 xen/drivers/passthrough/vtd/x86/vtd.c |  6 ------
 xen/drivers/passthrough/x86/iommu.c   | 14 ++------------
 xen/include/xen/iommu.h               |  2 +-
 6 files changed, 6 insertions(+), 42 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 0aeb786..3a9af17 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -664,8 +664,7 @@ Controls for how dom0 is constructed on x86 systems.
     hardware is not HAP-capable.
 
 ### dom0-iommu
-> `= List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>,
->              map-reserved=<bool> ]`
+> `= List of [ passthrough=<bool>, strict=<bool>, map-reserved=<bool> ]`
 
 Controls for the dom0 IOMMU setup.
 
@@ -692,9 +691,8 @@ Controls for the dom0 IOMMU setup.
     other domains in the system don't live in a compatible address space), and
     is ignored for ARM.
 
-*   The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up
-    identity IOMMU mappings for all non-RAM regions below 4GB except for
-    unusable ranges, and ranges belonging to Xen.
+*   The `map-reserved` boolean is applicable to x86, and sets up identity IOMMU
+    mappings for all E820 reserved regions below 4GB.
 
     Typically, some devices in a system use bits of RAM for communication, and
     these areas should be listed as reserved in the E820 table and identified
@@ -708,11 +706,6 @@ Controls for the dom0 IOMMU setup.
     This option is enabled by default on x86 systems, and invalid on ARM
     systems.
 
-*   The `map-reserved` functionality is very similar to `map-inclusive`, but is
-    applicable to both x86 PV and PVH dom0's, and represents a subset of the
-    correction by only mapping reserved memory regions rather than all non-RAM
-    regions.
-
 ### dom0\_ioports\_disable (x86)
 > `= List of <hex>-<hex>`
 
@@ -1253,12 +1246,6 @@ Specify the timeout of the device IOTLB invalidation in 
milliseconds.
 By default, the timeout is 1000 ms. When you see error 'Queue invalidate
 wait descriptor timed out', try increasing this value.
 
-### iommu_inclusive_mapping
-> `= <boolean>`
-
-**WARNING: This command line option is deprecated, and superseded by
-_dom0-iommu=map-inclusive_ - using both options in combination is undefined.**
-
 ### irq\_ratelimit (x86)
 > `= <integer>`
 
diff --git a/xen/drivers/passthrough/arm/smmu.c 
b/xen/drivers/passthrough/arm/smmu.c
index 9612c0f..16de7e1 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2711,10 +2711,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 == 1 )
-               printk(XENLOG_WARNING
-               "map-inclusive dom0-iommu option is not supported on ARM\n");
-       iommu_hwdom_inclusive = 0;
        if ( iommu_hwdom_reserved == 1 )
                printk(XENLOG_WARNING
                "map-reserved dom0-iommu option is not supported on ARM\n");
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index ac62d7f..d2ee2ee 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -61,7 +61,6 @@ bool_t __read_mostly iommu_intremap = 1;
 
 bool __hwdom_initdata iommu_hwdom_strict;
 bool __read_mostly iommu_hwdom_passthrough;
-int8_t __hwdom_initdata iommu_hwdom_inclusive = -1;
 int8_t __hwdom_initdata iommu_hwdom_reserved = -1;
 
 /*
@@ -154,8 +153,6 @@ static int __init parse_dom0_iommu_param(const char *s)
             iommu_hwdom_passthrough = val;
         else if ( (val = parse_boolean("strict", s, ss)) >= 0 )
             iommu_hwdom_strict = val;
-        else if ( (val = parse_boolean("map-inclusive", s, ss)) >= 0 )
-            iommu_hwdom_inclusive = val;
         else if ( (val = parse_boolean("map-reserved", s, ss)) >= 0 )
             iommu_hwdom_reserved = val;
         else
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c 
b/xen/drivers/passthrough/vtd/x86/vtd.c
index ff456e1..cf9d7e1 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -30,12 +30,6 @@
 #include "../vtd.h"
 #include "../extern.h"
 
-/*
- * iommu_inclusive_mapping: when set, all memory below 4GB is included in dom0
- * 1:1 iommu mappings except xen and unusable regions.
- */
-boolean_param("iommu_inclusive_mapping", iommu_hwdom_inclusive);
-
 void *map_vtd_domain_page(u64 maddr)
 {
     return map_domain_page(_mfn(paddr_to_pfn(maddr)));
diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index c68a722..0ccb754 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -169,10 +169,10 @@ static bool __hwdom_init hwdom_iommu_map(const struct 
domain *d,
     default:
         if ( type & RAM_TYPE_RESERVED )
         {
-            if ( !iommu_hwdom_inclusive && !iommu_hwdom_reserved )
+            if ( !iommu_hwdom_reserved )
                 return false;
         }
-        else if ( is_hvm_domain(d) || !iommu_hwdom_inclusive || pfn > max_pfn )
+        else if ( is_hvm_domain(d) || pfn > max_pfn )
             return false;
     }
 
@@ -210,20 +210,10 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
 
     BUG_ON(!is_hardware_domain(d));
 
-    /* Inclusive mappings are enabled by default for PV. */
-    if ( iommu_hwdom_inclusive == -1 )
-        iommu_hwdom_inclusive = is_pv_domain(d);
     /* Reserved IOMMU mappings are enabled by default. */
     if ( iommu_hwdom_reserved == -1 )
         iommu_hwdom_reserved = 1;
 
-    if ( iommu_hwdom_inclusive && !is_pv_domain(d) )
-    {
-        printk(XENLOG_WARNING
-               "IOMMU inclusive mappings are only supported on PV Dom0\n");
-        iommu_hwdom_inclusive = 0;
-    }
-
     if ( iommu_hwdom_passthrough )
         return;
 
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 3d78126..49d0f0e 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -61,7 +61,7 @@ extern bool_t iommu_debug;
 extern bool_t amd_iommu_perdev_intremap;
 
 extern bool iommu_hwdom_strict, iommu_hwdom_passthrough;
-extern int8_t iommu_hwdom_inclusive, iommu_hwdom_reserved;
+extern int8_t iommu_hwdom_reserved;
 
 extern unsigned int iommu_dev_iotlb_timeout;
 
-- 
2.1.4


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