[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v7 3/6] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
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é <roger.pau@xxxxxxxxxx> Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx> --- 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 <andrew.cooper3@xxxxxxxxxx> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Cc: Jan Beulich <jbeulich@xxxxxxxx> 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: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> Cc: Brian Woods <brian.woods@xxxxxxx> Cc: Kevin Tian <kevin.tian@xxxxxxxxx> --- 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.cindex 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 Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |