Hi Roger,
On 22/08/18 08:51, Roger Pau Monne wrote:
Introduce a new dom0-iommu=map-inclusive generic option that
supersedes iommu_inclusive_mapping. The previous behavior is preserved
and the option should only be enabled by default on Intel hardware.
Signed-off-by: Roger Pau Monné <[email protected]>
Reviewed-by: Paul Durrant <[email protected]>
Reviewed-by: Jan Beulich <[email protected]>
Reviewed-by: Kevin Tian <[email protected]>
---
Changes since v4:
- Use an if to set the default option value.
- Set the option to false unconditionally on ARM.
Changes since v2:
- Fix typo in commit message.
- Change style and text of the documentation in xen command line.
- Set the defaults in {intel/amd}_iommu_hwdom_init for inclusive.
- Re-add the iommu_dom0_passthrough || !is_pv_domain(d) check.
Changes since v1:
- Use dom0-iommu instead of the iommu option.
- Only enable by default on Intel hardware.
---
Cc: Andrew Cooper <[email protected]>
Cc: George Dunlap <[email protected]>
Cc: Ian Jackson <[email protected]>
Cc: Jan Beulich <[email protected]>
Cc: Julien Grall <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Stefano Stabellini <[email protected]>
Cc: Tim Deegan <[email protected]>
Cc: Wei Liu <[email protected]>
Cc: Suravee Suthikulpanit <[email protected]>
Cc: Brian Woods <[email protected]>
Cc: Kevin Tian <[email protected]>
---
docs/misc/xen-command-line.markdown | 13 ++++-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 ++
xen/drivers/passthrough/arm/iommu.c | 4 ++
xen/drivers/passthrough/arm/smmu.c | 2 +
xen/drivers/passthrough/iommu.c | 12 +++++
xen/drivers/passthrough/vtd/extern.h | 2 -
xen/drivers/passthrough/vtd/iommu.c | 8 ++-
xen/drivers/passthrough/vtd/x86/vtd.c | 58 +-------------------
xen/drivers/passthrough/x86/iommu.c | 59 +++++++++++++++++++++
xen/include/xen/iommu.h | 2 +
10 files changed, 99 insertions(+), 65 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown
b/docs/misc/xen-command-line.markdown
index cd57960ede..98f0f3b68b 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -682,7 +682,7 @@ Flag that makes a dom0 use shadow paging. Only works when
"pvh" is
enabled.
### dom0-iommu
-> `= List of [ passthrough | strict ]`
+> `= List of [ passthrough | strict | map-inclusive ]`
This list of booleans controls the iommu usage by Dom0:
@@ -696,6 +696,14 @@ This list of booleans controls the iommu usage by Dom0:
`true` for a PVH Dom0 and any attempt to overwrite it from the command line
is ignored.
+* `map-inclusive`: sets up DMA remapping for all the non-RAM regions below 4GB
+ except for unusable ranges. Use this to work around firmware issues providing
+ incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU
+ accesses for Dom0, with this option all pages up to 4GB, not marked as
+ unusable in the E820 table, will get a mapping established. Note that this
+ option is only applicable to a PV Dom0 and is enabled by default on Intel
+ hardware.
+
### dom0\_ioports\_disable (x86)
> `= List of <hex>-<hex>`
@@ -1233,6 +1241,9 @@ wait descriptor timed out', try increasing this value.
### iommu\_inclusive\_mapping (VT-d)
> `= <boolean>`
+**WARNING: This command line option is deprecated, and superseded by
+_dom0-iommu=map-inclusive_ - using both options in combination is undefined.**
+
> Default: `true`
Use this to work around firmware issues providing incorrect RMRR entries.
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index ab39e7500d..27eb49619d 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -253,6 +253,10 @@ static void __hwdom_init amd_iommu_hwdom_init(struct
domain *d)
unsigned long i;
const struct amd_iommu *iommu;
+ /* Inclusive IOMMU mappings are disabled by default on AMD hardware. */
+ if ( iommu_hwdom_inclusive == -1 )
+ iommu_hwdom_inclusive = false;
This is a quite bad practice to mix boolean and integer. Can you please
rework the code to avoid that?
+
if ( allocate_domain_resources(dom_iommu(d)) )
BUG();
diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
index 95b1abb972..325997b19f 100644
--- a/xen/drivers/passthrough/arm/iommu.c
+++ b/xen/drivers/passthrough/arm/iommu.c
@@ -73,3 +73,7 @@ int arch_iommu_populate_page_table(struct domain *d)
/* The IOMMU shares the p2m with the CPU */
return -ENOSYS;
}
+
+void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
+{
+}
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index 74c09b0991..b142677b8c 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2727,6 +2727,8 @@ 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. */
+ iommu_hwdom_inclusive = false;
I would prefer if print a warning when the value is not false and then
set to false.
}
static void arm_smmu_iommu_domain_teardown(struct domain *d)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 8f9975cf4e..5798142730 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -61,6 +61,7 @@ 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;
/*
* In the current implementation of VT-d posted interrupts, in some extreme
@@ -152,6 +153,8 @@ 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
rc = -EINVAL;
@@ -232,6 +235,15 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
}
hd->platform_ops->hwdom_init(d);
+
+ if ( iommu_hwdom_inclusive && !is_pv_domain(d) )
+ {
+ printk(XENLOG_WARNING
+ "IOMMU inclusive mappings are only supported on PV Dom0\n");
+ iommu_hwdom_inclusive = false;
Same remark as above about mixing boolean and integer.
+ }
+
+ arch_iommu_hwdom_init(d);
}
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
