* [PATCH v2 0/5] x86/iommu: PVH Dom0 workarounds for missing RMRR entries
@ 2018-08-01 11:03 Roger Pau Monne
2018-08-01 11:03 ` [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal Roger Pau Monne
` (4 more replies)
0 siblings, 5 replies; 29+ messages in thread
From: Roger Pau Monne @ 2018-08-01 11:03 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Hello,
The following series implement a workaround for missing RMRR
entries for a PVH Dom0. It's based on the iommu_inclusive_mapping VTd
option.
The PVH workaround identity maps all regions marked as reserved in the
memory map.
Note that this workaround is enabled by default on Intel hardware. It's
also available to AMD hardware, although it's disabled by default in
that case.
The series can be found at:
git://xenbits.xen.org/people/royger/xen.git iommu_inclusive_v2
Thanks, Roger.
Roger Pau Monne (5):
iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal
iommu: introduce dom0-iommu option
iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
dom0/pvh: change the order of the MMCFG initialization
x86/iommu: add PVH support to the inclusive options
docs/misc/xen-command-line.markdown | 55 +++++++++++
xen/arch/x86/hvm/dom0_build.c | 9 +-
xen/arch/x86/hvm/io.c | 5 +
xen/arch/x86/x86_64/mm.c | 3 +-
xen/drivers/passthrough/amd/iommu_init.c | 2 +-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 +-
xen/drivers/passthrough/arm/iommu.c | 4 +
xen/drivers/passthrough/iommu.c | 54 +++++++++--
xen/drivers/passthrough/vtd/extern.h | 2 -
xen/drivers/passthrough/vtd/iommu.c | 20 ++--
xen/drivers/passthrough/vtd/x86/vtd.c | 67 +------------
xen/drivers/passthrough/x86/iommu.c | 102 ++++++++++++++++++++
xen/include/asm-x86/hvm/io.h | 3 +
xen/include/xen/iommu.h | 8 +-
14 files changed, 242 insertions(+), 96 deletions(-)
--
2.18.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal
2018-08-01 11:03 [PATCH v2 0/5] x86/iommu: PVH Dom0 workarounds for missing RMRR entries Roger Pau Monne
@ 2018-08-01 11:03 ` Roger Pau Monne
2018-08-01 11:16 ` Paul Durrant
2018-08-02 7:36 ` Tian, Kevin
2018-08-01 11:03 ` [PATCH v2 2/5] iommu: introduce dom0-iommu option Roger Pau Monne
` (3 subsequent siblings)
4 siblings, 2 replies; 29+ messages in thread
From: Roger Pau Monne @ 2018-08-01 11:03 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Roger Pau Monne
Remove the handling for different page sizes now that ia64 is gone.
No functional change.
Reported by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
- New in this version.
---
Cc: Kevin Tian <kevin.tian@intel.com>
---
xen/drivers/passthrough/vtd/x86/vtd.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
index cc2bfea162..00a9891005 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -110,7 +110,7 @@ void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
{
- unsigned long i, j, tmp, top, max_pfn;
+ unsigned long i, top, max_pfn;
BUG_ON(!is_hardware_domain(d));
@@ -121,7 +121,7 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
{
unsigned long pfn = pdx_to_pfn(i);
bool map;
- int rc = 0;
+ int rc;
/*
* Set up 1:1 mapping for dom0. Default to include only
@@ -152,21 +152,12 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
continue;
- tmp = 1 << (PAGE_SHIFT - PAGE_SHIFT_4K);
- for ( j = 0; j < tmp; j++ )
- {
- int ret = iommu_map_page(d, pfn * tmp + j, pfn * tmp + j,
- IOMMUF_readable|IOMMUF_writable);
-
- if ( !rc )
- rc = ret;
- }
-
+ rc = iommu_map_page(d, pfn, pfn, IOMMUF_readable|IOMMUF_writable);
if ( rc )
printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed: %d\n",
d->domain_id, rc);
- if (!(i & (0xfffff >> (PAGE_SHIFT - PAGE_SHIFT_4K))))
+ if (!(i & 0xfffff))
process_pending_softirqs();
}
}
--
2.18.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-01 11:03 [PATCH v2 0/5] x86/iommu: PVH Dom0 workarounds for missing RMRR entries Roger Pau Monne
2018-08-01 11:03 ` [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal Roger Pau Monne
@ 2018-08-01 11:03 ` Roger Pau Monne
2018-08-01 11:15 ` Paul Durrant
` (2 more replies)
2018-08-01 11:03 ` [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu Roger Pau Monne
` (2 subsequent siblings)
4 siblings, 3 replies; 29+ messages in thread
From: Roger Pau Monne @ 2018-08-01 11:03 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, Jan Beulich, Brian Woods, Roger Pau Monne
To select the iommu configuration used by Dom0. This option supersedes
iommu=dom0-strict|dom0-passthrough.
No functional change.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
- New in this version.
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
docs/misc/xen-command-line.markdown | 32 ++++++++++++++
xen/arch/x86/x86_64/mm.c | 3 +-
xen/drivers/passthrough/amd/iommu_init.c | 2 +-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 +-
xen/drivers/passthrough/iommu.c | 46 +++++++++++++++++----
xen/drivers/passthrough/vtd/iommu.c | 16 +++----
xen/include/xen/iommu.h | 6 ++-
7 files changed, 88 insertions(+), 21 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 65b4754418..a2a07cc6c8 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1150,12 +1150,18 @@ detection of systems known to misbehave upon accesses to that port.
> `dom0-passthrough`
+> **WARNING: This command line option is deprecated, and superseded by
+> _dom0-iommu=none_ - using both options in combination is undefined.**
+
> Default: `false`
>> Control whether to disable DMA remapping for Dom0.
> `dom0-strict`
+> **WARNING: This command line option is deprecated, and superseded by
+> _dom0-iommu=strict_ - using both options in combination is undefined.**
+
> Default: `false`
>> Control whether to set up DMA remapping only for the memory Dom0 actually
@@ -1198,6 +1204,32 @@ detection of systems known to misbehave upon accesses to that port.
>> Enable IOMMU debugging code (implies `verbose`).
+### dom0-iommu
+> `= List of [ none | strict | relaxed ]`
+
+> Sub-options are of boolean kind and can be prefixed with `no-` to effect the
+> inverse meaning.
+
+> `none`
+
+> Default: `false`
+
+>> Control whether to disable DMA remapping for Dom0.
+
+> `strict`
+
+> Default: `false`
+
+>> Control whether to set up DMA remapping only for the memory Dom0 actually
+>> got assigned.
+
+> `relaxed`
+
+> Default: `true`
+
+>> Controls whether to setup DMA remappings for all the host RAM except regions
+>> in use by Xen.
+
### iommu\_dev\_iotlb\_timeout
> `= <integer>`
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index cca4ae926e..84226b3326 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1426,7 +1426,8 @@ int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm)
if ( ret )
goto destroy_m2p;
- if ( iommu_enabled && !iommu_passthrough && !need_iommu(hardware_domain) )
+ if ( iommu_enabled && !iommu_dom0_passthrough &&
+ !need_iommu(hardware_domain) )
{
for ( i = spfn; i < epfn; i++ )
if ( iommu_map_page(hardware_domain, i, i, IOMMUF_readable|IOMMUF_writable) )
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 474992a75a..ad8c48be1c 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1062,7 +1062,7 @@ static void __init amd_iommu_init_cleanup(void)
radix_tree_destroy(&ivrs_maps, xfree);
iommu_enabled = 0;
- iommu_passthrough = 0;
+ iommu_dom0_passthrough = false;
iommu_intremap = 0;
iommuv2_enabled = 0;
}
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 12d2695b89..eeacf713e4 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -121,7 +121,7 @@ static void amd_iommu_setup_domain_device(
BUG_ON( !hd->arch.root_table || !hd->arch.paging_mode ||
!iommu->dev_table.buffer );
- if ( iommu_passthrough && is_hardware_domain(domain) )
+ if ( iommu_dom0_passthrough && is_hardware_domain(domain) )
valid = 0;
if ( ats_enabled )
@@ -256,7 +256,7 @@ static void __hwdom_init amd_iommu_hwdom_init(struct domain *d)
if ( allocate_domain_resources(dom_iommu(d)) )
BUG();
- if ( !iommu_passthrough && !need_iommu(d) )
+ if ( !iommu_dom0_passthrough && !need_iommu(d) )
{
int rc = 0;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 70d218f910..88e23bbd04 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -22,6 +22,7 @@
#include <xsm/xsm.h>
static int parse_iommu_param(const char *s);
+static int parse_dom0_iommu_param(const char *s);
static void iommu_dump_p2m_table(unsigned char key);
unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
@@ -52,11 +53,9 @@ custom_param("iommu", parse_iommu_param);
bool_t __initdata iommu_enable = 1;
bool_t __read_mostly iommu_enabled;
bool_t __read_mostly force_iommu;
-bool_t __hwdom_initdata iommu_dom0_strict;
bool_t __read_mostly iommu_verbose;
bool_t __read_mostly iommu_workaround_bios_bug;
bool_t __read_mostly iommu_igfx = 1;
-bool_t __read_mostly iommu_passthrough;
bool_t __read_mostly iommu_snoop = 1;
bool_t __read_mostly iommu_qinval = 1;
bool_t __read_mostly iommu_intremap = 1;
@@ -72,6 +71,10 @@ bool_t __read_mostly iommu_hap_pt_share = 1;
bool_t __read_mostly iommu_debug;
bool_t __read_mostly amd_iommu_perdev_intremap = 1;
+custom_param("dom0-iommu", parse_dom0_iommu_param);
+bool __hwdom_initdata iommu_dom0_strict;
+bool __read_mostly iommu_dom0_passthrough;
+
DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
DEFINE_SPINLOCK(iommu_pt_cleanup_lock);
@@ -121,7 +124,7 @@ static int __init parse_iommu_param(const char *s)
else if ( !strncmp(s, "amd-iommu-perdev-intremap", ss - s) )
amd_iommu_perdev_intremap = val;
else if ( !strncmp(s, "dom0-passthrough", ss - s) )
- iommu_passthrough = val;
+ iommu_dom0_passthrough = val;
else if ( !strncmp(s, "dom0-strict", ss - s) )
iommu_dom0_strict = val;
else if ( !strncmp(s, "sharept", ss - s) )
@@ -135,6 +138,35 @@ static int __init parse_iommu_param(const char *s)
return rc;
}
+static int __init parse_dom0_iommu_param(const char *s)
+{
+ const char *ss;
+ int val, rc = 0;
+
+ do {
+ val = !!strncmp(s, "no-", 3);
+ if ( !val )
+ s += 3;
+
+ ss = strchr(s, ',');
+ if ( !ss )
+ ss = strchr(s, '\0');
+
+ if ( !strncmp(s, "none", ss - s) )
+ iommu_dom0_passthrough = val;
+ else if ( !strncmp(s, "strict", ss - s) )
+ iommu_dom0_strict = val;
+ else if ( !strncmp(s, "relaxed", ss - s) )
+ iommu_dom0_strict = !val;
+ else
+ rc = -EINVAL;
+
+ s = ss + 1;
+ } while ( *ss );
+
+ return rc;
+}
+
int iommu_domain_init(struct domain *d)
{
struct domain_iommu *hd = dom_iommu(d);
@@ -158,7 +190,7 @@ static void __hwdom_init check_hwdom_reqs(struct domain *d)
arch_iommu_check_autotranslated_hwdom(d);
- if ( iommu_passthrough )
+ if ( iommu_dom0_passthrough )
panic("Dom0 uses paging translated mode, dom0-passthrough must not be "
"enabled\n");
@@ -372,7 +404,7 @@ int __init iommu_setup(void)
bool_t force_intremap = force_iommu && iommu_intremap;
if ( iommu_dom0_strict )
- iommu_passthrough = 0;
+ iommu_dom0_passthrough = false;
if ( iommu_enable )
{
@@ -393,14 +425,14 @@ int __init iommu_setup(void)
if ( !iommu_enabled )
{
iommu_snoop = 0;
- iommu_passthrough = 0;
+ iommu_dom0_passthrough = false;
iommu_dom0_strict = 0;
}
printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
if ( iommu_enabled )
{
printk(" - Dom0 mode: %s\n",
- iommu_passthrough ? "Passthrough" :
+ iommu_dom0_passthrough ? "Passthrough" :
iommu_dom0_strict ? "Strict" : "Relaxed");
printk("Interrupt remapping %sabled\n", iommu_intremap ? "en" : "dis");
tasklet_init(&iommu_pt_cleanup_tasklet, iommu_free_pagetables, 0);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 1710256823..8ac774215b 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1304,7 +1304,7 @@ static void __hwdom_init intel_iommu_hwdom_init(struct domain *d)
{
struct acpi_drhd_unit *drhd;
- if ( !iommu_passthrough && is_pv_domain(d) )
+ if ( !iommu_dom0_passthrough && is_pv_domain(d) )
{
/* Set up 1:1 page table for hardware domain. */
vtd_set_hwdom_mapping(d);
@@ -1391,7 +1391,7 @@ int domain_context_mapping_one(
return res;
}
- if ( iommu_passthrough && is_hardware_domain(domain) )
+ if ( iommu_dom0_passthrough && is_hardware_domain(domain) )
{
context_set_translation_type(*context, CONTEXT_TT_PASS_THRU);
agaw = level_to_agaw(iommu->nr_pt_levels);
@@ -1781,7 +1781,7 @@ static int __must_check intel_iommu_map_page(struct domain *d,
return 0;
/* Do nothing if hardware domain and iommu supports pass thru. */
- if ( iommu_passthrough && is_hardware_domain(d) )
+ if ( iommu_dom0_passthrough && is_hardware_domain(d) )
return 0;
spin_lock(&hd->arch.mapping_lock);
@@ -1826,7 +1826,7 @@ static int __must_check intel_iommu_unmap_page(struct domain *d,
unsigned long gfn)
{
/* Do nothing if hardware domain and iommu supports pass thru. */
- if ( iommu_passthrough && is_hardware_domain(d) )
+ if ( iommu_dom0_passthrough && is_hardware_domain(d) )
return 0;
return dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
@@ -2269,8 +2269,8 @@ int __init intel_vtd_setup(void)
if ( iommu_snoop && !ecap_snp_ctl(iommu->ecap) )
iommu_snoop = 0;
- if ( iommu_passthrough && !ecap_pass_thru(iommu->ecap) )
- iommu_passthrough = 0;
+ if ( iommu_dom0_passthrough && !ecap_pass_thru(iommu->ecap) )
+ iommu_dom0_passthrough = false;
if ( iommu_qinval && !ecap_queued_inval(iommu->ecap) )
iommu_qinval = 0;
@@ -2308,7 +2308,7 @@ int __init intel_vtd_setup(void)
#define P(p,s) printk("Intel VT-d %s %senabled.\n", s, (p)? "" : "not ")
P(iommu_snoop, "Snoop Control");
- P(iommu_passthrough, "Dom0 DMA Passthrough");
+ P(iommu_dom0_passthrough, "Dom0 DMA Passthrough");
P(iommu_qinval, "Queued Invalidation");
P(iommu_intremap, "Interrupt Remapping");
P(iommu_intpost, "Posted Interrupt");
@@ -2330,7 +2330,7 @@ int __init intel_vtd_setup(void)
error:
iommu_enabled = 0;
iommu_snoop = 0;
- iommu_passthrough = 0;
+ iommu_dom0_passthrough = false;
iommu_qinval = 0;
iommu_intremap = 0;
iommu_intpost = 0;
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 6b42e3b876..c0c6975ac4 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -29,13 +29,15 @@
#include <asm/iommu.h>
extern bool_t iommu_enable, iommu_enabled;
-extern bool_t force_iommu, iommu_dom0_strict, iommu_verbose;
-extern bool_t iommu_workaround_bios_bug, iommu_igfx, iommu_passthrough;
+extern bool_t force_iommu, iommu_verbose;
+extern bool_t iommu_workaround_bios_bug, iommu_igfx;
extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
extern bool_t iommu_hap_pt_share;
extern bool_t iommu_debug;
extern bool_t amd_iommu_perdev_intremap;
+extern bool iommu_dom0_strict, iommu_dom0_passthrough;
+
extern unsigned int iommu_dev_iotlb_timeout;
int iommu_setup(void);
--
2.18.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
2018-08-01 11:03 [PATCH v2 0/5] x86/iommu: PVH Dom0 workarounds for missing RMRR entries Roger Pau Monne
2018-08-01 11:03 ` [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal Roger Pau Monne
2018-08-01 11:03 ` [PATCH v2 2/5] iommu: introduce dom0-iommu option Roger Pau Monne
@ 2018-08-01 11:03 ` Roger Pau Monne
2018-08-01 11:21 ` Paul Durrant
` (2 more replies)
2018-08-01 11:03 ` [PATCH v2 4/5] dom0/pvh: change the order of the MMCFG initialization Roger Pau Monne
2018-08-01 11:03 ` [PATCH v2 5/5] x86/iommu: add PVH support to the inclusive options Roger Pau Monne
4 siblings, 3 replies; 29+ messages in thread
From: Roger Pau Monne @ 2018-08-01 11:03 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
Jan Beulich, Roger Pau Monne
Introduce a new dom0-iommu=inclusive generic option that supersedes
iommu_inclusive_mapping. The prevcious behaviour is preserved and the
option should only be enabled by default on Intel hardware.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
- Use dom0-iommu instead of the iommu option.
- Only enable by default on Intel hardware.
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
docs/misc/xen-command-line.markdown | 14 +++++++
xen/drivers/passthrough/arm/iommu.c | 4 ++
xen/drivers/passthrough/iommu.c | 5 +++
xen/drivers/passthrough/vtd/extern.h | 2 -
xen/drivers/passthrough/vtd/iommu.c | 6 ---
xen/drivers/passthrough/vtd/x86/vtd.c | 58 +-------------------------
xen/drivers/passthrough/x86/iommu.c | 60 +++++++++++++++++++++++++++
xen/include/xen/iommu.h | 2 +
8 files changed, 86 insertions(+), 65 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index a2a07cc6c8..30d970bc2e 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1230,6 +1230,17 @@ detection of systems known to misbehave upon accesses to that port.
>> Controls whether to setup DMA remappings for all the host RAM except regions
>> in use by Xen.
+> `inclusive`
+
+> Default: `true` on Intel hardware, `false` otherwise
+
+>> Use this to work around firmware issues providing incorrect RMRR or 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. Also note that if `strict` mode is enabled
+>> then conventional RAM pages not assigned to dom0 will not be mapped.
+
### iommu\_dev\_iotlb\_timeout
> `= <integer>`
@@ -1242,6 +1253,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=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/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/iommu.c b/xen/drivers/passthrough/iommu.c
index 88e23bbd04..6611e13cc2 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -74,6 +74,7 @@ bool_t __read_mostly amd_iommu_perdev_intremap = 1;
custom_param("dom0-iommu", parse_dom0_iommu_param);
bool __hwdom_initdata iommu_dom0_strict;
bool __read_mostly iommu_dom0_passthrough;
+int8_t __hwdom_initdata iommu_dom0_inclusive = -1;
DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
@@ -158,6 +159,8 @@ static int __init parse_dom0_iommu_param(const char *s)
iommu_dom0_strict = val;
else if ( !strncmp(s, "relaxed", ss - s) )
iommu_dom0_strict = !val;
+ else if ( !strncmp(s, "inclusive", ss - s) )
+ iommu_dom0_inclusive = val;
else
rc = -EINVAL;
@@ -240,6 +243,8 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
}
hd->platform_ops->hwdom_init(d);
+
+ arch_iommu_hwdom_init(d);
}
void iommu_teardown(struct domain *d)
diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
index fb7edfaef9..91cadc602e 100644
--- a/xen/drivers/passthrough/vtd/extern.h
+++ b/xen/drivers/passthrough/vtd/extern.h
@@ -99,6 +99,4 @@ void pci_vtd_quirk(const struct pci_dev *);
bool_t platform_supports_intremap(void);
bool_t platform_supports_x2apic(void);
-void vtd_set_hwdom_mapping(struct domain *d);
-
#endif // _VTD_EXTERN_H_
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 8ac774215b..c880b0ce21 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1304,12 +1304,6 @@ static void __hwdom_init intel_iommu_hwdom_init(struct domain *d)
{
struct acpi_drhd_unit *drhd;
- if ( !iommu_dom0_passthrough && is_pv_domain(d) )
- {
- /* Set up 1:1 page table for hardware domain. */
- vtd_set_hwdom_mapping(d);
- }
-
setup_hwdom_pci_devices(d, setup_hwdom_device);
setup_hwdom_rmrr(d);
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
index 00a9891005..20323051d0 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -25,7 +25,6 @@
#include <xen/irq.h>
#include <xen/numa.h>
#include <asm/fixmap.h>
-#include <asm/setup.h>
#include "../iommu.h"
#include "../dmar.h"
#include "../vtd.h"
@@ -35,8 +34,7 @@
* iommu_inclusive_mapping: when set, all memory below 4GB is included in dom0
* 1:1 iommu mappings except xen and unusable regions.
*/
-static bool_t __hwdom_initdata iommu_inclusive_mapping = 1;
-boolean_param("iommu_inclusive_mapping", iommu_inclusive_mapping);
+boolean_param("iommu_inclusive_mapping", iommu_dom0_inclusive);
void *map_vtd_domain_page(u64 maddr)
{
@@ -108,57 +106,3 @@ void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
spin_unlock(&d->event_lock);
}
-void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
-{
- unsigned long i, top, max_pfn;
-
- BUG_ON(!is_hardware_domain(d));
-
- max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
- top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
-
- for ( i = 0; i < top; i++ )
- {
- unsigned long pfn = pdx_to_pfn(i);
- bool map;
- int rc;
-
- /*
- * Set up 1:1 mapping for dom0. Default to include only
- * conventional RAM areas and let RMRRs include needed reserved
- * regions. When set, the inclusive mapping additionally maps in
- * every pfn up to 4GB except those that fall in unusable ranges.
- */
- if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
- continue;
-
- if ( iommu_inclusive_mapping && pfn <= max_pfn )
- map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
- else
- map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
-
- if ( !map )
- continue;
-
- /* Exclude Xen bits */
- if ( xen_in_range(pfn) )
- continue;
-
- /*
- * If dom0-strict mode is enabled then exclude conventional RAM
- * and let the common code map dom0's pages.
- */
- if ( iommu_dom0_strict &&
- page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
- continue;
-
- rc = iommu_map_page(d, pfn, pfn, IOMMUF_readable|IOMMUF_writable);
- if ( rc )
- printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed: %d\n",
- d->domain_id, rc);
-
- if (!(i & 0xfffff))
- process_pending_softirqs();
- }
-}
-
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index 68182afd91..bf6edf4c04 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -20,6 +20,8 @@
#include <xen/softirq.h>
#include <xsm/xsm.h>
+#include <asm/setup.h>
+
void iommu_update_ire_from_apic(
unsigned int apic, unsigned int reg, unsigned int value)
{
@@ -132,6 +134,64 @@ void arch_iommu_domain_destroy(struct domain *d)
{
}
+void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
+{
+ unsigned long i, top, max_pfn;
+
+ BUG_ON(!is_hardware_domain(d));
+
+ /* Set the default value of inclusive depending on the hardware. */
+ if ( iommu_dom0_inclusive == -1 )
+ iommu_dom0_inclusive = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
+
+ max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
+ top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
+
+ for ( i = 0; i < top; i++ )
+ {
+ unsigned long pfn = pdx_to_pfn(i);
+ bool map;
+ int rc;
+
+ /*
+ * Set up 1:1 mapping for dom0. Default to include only
+ * conventional RAM areas and let RMRRs include needed reserved
+ * regions. When set, the inclusive mapping additionally maps in
+ * every pfn up to 4GB except those that fall in unusable ranges.
+ */
+ if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
+ continue;
+
+ if ( iommu_dom0_inclusive && pfn <= max_pfn )
+ map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
+ else
+ map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
+
+ if ( !map )
+ continue;
+
+ /* Exclude Xen bits */
+ if ( xen_in_range(pfn) )
+ continue;
+
+ /*
+ * If dom0-strict mode is enabled then exclude conventional RAM
+ * and let the common code map dom0's pages.
+ */
+ if ( iommu_dom0_strict &&
+ page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
+ continue;
+
+ rc = iommu_map_page(d, pfn, pfn, IOMMUF_readable|IOMMUF_writable);
+ if ( rc )
+ printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
+ d->domain_id, rc);
+
+ if (!(i & 0xfffff))
+ process_pending_softirqs();
+ }
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index c0c6975ac4..99e5b89c0f 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -37,6 +37,7 @@ extern bool_t iommu_debug;
extern bool_t amd_iommu_perdev_intremap;
extern bool iommu_dom0_strict, iommu_dom0_passthrough;
+extern int8_t iommu_dom0_inclusive;
extern unsigned int iommu_dev_iotlb_timeout;
@@ -51,6 +52,7 @@ void arch_iommu_domain_destroy(struct domain *d);
int arch_iommu_domain_init(struct domain *d);
int arch_iommu_populate_page_table(struct domain *d);
void arch_iommu_check_autotranslated_hwdom(struct domain *d);
+void arch_iommu_hwdom_init(struct domain *d);
int iommu_construct(struct domain *d);
--
2.18.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 4/5] dom0/pvh: change the order of the MMCFG initialization
2018-08-01 11:03 [PATCH v2 0/5] x86/iommu: PVH Dom0 workarounds for missing RMRR entries Roger Pau Monne
` (2 preceding siblings ...)
2018-08-01 11:03 ` [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu Roger Pau Monne
@ 2018-08-01 11:03 ` Roger Pau Monne
2018-08-01 11:03 ` [PATCH v2 5/5] x86/iommu: add PVH support to the inclusive options Roger Pau Monne
4 siblings, 0 replies; 29+ messages in thread
From: Roger Pau Monne @ 2018-08-01 11:03 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monne
So it's done before the iommu is initialized. This is required in
order to be able to fetch the MMCFG regions from the domain struct.
No functional change.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
- New in this version.
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/x86/hvm/dom0_build.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index f0cd63b1ec..5065729106 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -1100,6 +1100,13 @@ int __init dom0_construct_pvh(struct domain *d, const module_t *image,
return rc;
}
+ /*
+ * NB: MMCFG initialization needs to be performed before iommu
+ * initialization so the iommu code can fetch the MMCFG regions used by the
+ * domain.
+ */
+ pvh_setup_mmcfg(d);
+
iommu_hwdom_init(d);
rc = pvh_load_kernel(d, image, image_headroom, initrd, bootstrap_map(image),
@@ -1124,8 +1131,6 @@ int __init dom0_construct_pvh(struct domain *d, const module_t *image,
return rc;
}
- pvh_setup_mmcfg(d);
-
printk("WARNING: PVH is an experimental mode with limited functionality\n");
return 0;
}
--
2.18.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 5/5] x86/iommu: add PVH support to the inclusive options
2018-08-01 11:03 [PATCH v2 0/5] x86/iommu: PVH Dom0 workarounds for missing RMRR entries Roger Pau Monne
` (3 preceding siblings ...)
2018-08-01 11:03 ` [PATCH v2 4/5] dom0/pvh: change the order of the MMCFG initialization Roger Pau Monne
@ 2018-08-01 11:03 ` Roger Pau Monne
2018-08-01 11:27 ` Paul Durrant
2018-08-02 8:03 ` Tian, Kevin
4 siblings, 2 replies; 29+ messages in thread
From: Roger Pau Monne @ 2018-08-01 11:03 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich,
Roger Pau Monne
Several people have reported hardware issues (malfunctioning USB
controllers) due to iommu page faults on Intel hardware. Those faults
are caused by missing RMRR (VTd) entries in the ACPI tables. Those can
be worked around on VTd hardware by manually adding RMRR entries on
the command line, this is however limited to Intel hardware and quite
cumbersome to do.
In order to solve those issues add a new dom0-iommu=reserved option
that identity maps all regions marked as reserved in the memory map.
Note that regions used by devices emulated by Xen (LAPIC, IO-APIC or
PCIe MCFG regions) are specifically avoided. Note that this option is
available to a PVH Dom0 (as opposed to the inclusive option which only
works for PV Dom0).
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
- Introduce a new reserved option instead of abusing the inclusive
option.
- Use the same helper function for PV and PVH in order to decide if a
page should be added to the domain page tables.
- Use the data inside of the domain struct to detect overlaps with
emulated MMIO regions.
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
docs/misc/xen-command-line.markdown | 9 +++
xen/arch/x86/hvm/io.c | 5 ++
xen/drivers/passthrough/iommu.c | 3 +
xen/drivers/passthrough/x86/iommu.c | 88 +++++++++++++++++++++--------
xen/include/asm-x86/hvm/io.h | 3 +
xen/include/xen/iommu.h | 2 +-
6 files changed, 86 insertions(+), 24 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 30d970bc2e..526a96ffc5 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1241,6 +1241,15 @@ detection of systems known to misbehave upon accesses to that port.
>> applicable to a PV dom0. Also note that if `strict` mode is enabled
>> then conventional RAM pages not assigned to dom0 will not be mapped.
+> `reserved`
+
+> Default: `true` on Intel hardware, `false` otherwise
+
+>> Use this to work around firmware issues providing incorrect RMRR or IVMD
+>> entries. Rather than only mapping RAM pages for IOMMU accesses for Dom0,
+>> all memory regions marked as reserved in the memory map that don't overlap
+>> with any MMIO region from emulated devices will be identity mapped.
+
### iommu\_dev\_iotlb\_timeout
> `= <integer>`
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index bf4d8748d3..5e01c33890 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -404,6 +404,11 @@ static const struct hvm_mmcfg *vpci_mmcfg_find(const struct domain *d,
return NULL;
}
+bool vpci_mmcfg_address(const struct domain *d, paddr_t addr)
+{
+ return vpci_mmcfg_find(d, addr);
+}
+
static unsigned int vpci_mmcfg_decode_addr(const struct hvm_mmcfg *mmcfg,
paddr_t addr, pci_sbdf_t *sbdf)
{
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 6611e13cc2..a3eb7c5b7f 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -75,6 +75,7 @@ custom_param("dom0-iommu", parse_dom0_iommu_param);
bool __hwdom_initdata iommu_dom0_strict;
bool __read_mostly iommu_dom0_passthrough;
int8_t __hwdom_initdata iommu_dom0_inclusive = -1;
+int8_t __hwdom_initdata iommu_dom0_reserved = -1;
DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
@@ -161,6 +162,8 @@ static int __init parse_dom0_iommu_param(const char *s)
iommu_dom0_strict = !val;
else if ( !strncmp(s, "inclusive", ss - s) )
iommu_dom0_inclusive = val;
+ else if ( !strncmp(s, "reserved", ss - s) )
+ iommu_dom0_reserved = val;
else
rc = -EINVAL;
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index bf6edf4c04..66c5cc28ed 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -20,6 +20,7 @@
#include <xen/softirq.h>
#include <xsm/xsm.h>
+#include <asm/hvm/io.h>
#include <asm/setup.h>
void iommu_update_ire_from_apic(
@@ -134,15 +135,75 @@ void arch_iommu_domain_destroy(struct domain *d)
{
}
+static bool __hwdom_init iommu_map(const struct domain *d, unsigned long pfn,
+ unsigned long max_pfn)
+{
+ unsigned int i;
+
+ /*
+ * Ignore any address below 1MB, that's already identity mapped by the
+ * domain builder for HVM.
+ */
+ if ( is_hvm_domain(d) && pfn < PFN_DOWN(MB(1)) )
+ return false;
+
+ /*
+ * If dom0-strict mode is enabled then exclude conventional RAM and let the
+ * common code map dom0's pages.
+ */
+ if ( page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) &&
+ (iommu_dom0_strict || is_hvm_domain(d)) )
+ return false;
+ if ( page_is_ram_type(pfn, RAM_TYPE_RESERVED) &&
+ (!iommu_dom0_reserved || !iommu_dom0_inclusive) )
+ return false;
+ if ( !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE) &&
+ !page_is_ram_type(pfn, RAM_TYPE_RESERVED) &&
+ !page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) &&
+ (!iommu_dom0_inclusive || pfn > max_pfn) )
+ return false;
+
+ /* Check that it doesn't overlap with the LAPIC */
+ if ( has_vlapic(d) )
+ {
+ const struct vcpu *v;
+
+ for_each_vcpu(d, v)
+ if ( pfn == PFN_DOWN(vlapic_base_address(vcpu_vlapic(v))) )
+ return false;
+ }
+ /* ... or the IO-APIC */
+ for ( i = 0; has_vioapic(d) && i < d->arch.hvm_domain.nr_vioapics; i++ )
+ if ( pfn == PFN_DOWN(domain_vioapic(d, i)->base_address) )
+ return false;
+ /*
+ * ... or the PCIe MCFG regions.
+ * TODO: runtime added MMCFG regions are not checked to make sure they
+ * don't overlap with already mapped regions, thus preventing trapping.
+ */
+ if ( has_vpci(d) && vpci_mmcfg_address(d, pfn << PAGE_SHIFT) )
+ return false;
+
+ return true;
+}
+
void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
{
unsigned long i, top, max_pfn;
+ if ( iommu_dom0_passthrough )
+ return;
+
BUG_ON(!is_hardware_domain(d));
- /* Set the default value of inclusive depending on the hardware. */
+ /*
+ * Set the default value of inclusive and reserved depending on the
+ * hardware.
+ */
if ( iommu_dom0_inclusive == -1 )
iommu_dom0_inclusive = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
+ if ( iommu_dom0_reserved == -1 )
+ iommu_dom0_reserved = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
@@ -150,7 +211,6 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
for ( i = 0; i < top; i++ )
{
unsigned long pfn = pdx_to_pfn(i);
- bool map;
int rc;
/*
@@ -159,27 +219,9 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
* regions. When set, the inclusive mapping additionally maps in
* every pfn up to 4GB except those that fall in unusable ranges.
*/
- if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
- continue;
-
- if ( iommu_dom0_inclusive && pfn <= max_pfn )
- map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
- else
- map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
-
- if ( !map )
- continue;
-
- /* Exclude Xen bits */
- if ( xen_in_range(pfn) )
- continue;
-
- /*
- * If dom0-strict mode is enabled then exclude conventional RAM
- * and let the common code map dom0's pages.
- */
- if ( iommu_dom0_strict &&
- page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
+ if ( (pfn > max_pfn && !mfn_valid(_mfn(pfn))) ||
+ /* Exclude Xen bits */
+ xen_in_range(pfn) || !iommu_map(d, pfn, max_pfn) )
continue;
rc = iommu_map_page(d, pfn, pfn, IOMMUF_readable|IOMMUF_writable);
diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
index e6b6ed0b92..8cca456b55 100644
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -180,6 +180,9 @@ int register_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
/* Destroy tracked MMCFG areas. */
void destroy_vpci_mmcfg(struct domain *d);
+/* Check if an address is between a MMCFG region for a domain. */
+bool vpci_mmcfg_address(const struct domain *d, paddr_t addr);
+
#endif /* __ASM_X86_HVM_IO_H__ */
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 99e5b89c0f..fed1b1ea7a 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -37,7 +37,7 @@ extern bool_t iommu_debug;
extern bool_t amd_iommu_perdev_intremap;
extern bool iommu_dom0_strict, iommu_dom0_passthrough;
-extern int8_t iommu_dom0_inclusive;
+extern int8_t iommu_dom0_inclusive, iommu_dom0_reserved;
extern unsigned int iommu_dev_iotlb_timeout;
--
2.18.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-01 11:03 ` [PATCH v2 2/5] iommu: introduce dom0-iommu option Roger Pau Monne
@ 2018-08-01 11:15 ` Paul Durrant
2018-08-01 11:16 ` Andrew Cooper
2018-08-02 7:46 ` Tian, Kevin
2 siblings, 0 replies; 29+ messages in thread
From: Paul Durrant @ 2018-08-01 11:15 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jan Beulich,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Suravee Suthikulpanit, Ian Jackson,
Brian Woods, Roger Pau Monne
> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Roger Pau Monne
> Sent: 01 August 2018 12:04
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; Suravee
> Suthikulpanit <suravee.suthikulpanit@amd.com>; George Dunlap
> <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Jan Beulich
> <jbeulich@suse.com>; Brian Woods <brian.woods@amd.com>; Roger Pau
> Monne <roger.pau@citrix.com>
> Subject: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu option
>
> To select the iommu configuration used by Dom0. This option supersedes
> iommu=dom0-strict|dom0-passthrough.
>
> No functional change.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Changes since v1:
> - New in this version.
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Brian Woods <brian.woods@amd.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> ---
> docs/misc/xen-command-line.markdown | 32 ++++++++++++++
> xen/arch/x86/x86_64/mm.c | 3 +-
> xen/drivers/passthrough/amd/iommu_init.c | 2 +-
> xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 +-
> xen/drivers/passthrough/iommu.c | 46 +++++++++++++++++----
> xen/drivers/passthrough/vtd/iommu.c | 16 +++----
> xen/include/xen/iommu.h | 6 ++-
> 7 files changed, 88 insertions(+), 21 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> command-line.markdown
> index 65b4754418..a2a07cc6c8 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1150,12 +1150,18 @@ detection of systems known to misbehave upon
> accesses to that port.
>
> > `dom0-passthrough`
>
> +> **WARNING: This command line option is deprecated, and superseded
> by
> +> _dom0-iommu=none_ - using both options in combination is
> undefined.**
> +
> > Default: `false`
>
> >> Control whether to disable DMA remapping for Dom0.
>
> > `dom0-strict`
>
> +> **WARNING: This command line option is deprecated, and superseded
> by
> +> _dom0-iommu=strict_ - using both options in combination is
> undefined.**
> +
> > Default: `false`
>
> >> Control whether to set up DMA remapping only for the memory Dom0
> actually
> @@ -1198,6 +1204,32 @@ detection of systems known to misbehave upon
> accesses to that port.
>
> >> Enable IOMMU debugging code (implies `verbose`).
>
> +### dom0-iommu
> +> `= List of [ none | strict | relaxed ]`
> +
> +> Sub-options are of boolean kind and can be prefixed with `no-` to effect
> the
> +> inverse meaning.
> +
> +> `none`
> +
> +> Default: `false`
> +
> +>> Control whether to disable DMA remapping for Dom0.
> +
> +> `strict`
> +
> +> Default: `false`
> +
> +>> Control whether to set up DMA remapping only for the memory Dom0
> actually
> +>> got assigned.
> +
> +> `relaxed`
> +
> +> Default: `true`
> +
> +>> Controls whether to setup DMA remappings for all the host RAM except
> regions
> +>> in use by Xen.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= <integer>`
>
> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> index cca4ae926e..84226b3326 100644
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1426,7 +1426,8 @@ int memory_add(unsigned long spfn, unsigned long
> epfn, unsigned int pxm)
> if ( ret )
> goto destroy_m2p;
>
> - if ( iommu_enabled && !iommu_passthrough &&
> !need_iommu(hardware_domain) )
> + if ( iommu_enabled && !iommu_dom0_passthrough &&
> + !need_iommu(hardware_domain) )
> {
> for ( i = spfn; i < epfn; i++ )
> if ( iommu_map_page(hardware_domain, i, i,
> IOMMUF_readable|IOMMUF_writable) )
> diff --git a/xen/drivers/passthrough/amd/iommu_init.c
> b/xen/drivers/passthrough/amd/iommu_init.c
> index 474992a75a..ad8c48be1c 100644
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -1062,7 +1062,7 @@ static void __init amd_iommu_init_cleanup(void)
> radix_tree_destroy(&ivrs_maps, xfree);
>
> iommu_enabled = 0;
> - iommu_passthrough = 0;
> + iommu_dom0_passthrough = false;
> iommu_intremap = 0;
> iommuv2_enabled = 0;
> }
> diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> index 12d2695b89..eeacf713e4 100644
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -121,7 +121,7 @@ static void amd_iommu_setup_domain_device(
> BUG_ON( !hd->arch.root_table || !hd->arch.paging_mode ||
> !iommu->dev_table.buffer );
>
> - if ( iommu_passthrough && is_hardware_domain(domain) )
> + if ( iommu_dom0_passthrough && is_hardware_domain(domain) )
> valid = 0;
>
> if ( ats_enabled )
> @@ -256,7 +256,7 @@ static void __hwdom_init
> amd_iommu_hwdom_init(struct domain *d)
> if ( allocate_domain_resources(dom_iommu(d)) )
> BUG();
>
> - if ( !iommu_passthrough && !need_iommu(d) )
> + if ( !iommu_dom0_passthrough && !need_iommu(d) )
> {
> int rc = 0;
>
> diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> index 70d218f910..88e23bbd04 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -22,6 +22,7 @@
> #include <xsm/xsm.h>
>
> static int parse_iommu_param(const char *s);
> +static int parse_dom0_iommu_param(const char *s);
> static void iommu_dump_p2m_table(unsigned char key);
>
> unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
> @@ -52,11 +53,9 @@ custom_param("iommu", parse_iommu_param);
> bool_t __initdata iommu_enable = 1;
> bool_t __read_mostly iommu_enabled;
> bool_t __read_mostly force_iommu;
> -bool_t __hwdom_initdata iommu_dom0_strict;
> bool_t __read_mostly iommu_verbose;
> bool_t __read_mostly iommu_workaround_bios_bug;
> bool_t __read_mostly iommu_igfx = 1;
> -bool_t __read_mostly iommu_passthrough;
> bool_t __read_mostly iommu_snoop = 1;
> bool_t __read_mostly iommu_qinval = 1;
> bool_t __read_mostly iommu_intremap = 1;
> @@ -72,6 +71,10 @@ bool_t __read_mostly iommu_hap_pt_share = 1;
> bool_t __read_mostly iommu_debug;
> bool_t __read_mostly amd_iommu_perdev_intremap = 1;
>
> +custom_param("dom0-iommu", parse_dom0_iommu_param);
> +bool __hwdom_initdata iommu_dom0_strict;
> +bool __read_mostly iommu_dom0_passthrough;
> +
> DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
>
> DEFINE_SPINLOCK(iommu_pt_cleanup_lock);
> @@ -121,7 +124,7 @@ static int __init parse_iommu_param(const char *s)
> else if ( !strncmp(s, "amd-iommu-perdev-intremap", ss - s) )
> amd_iommu_perdev_intremap = val;
> else if ( !strncmp(s, "dom0-passthrough", ss - s) )
> - iommu_passthrough = val;
> + iommu_dom0_passthrough = val;
> else if ( !strncmp(s, "dom0-strict", ss - s) )
> iommu_dom0_strict = val;
> else if ( !strncmp(s, "sharept", ss - s) )
> @@ -135,6 +138,35 @@ static int __init parse_iommu_param(const char *s)
> return rc;
> }
>
> +static int __init parse_dom0_iommu_param(const char *s)
> +{
> + const char *ss;
> + int val, rc = 0;
> +
> + do {
> + val = !!strncmp(s, "no-", 3);
> + if ( !val )
> + s += 3;
> +
> + ss = strchr(s, ',');
> + if ( !ss )
> + ss = strchr(s, '\0');
> +
> + if ( !strncmp(s, "none", ss - s) )
> + iommu_dom0_passthrough = val;
> + else if ( !strncmp(s, "strict", ss - s) )
> + iommu_dom0_strict = val;
> + else if ( !strncmp(s, "relaxed", ss - s) )
> + iommu_dom0_strict = !val;
> + else
> + rc = -EINVAL;
> +
> + s = ss + 1;
> + } while ( *ss );
> +
> + return rc;
> +}
> +
> int iommu_domain_init(struct domain *d)
> {
> struct domain_iommu *hd = dom_iommu(d);
> @@ -158,7 +190,7 @@ static void __hwdom_init check_hwdom_reqs(struct
> domain *d)
>
> arch_iommu_check_autotranslated_hwdom(d);
>
> - if ( iommu_passthrough )
> + if ( iommu_dom0_passthrough )
> panic("Dom0 uses paging translated mode, dom0-passthrough must not
> be "
> "enabled\n");
>
> @@ -372,7 +404,7 @@ int __init iommu_setup(void)
> bool_t force_intremap = force_iommu && iommu_intremap;
>
> if ( iommu_dom0_strict )
> - iommu_passthrough = 0;
> + iommu_dom0_passthrough = false;
>
> if ( iommu_enable )
> {
> @@ -393,14 +425,14 @@ int __init iommu_setup(void)
> if ( !iommu_enabled )
> {
> iommu_snoop = 0;
> - iommu_passthrough = 0;
> + iommu_dom0_passthrough = false;
> iommu_dom0_strict = 0;
> }
> printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
> if ( iommu_enabled )
> {
> printk(" - Dom0 mode: %s\n",
> - iommu_passthrough ? "Passthrough" :
> + iommu_dom0_passthrough ? "Passthrough" :
> iommu_dom0_strict ? "Strict" : "Relaxed");
> printk("Interrupt remapping %sabled\n", iommu_intremap ? "en" :
> "dis");
> tasklet_init(&iommu_pt_cleanup_tasklet, iommu_free_pagetables, 0);
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 1710256823..8ac774215b 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1304,7 +1304,7 @@ static void __hwdom_init
> intel_iommu_hwdom_init(struct domain *d)
> {
> struct acpi_drhd_unit *drhd;
>
> - if ( !iommu_passthrough && is_pv_domain(d) )
> + if ( !iommu_dom0_passthrough && is_pv_domain(d) )
> {
> /* Set up 1:1 page table for hardware domain. */
> vtd_set_hwdom_mapping(d);
> @@ -1391,7 +1391,7 @@ int domain_context_mapping_one(
> return res;
> }
>
> - if ( iommu_passthrough && is_hardware_domain(domain) )
> + if ( iommu_dom0_passthrough && is_hardware_domain(domain) )
> {
> context_set_translation_type(*context, CONTEXT_TT_PASS_THRU);
> agaw = level_to_agaw(iommu->nr_pt_levels);
> @@ -1781,7 +1781,7 @@ static int __must_check
> intel_iommu_map_page(struct domain *d,
> return 0;
>
> /* Do nothing if hardware domain and iommu supports pass thru. */
> - if ( iommu_passthrough && is_hardware_domain(d) )
> + if ( iommu_dom0_passthrough && is_hardware_domain(d) )
> return 0;
>
> spin_lock(&hd->arch.mapping_lock);
> @@ -1826,7 +1826,7 @@ static int __must_check
> intel_iommu_unmap_page(struct domain *d,
> unsigned long gfn)
> {
> /* Do nothing if hardware domain and iommu supports pass thru. */
> - if ( iommu_passthrough && is_hardware_domain(d) )
> + if ( iommu_dom0_passthrough && is_hardware_domain(d) )
> return 0;
>
> return dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
> @@ -2269,8 +2269,8 @@ int __init intel_vtd_setup(void)
> if ( iommu_snoop && !ecap_snp_ctl(iommu->ecap) )
> iommu_snoop = 0;
>
> - if ( iommu_passthrough && !ecap_pass_thru(iommu->ecap) )
> - iommu_passthrough = 0;
> + if ( iommu_dom0_passthrough && !ecap_pass_thru(iommu->ecap) )
> + iommu_dom0_passthrough = false;
>
> if ( iommu_qinval && !ecap_queued_inval(iommu->ecap) )
> iommu_qinval = 0;
> @@ -2308,7 +2308,7 @@ int __init intel_vtd_setup(void)
>
> #define P(p,s) printk("Intel VT-d %s %senabled.\n", s, (p)? "" : "not ")
> P(iommu_snoop, "Snoop Control");
> - P(iommu_passthrough, "Dom0 DMA Passthrough");
> + P(iommu_dom0_passthrough, "Dom0 DMA Passthrough");
> P(iommu_qinval, "Queued Invalidation");
> P(iommu_intremap, "Interrupt Remapping");
> P(iommu_intpost, "Posted Interrupt");
> @@ -2330,7 +2330,7 @@ int __init intel_vtd_setup(void)
> error:
> iommu_enabled = 0;
> iommu_snoop = 0;
> - iommu_passthrough = 0;
> + iommu_dom0_passthrough = false;
> iommu_qinval = 0;
> iommu_intremap = 0;
> iommu_intpost = 0;
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 6b42e3b876..c0c6975ac4 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -29,13 +29,15 @@
> #include <asm/iommu.h>
>
> extern bool_t iommu_enable, iommu_enabled;
> -extern bool_t force_iommu, iommu_dom0_strict, iommu_verbose;
> -extern bool_t iommu_workaround_bios_bug, iommu_igfx,
> iommu_passthrough;
> +extern bool_t force_iommu, iommu_verbose;
> +extern bool_t iommu_workaround_bios_bug, iommu_igfx;
> extern bool_t iommu_snoop, iommu_qinval, iommu_intremap,
> iommu_intpost;
> extern bool_t iommu_hap_pt_share;
> extern bool_t iommu_debug;
> extern bool_t amd_iommu_perdev_intremap;
>
> +extern bool iommu_dom0_strict, iommu_dom0_passthrough;
> +
> extern unsigned int iommu_dev_iotlb_timeout;
>
> int iommu_setup(void);
> --
> 2.18.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-01 11:03 ` [PATCH v2 2/5] iommu: introduce dom0-iommu option Roger Pau Monne
2018-08-01 11:15 ` Paul Durrant
@ 2018-08-01 11:16 ` Andrew Cooper
2018-08-02 7:46 ` Tian, Kevin
2 siblings, 0 replies; 29+ messages in thread
From: Andrew Cooper @ 2018-08-01 11:16 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
George Dunlap, Tim Deegan, Ian Jackson, Julien Grall,
Jan Beulich, Brian Woods
On 01/08/18 12:03, Roger Pau Monne wrote:
> @@ -1198,6 +1204,32 @@ detection of systems known to misbehave upon accesses to that port.
>
> >> Enable IOMMU debugging code (implies `verbose`).
>
> +### dom0-iommu
> +> `= List of [ none | strict | relaxed ]`
> +
> +> Sub-options are of boolean kind and can be prefixed with `no-` to effect the
> +> inverse meaning.
> +
> +> `none`
> +
> +> Default: `false`
> +
> +>> Control whether to disable DMA remapping for Dom0.
As a minor style issue, please could you follow the style of the
spec-ctrl=? I haven't had time to fix the iommu= formatting to avoid
using block quotations for all the text.
~Andrew
> +
> +> `strict`
> +
> +> Default: `false`
> +
> +>> Control whether to set up DMA remapping only for the memory Dom0 actually
> +>> got assigned.
> +
> +> `relaxed`
> +
> +> Default: `true`
> +
> +>> Controls whether to setup DMA remappings for all the host RAM except regions
> +>> in use by Xen.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= <integer>`
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal
2018-08-01 11:03 ` [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal Roger Pau Monne
@ 2018-08-01 11:16 ` Paul Durrant
2018-08-02 7:36 ` Tian, Kevin
1 sibling, 0 replies; 29+ messages in thread
From: Paul Durrant @ 2018-08-01 11:16 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Roger Pau Monne
> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Roger Pau Monne
> Sent: 01 August 2018 12:04
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian <kevin.tian@intel.com>; Roger Pau Monne
> <roger.pau@citrix.com>
> Subject: [Xen-devel] [PATCH v2 1/5] iommu/vtd: cleanup
> vtd_set_hwdom_mapping after ia64 removal
>
> Remove the handling for different page sizes now that ia64 is gone.
>
> No functional change.
>
> Reported by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Changes since v1:
> - New in this version.
> ---
> Cc: Kevin Tian <kevin.tian@intel.com>
> ---
> xen/drivers/passthrough/vtd/x86/vtd.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c
> b/xen/drivers/passthrough/vtd/x86/vtd.c
> index cc2bfea162..00a9891005 100644
> --- a/xen/drivers/passthrough/vtd/x86/vtd.c
> +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
> @@ -110,7 +110,7 @@ void hvm_dpci_isairq_eoi(struct domain *d, unsigned
> int isairq)
>
> void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
> {
> - unsigned long i, j, tmp, top, max_pfn;
> + unsigned long i, top, max_pfn;
>
> BUG_ON(!is_hardware_domain(d));
>
> @@ -121,7 +121,7 @@ void __hwdom_init vtd_set_hwdom_mapping(struct
> domain *d)
> {
> unsigned long pfn = pdx_to_pfn(i);
> bool map;
> - int rc = 0;
> + int rc;
>
> /*
> * Set up 1:1 mapping for dom0. Default to include only
> @@ -152,21 +152,12 @@ void __hwdom_init
> vtd_set_hwdom_mapping(struct domain *d)
> page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> continue;
>
> - tmp = 1 << (PAGE_SHIFT - PAGE_SHIFT_4K);
> - for ( j = 0; j < tmp; j++ )
> - {
> - int ret = iommu_map_page(d, pfn * tmp + j, pfn * tmp + j,
> - IOMMUF_readable|IOMMUF_writable);
> -
> - if ( !rc )
> - rc = ret;
> - }
> -
> + rc = iommu_map_page(d, pfn, pfn,
> IOMMUF_readable|IOMMUF_writable);
> if ( rc )
> printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed:
> %d\n",
> d->domain_id, rc);
>
> - if (!(i & (0xfffff >> (PAGE_SHIFT - PAGE_SHIFT_4K))))
> + if (!(i & 0xfffff))
> process_pending_softirqs();
> }
> }
> --
> 2.18.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
2018-08-01 11:03 ` [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu Roger Pau Monne
@ 2018-08-01 11:21 ` Paul Durrant
2018-08-02 7:58 ` Tian, Kevin
2018-08-03 9:09 ` Julien Grall
2 siblings, 0 replies; 29+ messages in thread
From: Paul Durrant @ 2018-08-01 11:21 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Andrew Cooper,
Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, Ian Jackson,
Roger Pau Monne
> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Roger Pau Monne
> Sent: 01 August 2018 12:04
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Jan Beulich
> <jbeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: [Xen-devel] [PATCH v2 3/5] iommu: make
> iommu_inclusive_mapping a suboption of dom0-iommu
>
> Introduce a new dom0-iommu=inclusive generic option that supersedes
> iommu_inclusive_mapping. The prevcious behaviour is preserved and the
s/prevcious/previous
> option should only be enabled by default on Intel hardware.
>
> No functional change intended.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Otherwise LGTM.
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Changes since v1:
> - Use dom0-iommu instead of the iommu option.
> - Only enable by default on Intel hardware.
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> ---
> docs/misc/xen-command-line.markdown | 14 +++++++
> xen/drivers/passthrough/arm/iommu.c | 4 ++
> xen/drivers/passthrough/iommu.c | 5 +++
> xen/drivers/passthrough/vtd/extern.h | 2 -
> xen/drivers/passthrough/vtd/iommu.c | 6 ---
> xen/drivers/passthrough/vtd/x86/vtd.c | 58 +-------------------------
> xen/drivers/passthrough/x86/iommu.c | 60
> +++++++++++++++++++++++++++
> xen/include/xen/iommu.h | 2 +
> 8 files changed, 86 insertions(+), 65 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> command-line.markdown
> index a2a07cc6c8..30d970bc2e 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1230,6 +1230,17 @@ detection of systems known to misbehave upon
> accesses to that port.
> >> Controls whether to setup DMA remappings for all the host RAM except
> regions
> >> in use by Xen.
>
> +> `inclusive`
> +
> +> Default: `true` on Intel hardware, `false` otherwise
> +
> +>> Use this to work around firmware issues providing incorrect RMRR or
> 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. Also note that if `strict` mode is enabled
> +>> then conventional RAM pages not assigned to dom0 will not be mapped.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= <integer>`
>
> @@ -1242,6 +1253,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=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/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/iommu.c
> b/xen/drivers/passthrough/iommu.c
> index 88e23bbd04..6611e13cc2 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -74,6 +74,7 @@ bool_t __read_mostly amd_iommu_perdev_intremap
> = 1;
> custom_param("dom0-iommu", parse_dom0_iommu_param);
> bool __hwdom_initdata iommu_dom0_strict;
> bool __read_mostly iommu_dom0_passthrough;
> +int8_t __hwdom_initdata iommu_dom0_inclusive = -1;
>
> DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
>
> @@ -158,6 +159,8 @@ static int __init parse_dom0_iommu_param(const
> char *s)
> iommu_dom0_strict = val;
> else if ( !strncmp(s, "relaxed", ss - s) )
> iommu_dom0_strict = !val;
> + else if ( !strncmp(s, "inclusive", ss - s) )
> + iommu_dom0_inclusive = val;
> else
> rc = -EINVAL;
>
> @@ -240,6 +243,8 @@ void __hwdom_init iommu_hwdom_init(struct
> domain *d)
> }
>
> hd->platform_ops->hwdom_init(d);
> +
> + arch_iommu_hwdom_init(d);
> }
>
> void iommu_teardown(struct domain *d)
> diff --git a/xen/drivers/passthrough/vtd/extern.h
> b/xen/drivers/passthrough/vtd/extern.h
> index fb7edfaef9..91cadc602e 100644
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -99,6 +99,4 @@ void pci_vtd_quirk(const struct pci_dev *);
> bool_t platform_supports_intremap(void);
> bool_t platform_supports_x2apic(void);
>
> -void vtd_set_hwdom_mapping(struct domain *d);
> -
> #endif // _VTD_EXTERN_H_
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 8ac774215b..c880b0ce21 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1304,12 +1304,6 @@ static void __hwdom_init
> intel_iommu_hwdom_init(struct domain *d)
> {
> struct acpi_drhd_unit *drhd;
>
> - if ( !iommu_dom0_passthrough && is_pv_domain(d) )
> - {
> - /* Set up 1:1 page table for hardware domain. */
> - vtd_set_hwdom_mapping(d);
> - }
> -
> setup_hwdom_pci_devices(d, setup_hwdom_device);
> setup_hwdom_rmrr(d);
>
> diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c
> b/xen/drivers/passthrough/vtd/x86/vtd.c
> index 00a9891005..20323051d0 100644
> --- a/xen/drivers/passthrough/vtd/x86/vtd.c
> +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
> @@ -25,7 +25,6 @@
> #include <xen/irq.h>
> #include <xen/numa.h>
> #include <asm/fixmap.h>
> -#include <asm/setup.h>
> #include "../iommu.h"
> #include "../dmar.h"
> #include "../vtd.h"
> @@ -35,8 +34,7 @@
> * iommu_inclusive_mapping: when set, all memory below 4GB is included in
> dom0
> * 1:1 iommu mappings except xen and unusable regions.
> */
> -static bool_t __hwdom_initdata iommu_inclusive_mapping = 1;
> -boolean_param("iommu_inclusive_mapping", iommu_inclusive_mapping);
> +boolean_param("iommu_inclusive_mapping", iommu_dom0_inclusive);
>
> void *map_vtd_domain_page(u64 maddr)
> {
> @@ -108,57 +106,3 @@ void hvm_dpci_isairq_eoi(struct domain *d,
> unsigned int isairq)
> spin_unlock(&d->event_lock);
> }
>
> -void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
> -{
> - unsigned long i, top, max_pfn;
> -
> - BUG_ON(!is_hardware_domain(d));
> -
> - max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> - top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> -
> - for ( i = 0; i < top; i++ )
> - {
> - unsigned long pfn = pdx_to_pfn(i);
> - bool map;
> - int rc;
> -
> - /*
> - * Set up 1:1 mapping for dom0. Default to include only
> - * conventional RAM areas and let RMRRs include needed reserved
> - * regions. When set, the inclusive mapping additionally maps in
> - * every pfn up to 4GB except those that fall in unusable ranges.
> - */
> - if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
> - continue;
> -
> - if ( iommu_inclusive_mapping && pfn <= max_pfn )
> - map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> - else
> - map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> -
> - if ( !map )
> - continue;
> -
> - /* Exclude Xen bits */
> - if ( xen_in_range(pfn) )
> - continue;
> -
> - /*
> - * If dom0-strict mode is enabled then exclude conventional RAM
> - * and let the common code map dom0's pages.
> - */
> - if ( iommu_dom0_strict &&
> - page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> - continue;
> -
> - rc = iommu_map_page(d, pfn, pfn,
> IOMMUF_readable|IOMMUF_writable);
> - if ( rc )
> - printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed:
> %d\n",
> - d->domain_id, rc);
> -
> - if (!(i & 0xfffff))
> - process_pending_softirqs();
> - }
> -}
> -
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index 68182afd91..bf6edf4c04 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -20,6 +20,8 @@
> #include <xen/softirq.h>
> #include <xsm/xsm.h>
>
> +#include <asm/setup.h>
> +
> void iommu_update_ire_from_apic(
> unsigned int apic, unsigned int reg, unsigned int value)
> {
> @@ -132,6 +134,64 @@ void arch_iommu_domain_destroy(struct domain
> *d)
> {
> }
>
> +void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> +{
> + unsigned long i, top, max_pfn;
> +
> + BUG_ON(!is_hardware_domain(d));
> +
> + /* Set the default value of inclusive depending on the hardware. */
> + if ( iommu_dom0_inclusive == -1 )
> + iommu_dom0_inclusive = boot_cpu_data.x86_vendor ==
> X86_VENDOR_INTEL;
> +
> + max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> + top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> +
> + for ( i = 0; i < top; i++ )
> + {
> + unsigned long pfn = pdx_to_pfn(i);
> + bool map;
> + int rc;
> +
> + /*
> + * Set up 1:1 mapping for dom0. Default to include only
> + * conventional RAM areas and let RMRRs include needed reserved
> + * regions. When set, the inclusive mapping additionally maps in
> + * every pfn up to 4GB except those that fall in unusable ranges.
> + */
> + if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
> + continue;
> +
> + if ( iommu_dom0_inclusive && pfn <= max_pfn )
> + map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> + else
> + map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> +
> + if ( !map )
> + continue;
> +
> + /* Exclude Xen bits */
> + if ( xen_in_range(pfn) )
> + continue;
> +
> + /*
> + * If dom0-strict mode is enabled then exclude conventional RAM
> + * and let the common code map dom0's pages.
> + */
> + if ( iommu_dom0_strict &&
> + page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> + continue;
> +
> + rc = iommu_map_page(d, pfn, pfn,
> IOMMUF_readable|IOMMUF_writable);
> + if ( rc )
> + printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
> + d->domain_id, rc);
> +
> + if (!(i & 0xfffff))
> + process_pending_softirqs();
> + }
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index c0c6975ac4..99e5b89c0f 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -37,6 +37,7 @@ extern bool_t iommu_debug;
> extern bool_t amd_iommu_perdev_intremap;
>
> extern bool iommu_dom0_strict, iommu_dom0_passthrough;
> +extern int8_t iommu_dom0_inclusive;
>
> extern unsigned int iommu_dev_iotlb_timeout;
>
> @@ -51,6 +52,7 @@ void arch_iommu_domain_destroy(struct domain *d);
> int arch_iommu_domain_init(struct domain *d);
> int arch_iommu_populate_page_table(struct domain *d);
> void arch_iommu_check_autotranslated_hwdom(struct domain *d);
> +void arch_iommu_hwdom_init(struct domain *d);
>
> int iommu_construct(struct domain *d);
>
> --
> 2.18.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 5/5] x86/iommu: add PVH support to the inclusive options
2018-08-01 11:03 ` [PATCH v2 5/5] x86/iommu: add PVH support to the inclusive options Roger Pau Monne
@ 2018-08-01 11:27 ` Paul Durrant
2018-08-02 8:03 ` Tian, Kevin
1 sibling, 0 replies; 29+ messages in thread
From: Paul Durrant @ 2018-08-01 11:27 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, Ian Jackson,
Roger Pau Monne
> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Roger Pau Monne
> Sent: 01 August 2018 12:04
> To: xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> <wei.liu2@citrix.com>; George Dunlap <George.Dunlap@citrix.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>; Julien Grall
> <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>; Roger Pau
> Monne <roger.pau@citrix.com>
> Subject: [Xen-devel] [PATCH v2 5/5] x86/iommu: add PVH support to the
> inclusive options
>
> Several people have reported hardware issues (malfunctioning USB
> controllers) due to iommu page faults on Intel hardware. Those faults
> are caused by missing RMRR (VTd) entries in the ACPI tables. Those can
> be worked around on VTd hardware by manually adding RMRR entries on
> the command line, this is however limited to Intel hardware and quite
> cumbersome to do.
>
> In order to solve those issues add a new dom0-iommu=reserved option
> that identity maps all regions marked as reserved in the memory map.
> Note that regions used by devices emulated by Xen (LAPIC, IO-APIC or
> PCIe MCFG regions) are specifically avoided. Note that this option is
> available to a PVH Dom0 (as opposed to the inclusive option which only
> works for PV Dom0).
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Changes since v1:
> - Introduce a new reserved option instead of abusing the inclusive
> option.
> - Use the same helper function for PV and PVH in order to decide if a
> page should be added to the domain page tables.
> - Use the data inside of the domain struct to detect overlaps with
> emulated MMIO regions.
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> docs/misc/xen-command-line.markdown | 9 +++
> xen/arch/x86/hvm/io.c | 5 ++
> xen/drivers/passthrough/iommu.c | 3 +
> xen/drivers/passthrough/x86/iommu.c | 88 +++++++++++++++++++++------
> --
> xen/include/asm-x86/hvm/io.h | 3 +
> xen/include/xen/iommu.h | 2 +-
> 6 files changed, 86 insertions(+), 24 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> command-line.markdown
> index 30d970bc2e..526a96ffc5 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1241,6 +1241,15 @@ detection of systems known to misbehave upon
> accesses to that port.
> >> applicable to a PV dom0. Also note that if `strict` mode is enabled
> >> then conventional RAM pages not assigned to dom0 will not be mapped.
>
> +> `reserved`
> +
> +> Default: `true` on Intel hardware, `false` otherwise
> +
> +>> Use this to work around firmware issues providing incorrect RMRR or
> IVMD
> +>> entries. Rather than only mapping RAM pages for IOMMU accesses for
> Dom0,
> +>> all memory regions marked as reserved in the memory map that don't
> overlap
> +>> with any MMIO region from emulated devices will be identity mapped.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= <integer>`
>
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index bf4d8748d3..5e01c33890 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -404,6 +404,11 @@ static const struct hvm_mmcfg
> *vpci_mmcfg_find(const struct domain *d,
> return NULL;
> }
>
> +bool vpci_mmcfg_address(const struct domain *d, paddr_t addr)
> +{
> + return vpci_mmcfg_find(d, addr);
> +}
> +
> static unsigned int vpci_mmcfg_decode_addr(const struct hvm_mmcfg
> *mmcfg,
> paddr_t addr, pci_sbdf_t *sbdf)
> {
> diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> index 6611e13cc2..a3eb7c5b7f 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -75,6 +75,7 @@ custom_param("dom0-iommu",
> parse_dom0_iommu_param);
> bool __hwdom_initdata iommu_dom0_strict;
> bool __read_mostly iommu_dom0_passthrough;
> int8_t __hwdom_initdata iommu_dom0_inclusive = -1;
> +int8_t __hwdom_initdata iommu_dom0_reserved = -1;
>
> DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
>
> @@ -161,6 +162,8 @@ static int __init parse_dom0_iommu_param(const
> char *s)
> iommu_dom0_strict = !val;
> else if ( !strncmp(s, "inclusive", ss - s) )
> iommu_dom0_inclusive = val;
> + else if ( !strncmp(s, "reserved", ss - s) )
> + iommu_dom0_reserved = val;
> else
> rc = -EINVAL;
>
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index bf6edf4c04..66c5cc28ed 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -20,6 +20,7 @@
> #include <xen/softirq.h>
> #include <xsm/xsm.h>
>
> +#include <asm/hvm/io.h>
> #include <asm/setup.h>
>
> void iommu_update_ire_from_apic(
> @@ -134,15 +135,75 @@ void arch_iommu_domain_destroy(struct domain
> *d)
> {
> }
>
> +static bool __hwdom_init iommu_map(const struct domain *d, unsigned
> long pfn,
> + unsigned long max_pfn)
> +{
> + unsigned int i;
> +
> + /*
> + * Ignore any address below 1MB, that's already identity mapped by the
> + * domain builder for HVM.
> + */
> + if ( is_hvm_domain(d) && pfn < PFN_DOWN(MB(1)) )
> + return false;
> +
> + /*
> + * If dom0-strict mode is enabled then exclude conventional RAM and let
> the
> + * common code map dom0's pages.
Probably ought to amend the above comment now that the test below is not simply for iommu_dom0_strict, but also for is_hvm_domain() now.
Paul
> + */
> + if ( page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) &&
> + (iommu_dom0_strict || is_hvm_domain(d)) )
> + return false;
> + if ( page_is_ram_type(pfn, RAM_TYPE_RESERVED) &&
> + (!iommu_dom0_reserved || !iommu_dom0_inclusive) )
> + return false;
> + if ( !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE) &&
> + !page_is_ram_type(pfn, RAM_TYPE_RESERVED) &&
> + !page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) &&
> + (!iommu_dom0_inclusive || pfn > max_pfn) )
> + return false;
> +
> + /* Check that it doesn't overlap with the LAPIC */
> + if ( has_vlapic(d) )
> + {
> + const struct vcpu *v;
> +
> + for_each_vcpu(d, v)
> + if ( pfn == PFN_DOWN(vlapic_base_address(vcpu_vlapic(v))) )
> + return false;
> + }
> + /* ... or the IO-APIC */
> + for ( i = 0; has_vioapic(d) && i < d->arch.hvm_domain.nr_vioapics; i++ )
> + if ( pfn == PFN_DOWN(domain_vioapic(d, i)->base_address) )
> + return false;
> + /*
> + * ... or the PCIe MCFG regions.
> + * TODO: runtime added MMCFG regions are not checked to make sure
> they
> + * don't overlap with already mapped regions, thus preventing trapping.
> + */
> + if ( has_vpci(d) && vpci_mmcfg_address(d, pfn << PAGE_SHIFT) )
> + return false;
> +
> + return true;
> +}
> +
> void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> {
> unsigned long i, top, max_pfn;
>
> + if ( iommu_dom0_passthrough )
> + return;
> +
> BUG_ON(!is_hardware_domain(d));
>
> - /* Set the default value of inclusive depending on the hardware. */
> + /*
> + * Set the default value of inclusive and reserved depending on the
> + * hardware.
> + */
> if ( iommu_dom0_inclusive == -1 )
> iommu_dom0_inclusive = boot_cpu_data.x86_vendor ==
> X86_VENDOR_INTEL;
> + if ( iommu_dom0_reserved == -1 )
> + iommu_dom0_reserved = boot_cpu_data.x86_vendor ==
> X86_VENDOR_INTEL;
>
> max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> @@ -150,7 +211,6 @@ void __hwdom_init arch_iommu_hwdom_init(struct
> domain *d)
> for ( i = 0; i < top; i++ )
> {
> unsigned long pfn = pdx_to_pfn(i);
> - bool map;
> int rc;
>
> /*
> @@ -159,27 +219,9 @@ void __hwdom_init arch_iommu_hwdom_init(struct
> domain *d)
> * regions. When set, the inclusive mapping additionally maps in
> * every pfn up to 4GB except those that fall in unusable ranges.
> */
> - if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
> - continue;
> -
> - if ( iommu_dom0_inclusive && pfn <= max_pfn )
> - map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> - else
> - map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> -
> - if ( !map )
> - continue;
> -
> - /* Exclude Xen bits */
> - if ( xen_in_range(pfn) )
> - continue;
> -
> - /*
> - * If dom0-strict mode is enabled then exclude conventional RAM
> - * and let the common code map dom0's pages.
> - */
> - if ( iommu_dom0_strict &&
> - page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> + if ( (pfn > max_pfn && !mfn_valid(_mfn(pfn))) ||
> + /* Exclude Xen bits */
> + xen_in_range(pfn) || !iommu_map(d, pfn, max_pfn) )
> continue;
>
> rc = iommu_map_page(d, pfn, pfn,
> IOMMUF_readable|IOMMUF_writable);
> diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
> index e6b6ed0b92..8cca456b55 100644
> --- a/xen/include/asm-x86/hvm/io.h
> +++ b/xen/include/asm-x86/hvm/io.h
> @@ -180,6 +180,9 @@ int register_vpci_mmcfg_handler(struct domain *d,
> paddr_t addr,
> /* Destroy tracked MMCFG areas. */
> void destroy_vpci_mmcfg(struct domain *d);
>
> +/* Check if an address is between a MMCFG region for a domain. */
> +bool vpci_mmcfg_address(const struct domain *d, paddr_t addr);
> +
> #endif /* __ASM_X86_HVM_IO_H__ */
>
>
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 99e5b89c0f..fed1b1ea7a 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -37,7 +37,7 @@ extern bool_t iommu_debug;
> extern bool_t amd_iommu_perdev_intremap;
>
> extern bool iommu_dom0_strict, iommu_dom0_passthrough;
> -extern int8_t iommu_dom0_inclusive;
> +extern int8_t iommu_dom0_inclusive, iommu_dom0_reserved;
>
> extern unsigned int iommu_dev_iotlb_timeout;
>
> --
> 2.18.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal
2018-08-01 11:03 ` [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal Roger Pau Monne
2018-08-01 11:16 ` Paul Durrant
@ 2018-08-02 7:36 ` Tian, Kevin
1 sibling, 0 replies; 29+ messages in thread
From: Tian, Kevin @ 2018-08-02 7:36 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel
> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> Sent: Wednesday, August 1, 2018 7:04 PM
>
> Remove the handling for different page sizes now that ia64 is gone.
>
> No functional change.
>
> Reported by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-01 11:03 ` [PATCH v2 2/5] iommu: introduce dom0-iommu option Roger Pau Monne
2018-08-01 11:15 ` Paul Durrant
2018-08-01 11:16 ` Andrew Cooper
@ 2018-08-02 7:46 ` Tian, Kevin
2018-08-02 8:23 ` Jan Beulich
2 siblings, 1 reply; 29+ messages in thread
From: Tian, Kevin @ 2018-08-02 7:46 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel
Cc: Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, Jan Beulich, Brian Woods
> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> Sent: Wednesday, August 1, 2018 7:04 PM
>
> To select the iommu configuration used by Dom0. This option supersedes
> iommu=dom0-strict|dom0-passthrough.
>
> No functional change.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Changes since v1:
> - New in this version.
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Brian Woods <brian.woods@amd.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> ---
> docs/misc/xen-command-line.markdown | 32 ++++++++++++++
> xen/arch/x86/x86_64/mm.c | 3 +-
> xen/drivers/passthrough/amd/iommu_init.c | 2 +-
> xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 +-
> xen/drivers/passthrough/iommu.c | 46 +++++++++++++++++----
> xen/drivers/passthrough/vtd/iommu.c | 16 +++----
> xen/include/xen/iommu.h | 6 ++-
> 7 files changed, 88 insertions(+), 21 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> command-line.markdown
> index 65b4754418..a2a07cc6c8 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1150,12 +1150,18 @@ detection of systems known to misbehave
> upon accesses to that port.
>
> > `dom0-passthrough`
>
> +> **WARNING: This command line option is deprecated, and superseded
> by
> +> _dom0-iommu=none_ - using both options in combination is
> undefined.**
> +
in patch description you said 'supersede'... is it better to say that
new dom0_iommu is favored if both options are specified than
saying 'undefined'?
> > Default: `false`
>
> >> Control whether to disable DMA remapping for Dom0.
>
> > `dom0-strict`
>
> +> **WARNING: This command line option is deprecated, and superseded
> by
> +> _dom0-iommu=strict_ - using both options in combination is
> undefined.**
> +
> > Default: `false`
>
> >> Control whether to set up DMA remapping only for the memory Dom0
> actually
> @@ -1198,6 +1204,32 @@ detection of systems known to misbehave upon
> accesses to that port.
>
> >> Enable IOMMU debugging code (implies `verbose`).
>
> +### dom0-iommu
> +> `= List of [ none | strict | relaxed ]`
> +
> +> Sub-options are of boolean kind and can be prefixed with `no-` to effect
> the
> +> inverse meaning.
not important comment, but doesn't "no-none" sound weird? :-)
> +
> +> `none`
> +
> +> Default: `false`
> +
> +>> Control whether to disable DMA remapping for Dom0.
> +
> +> `strict`
> +
> +> Default: `false`
> +
> +>> Control whether to set up DMA remapping only for the memory Dom0
> actually
> +>> got assigned.
> +
> +> `relaxed`
> +
> +> Default: `true`
> +
> +>> Controls whether to setup DMA remappings for all the host RAM
> except regions
> +>> in use by Xen.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= <integer>`
>
> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> index cca4ae926e..84226b3326 100644
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1426,7 +1426,8 @@ int memory_add(unsigned long spfn, unsigned
> long epfn, unsigned int pxm)
> if ( ret )
> goto destroy_m2p;
>
> - if ( iommu_enabled && !iommu_passthrough
> && !need_iommu(hardware_domain) )
> + if ( iommu_enabled && !iommu_dom0_passthrough &&
> + !need_iommu(hardware_domain) )
> {
> for ( i = spfn; i < epfn; i++ )
> if ( iommu_map_page(hardware_domain, i, i,
> IOMMUF_readable|IOMMUF_writable) )
> diff --git a/xen/drivers/passthrough/amd/iommu_init.c
> b/xen/drivers/passthrough/amd/iommu_init.c
> index 474992a75a..ad8c48be1c 100644
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -1062,7 +1062,7 @@ static void __init amd_iommu_init_cleanup(void)
> radix_tree_destroy(&ivrs_maps, xfree);
>
> iommu_enabled = 0;
> - iommu_passthrough = 0;
> + iommu_dom0_passthrough = false;
ah, I see why "undefined" was used earlier. Both options control same
variable, thus the behavior is undefined when both are specified. If that
is the case, possibly clearer to say " This option *deprecates* iommu=
dom0-strict|dom0-passthrough" in patch description?
Thanks
Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
2018-08-01 11:03 ` [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu Roger Pau Monne
2018-08-01 11:21 ` Paul Durrant
@ 2018-08-02 7:58 ` Tian, Kevin
2018-08-03 10:39 ` Roger Pau Monné
2018-08-03 9:09 ` Julien Grall
2 siblings, 1 reply; 29+ messages in thread
From: Tian, Kevin @ 2018-08-02 7:58 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich
> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> Sent: Wednesday, August 1, 2018 7:04 PM
>
> Introduce a new dom0-iommu=inclusive generic option that supersedes
> iommu_inclusive_mapping. The prevcious behaviour is preserved and the
> option should only be enabled by default on Intel hardware.
>
> No functional change intended.
there is functional change. Original condition is:
- if ( !iommu_dom0_passthrough && is_pv_domain(d) )
- {
- /* Set up 1:1 page table for hardware domain. */
- vtd_set_hwdom_mapping(d);
- }
Now it is always called.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Changes since v1:
> - Use dom0-iommu instead of the iommu option.
> - Only enable by default on Intel hardware.
[...]
>
> +void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> +{
> + unsigned long i, top, max_pfn;
> +
> + BUG_ON(!is_hardware_domain(d));
> +
> + /* Set the default value of inclusive depending on the hardware. */
> + if ( iommu_dom0_inclusive == -1 )
> + iommu_dom0_inclusive = boot_cpu_data.x86_vendor ==
> X86_VENDOR_INTEL;
> +
I don't like above style.
btw It's better to set it in intel_iommu_hwdom_init.
Thanks
Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 5/5] x86/iommu: add PVH support to the inclusive options
2018-08-01 11:03 ` [PATCH v2 5/5] x86/iommu: add PVH support to the inclusive options Roger Pau Monne
2018-08-01 11:27 ` Paul Durrant
@ 2018-08-02 8:03 ` Tian, Kevin
1 sibling, 0 replies; 29+ messages in thread
From: Tian, Kevin @ 2018-08-02 8:03 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich
> From: Roger Pau Monne
> Sent: Wednesday, August 1, 2018 7:04 PM
>
> Several people have reported hardware issues (malfunctioning USB
> controllers) due to iommu page faults on Intel hardware. Those faults
> are caused by missing RMRR (VTd) entries in the ACPI tables. Those can
> be worked around on VTd hardware by manually adding RMRR entries on
> the command line, this is however limited to Intel hardware and quite
> cumbersome to do.
>
> In order to solve those issues add a new dom0-iommu=reserved option
> that identity maps all regions marked as reserved in the memory map.
> Note that regions used by devices emulated by Xen (LAPIC, IO-APIC or
> PCIe MCFG regions) are specifically avoided. Note that this option is
> available to a PVH Dom0 (as opposed to the inclusive option which only
> works for PV Dom0).
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Changes since v1:
> - Introduce a new reserved option instead of abusing the inclusive
> option.
> - Use the same helper function for PV and PVH in order to decide if a
> page should be added to the domain page tables.
> - Use the data inside of the domain struct to detect overlaps with
> emulated MMIO regions.
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> docs/misc/xen-command-line.markdown | 9 +++
> xen/arch/x86/hvm/io.c | 5 ++
> xen/drivers/passthrough/iommu.c | 3 +
> xen/drivers/passthrough/x86/iommu.c | 88 +++++++++++++++++++++-------
> -
> xen/include/asm-x86/hvm/io.h | 3 +
> xen/include/xen/iommu.h | 2 +-
> 6 files changed, 86 insertions(+), 24 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> command-line.markdown
> index 30d970bc2e..526a96ffc5 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1241,6 +1241,15 @@ detection of systems known to misbehave upon
> accesses to that port.
> >> applicable to a PV dom0. Also note that if `strict` mode is enabled
> >> then conventional RAM pages not assigned to dom0 will not be mapped.
>
> +> `reserved`
> +
> +> Default: `true` on Intel hardware, `false` otherwise
> +
> +>> Use this to work around firmware issues providing incorrect RMRR or
> IVMD
> +>> entries. Rather than only mapping RAM pages for IOMMU accesses for
> Dom0,
> +>> all memory regions marked as reserved in the memory map that don't
> overlap
> +>> with any MMIO region from emulated devices will be identity mapped.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= <integer>`
>
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index bf4d8748d3..5e01c33890 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -404,6 +404,11 @@ static const struct hvm_mmcfg
> *vpci_mmcfg_find(const struct domain *d,
> return NULL;
> }
>
> +bool vpci_mmcfg_address(const struct domain *d, paddr_t addr)
> +{
> + return vpci_mmcfg_find(d, addr);
> +}
> +
> static unsigned int vpci_mmcfg_decode_addr(const struct hvm_mmcfg
> *mmcfg,
> paddr_t addr, pci_sbdf_t *sbdf)
> {
> diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> index 6611e13cc2..a3eb7c5b7f 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -75,6 +75,7 @@ custom_param("dom0-iommu",
> parse_dom0_iommu_param);
> bool __hwdom_initdata iommu_dom0_strict;
> bool __read_mostly iommu_dom0_passthrough;
> int8_t __hwdom_initdata iommu_dom0_inclusive = -1;
> +int8_t __hwdom_initdata iommu_dom0_reserved = -1;
>
> DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
>
> @@ -161,6 +162,8 @@ static int __init parse_dom0_iommu_param(const
> char *s)
> iommu_dom0_strict = !val;
> else if ( !strncmp(s, "inclusive", ss - s) )
> iommu_dom0_inclusive = val;
> + else if ( !strncmp(s, "reserved", ss - s) )
> + iommu_dom0_reserved = val;
> else
> rc = -EINVAL;
>
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index bf6edf4c04..66c5cc28ed 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -20,6 +20,7 @@
> #include <xen/softirq.h>
> #include <xsm/xsm.h>
>
> +#include <asm/hvm/io.h>
> #include <asm/setup.h>
>
> void iommu_update_ire_from_apic(
> @@ -134,15 +135,75 @@ void arch_iommu_domain_destroy(struct
> domain *d)
> {
> }
>
> +static bool __hwdom_init iommu_map(const struct domain *d, unsigned
> long pfn,
> + unsigned long max_pfn)
since the logic is limited to dom0, call "dom0_iommu_map" is clearer.
> +{
> + unsigned int i;
> +
> + /*
> + * Ignore any address below 1MB, that's already identity mapped by the
> + * domain builder for HVM.
> + */
> + if ( is_hvm_domain(d) && pfn < PFN_DOWN(MB(1)) )
> + return false;
> +
> + /*
> + * If dom0-strict mode is enabled then exclude conventional RAM and
> let the
> + * common code map dom0's pages.
> + */
> + if ( page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) &&
> + (iommu_dom0_strict || is_hvm_domain(d)) )
> + return false;
> + if ( page_is_ram_type(pfn, RAM_TYPE_RESERVED) &&
> + (!iommu_dom0_reserved || !iommu_dom0_inclusive) )
> + return false;
> + if ( !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE) &&
> + !page_is_ram_type(pfn, RAM_TYPE_RESERVED) &&
> + !page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) &&
> + (!iommu_dom0_inclusive || pfn > max_pfn) )
> + return false;
> +
> + /* Check that it doesn't overlap with the LAPIC */
> + if ( has_vlapic(d) )
> + {
> + const struct vcpu *v;
> +
> + for_each_vcpu(d, v)
> + if ( pfn == PFN_DOWN(vlapic_base_address(vcpu_vlapic(v))) )
> + return false;
> + }
> + /* ... or the IO-APIC */
> + for ( i = 0; has_vioapic(d) && i < d->arch.hvm_domain.nr_vioapics; i++ )
> + if ( pfn == PFN_DOWN(domain_vioapic(d, i)->base_address) )
> + return false;
> + /*
> + * ... or the PCIe MCFG regions.
> + * TODO: runtime added MMCFG regions are not checked to make sure
> they
> + * don't overlap with already mapped regions, thus preventing trapping.
> + */
> + if ( has_vpci(d) && vpci_mmcfg_address(d, pfn << PAGE_SHIFT) )
> + return false;
> +
> + return true;
> +}
> +
> void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> {
> unsigned long i, top, max_pfn;
>
> + if ( iommu_dom0_passthrough )
> + return;
> +
> BUG_ON(!is_hardware_domain(d));
>
> - /* Set the default value of inclusive depending on the hardware. */
> + /*
> + * Set the default value of inclusive and reserved depending on the
> + * hardware.
> + */
> if ( iommu_dom0_inclusive == -1 )
> iommu_dom0_inclusive = boot_cpu_data.x86_vendor ==
> X86_VENDOR_INTEL;
> + if ( iommu_dom0_reserved == -1 )
> + iommu_dom0_reserved = boot_cpu_data.x86_vendor ==
> X86_VENDOR_INTEL;
same comment as for 4/5
>
> max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> @@ -150,7 +211,6 @@ void __hwdom_init
> arch_iommu_hwdom_init(struct domain *d)
> for ( i = 0; i < top; i++ )
> {
> unsigned long pfn = pdx_to_pfn(i);
> - bool map;
> int rc;
>
> /*
> @@ -159,27 +219,9 @@ void __hwdom_init
> arch_iommu_hwdom_init(struct domain *d)
> * regions. When set, the inclusive mapping additionally maps in
> * every pfn up to 4GB except those that fall in unusable ranges.
> */
> - if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
> - continue;
> -
> - if ( iommu_dom0_inclusive && pfn <= max_pfn )
> - map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> - else
> - map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> -
> - if ( !map )
> - continue;
> -
> - /* Exclude Xen bits */
> - if ( xen_in_range(pfn) )
> - continue;
> -
> - /*
> - * If dom0-strict mode is enabled then exclude conventional RAM
> - * and let the common code map dom0's pages.
> - */
> - if ( iommu_dom0_strict &&
> - page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> + if ( (pfn > max_pfn && !mfn_valid(_mfn(pfn))) ||
> + /* Exclude Xen bits */
> + xen_in_range(pfn) || !iommu_map(d, pfn, max_pfn) )
> continue;
>
> rc = iommu_map_page(d, pfn, pfn,
> IOMMUF_readable|IOMMUF_writable);
> diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-
> x86/hvm/io.h
> index e6b6ed0b92..8cca456b55 100644
> --- a/xen/include/asm-x86/hvm/io.h
> +++ b/xen/include/asm-x86/hvm/io.h
> @@ -180,6 +180,9 @@ int register_vpci_mmcfg_handler(struct domain *d,
> paddr_t addr,
> /* Destroy tracked MMCFG areas. */
> void destroy_vpci_mmcfg(struct domain *d);
>
> +/* Check if an address is between a MMCFG region for a domain. */
> +bool vpci_mmcfg_address(const struct domain *d, paddr_t addr);
> +
> #endif /* __ASM_X86_HVM_IO_H__ */
>
>
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 99e5b89c0f..fed1b1ea7a 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -37,7 +37,7 @@ extern bool_t iommu_debug;
> extern bool_t amd_iommu_perdev_intremap;
>
> extern bool iommu_dom0_strict, iommu_dom0_passthrough;
> -extern int8_t iommu_dom0_inclusive;
> +extern int8_t iommu_dom0_inclusive, iommu_dom0_reserved;
>
> extern unsigned int iommu_dev_iotlb_timeout;
>
> --
> 2.18.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-02 7:46 ` Tian, Kevin
@ 2018-08-02 8:23 ` Jan Beulich
2018-08-03 8:14 ` Roger Pau Monné
0 siblings, 1 reply; 29+ messages in thread
From: Jan Beulich @ 2018-08-02 8:23 UTC (permalink / raw)
To: Kevin Tian
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Suravee Suthikulpanit,
xen-devel, Brian Woods, Roger Pau Monne
>>> On 02.08.18 at 09:46, <kevin.tian@intel.com> wrote:
>> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
>> Sent: Wednesday, August 1, 2018 7:04 PM
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -1150,12 +1150,18 @@ detection of systems known to misbehave
>> upon accesses to that port.
>>
>> > `dom0-passthrough`
>>
>> +> **WARNING: This command line option is deprecated, and superseded
>> by
>> +> _dom0-iommu=none_ - using both options in combination is
>> undefined.**
>> +
>
> in patch description you said 'supersede'... is it better to say that
> new dom0_iommu is favored if both options are specified than
> saying 'undefined'?
That would complicate handling (perhaps just slightly, but anyway),
since we'd have to maintain a second boolean. Without that the
order on the command line determines behavior. (And I see that in
the end you've figured that out.)
>> @@ -1198,6 +1204,32 @@ detection of systems known to misbehave upon
>> accesses to that port.
>>
>> >> Enable IOMMU debugging code (implies `verbose`).
>>
>> +### dom0-iommu
>> +> `= List of [ none | strict | relaxed ]`
>> +
>> +> Sub-options are of boolean kind and can be prefixed with `no-` to effect
>> the
>> +> inverse meaning.
>
> not important comment, but doesn't "no-none" sound weird? :-)
Only positive (true) values should be permitted for I think all of
these. I didn't look at the patch yes, so perhaps that's already
the case.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-02 8:23 ` Jan Beulich
@ 2018-08-03 8:14 ` Roger Pau Monné
2018-08-03 8:35 ` Paul Durrant
0 siblings, 1 reply; 29+ messages in thread
From: Roger Pau Monné @ 2018-08-03 8:14 UTC (permalink / raw)
To: Jan Beulich
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
Suravee Suthikulpanit, xen-devel, Brian Woods
On Thu, Aug 02, 2018 at 02:23:23AM -0600, Jan Beulich wrote:
> >>> On 02.08.18 at 09:46, <kevin.tian@intel.com> wrote:
> >> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> >> Sent: Wednesday, August 1, 2018 7:04 PM
> >> --- a/docs/misc/xen-command-line.markdown
> >> +++ b/docs/misc/xen-command-line.markdown
> >> @@ -1150,12 +1150,18 @@ detection of systems known to misbehave
> >> upon accesses to that port.
> >>
> >> > `dom0-passthrough`
> >>
> >> +> **WARNING: This command line option is deprecated, and superseded
> >> by
> >> +> _dom0-iommu=none_ - using both options in combination is
> >> undefined.**
> >> +
> >
> > in patch description you said 'supersede'... is it better to say that
> > new dom0_iommu is favored if both options are specified than
> > saying 'undefined'?
>
> That would complicate handling (perhaps just slightly, but anyway),
> since we'd have to maintain a second boolean. Without that the
> order on the command line determines behavior. (And I see that in
> the end you've figured that out.)
>
> >> @@ -1198,6 +1204,32 @@ detection of systems known to misbehave upon
> >> accesses to that port.
> >>
> >> >> Enable IOMMU debugging code (implies `verbose`).
> >>
> >> +### dom0-iommu
> >> +> `= List of [ none | strict | relaxed ]`
> >> +
> >> +> Sub-options are of boolean kind and can be prefixed with `no-` to effect
> >> the
> >> +> inverse meaning.
> >
> > not important comment, but doesn't "no-none" sound weird? :-)
>
> Only positive (true) values should be permitted for I think all of
> these. I didn't look at the patch yes, so perhaps that's already
> the case.
For the current set of options introduced in this patch none, strict
or relaxed it doesn't make much sense to allow the no- prefix.
For options added in later patches (inclusive and reserved) it makes
sense to allow the no- prefix, so that a user can do
'dom0-iommu=no-inclusive' in order to change the default value.
I will make it clear which options allow the no- prefix, and add the
code to parse such prefix when it's needed.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-03 8:14 ` Roger Pau Monné
@ 2018-08-03 8:35 ` Paul Durrant
2018-08-03 8:52 ` Roger Pau Monné
0 siblings, 1 reply; 29+ messages in thread
From: Paul Durrant @ 2018-08-03 8:35 UTC (permalink / raw)
To: Roger Pau Monne, Jan Beulich
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Andrew Cooper,
Tim (Xen.org),
George Dunlap, Julien Grall, Suravee Suthikulpanit, xen-devel,
Ian Jackson, Brian Woods
> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Roger Pau Monné
> Sent: 03 August 2018 09:14
> To: Jan Beulich <JBeulich@suse.com>
> Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> option
>
> On Thu, Aug 02, 2018 at 02:23:23AM -0600, Jan Beulich wrote:
> > >>> On 02.08.18 at 09:46, <kevin.tian@intel.com> wrote:
> > >> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> > >> Sent: Wednesday, August 1, 2018 7:04 PM
> > >> --- a/docs/misc/xen-command-line.markdown
> > >> +++ b/docs/misc/xen-command-line.markdown
> > >> @@ -1150,12 +1150,18 @@ detection of systems known to misbehave
> > >> upon accesses to that port.
> > >>
> > >> > `dom0-passthrough`
> > >>
> > >> +> **WARNING: This command line option is deprecated, and
> superseded
> > >> by
> > >> +> _dom0-iommu=none_ - using both options in combination is
> > >> undefined.**
> > >> +
> > >
> > > in patch description you said 'supersede'... is it better to say that
> > > new dom0_iommu is favored if both options are specified than
> > > saying 'undefined'?
> >
> > That would complicate handling (perhaps just slightly, but anyway),
> > since we'd have to maintain a second boolean. Without that the
> > order on the command line determines behavior. (And I see that in
> > the end you've figured that out.)
> >
> > >> @@ -1198,6 +1204,32 @@ detection of systems known to misbehave
> upon
> > >> accesses to that port.
> > >>
> > >> >> Enable IOMMU debugging code (implies `verbose`).
> > >>
> > >> +### dom0-iommu
> > >> +> `= List of [ none | strict | relaxed ]`
> > >> +
> > >> +> Sub-options are of boolean kind and can be prefixed with `no-` to
> effect
> > >> the
> > >> +> inverse meaning.
> > >
> > > not important comment, but doesn't "no-none" sound weird? :-)
> >
> > Only positive (true) values should be permitted for I think all of
> > these. I didn't look at the patch yes, so perhaps that's already
> > the case.
>
> For the current set of options introduced in this patch none, strict
> or relaxed it doesn't make much sense to allow the no- prefix.
>
> For options added in later patches (inclusive and reserved) it makes
> sense to allow the no- prefix, so that a user can do
> 'dom0-iommu=no-inclusive' in order to change the default value.
>
But what does that mean? 'no-inclusive' clearly means you don't get the inclusive map, but what do you get instead? Strict? None?
Paul
> I will make it clear which options allow the no- prefix, and add the
> code to parse such prefix when it's needed.
>
> Roger.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-03 8:35 ` Paul Durrant
@ 2018-08-03 8:52 ` Roger Pau Monné
2018-08-03 9:05 ` Paul Durrant
0 siblings, 1 reply; 29+ messages in thread
From: Roger Pau Monné @ 2018-08-03 8:52 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
On Fri, Aug 03, 2018 at 09:35:58AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> > Of Roger Pau Monné
> > Sent: 03 August 2018 09:14
> > To: Jan Beulich <JBeulich@suse.com>
> > Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> > <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> > <George.Dunlap@citrix.com>; Andrew Cooper
> > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > option
> >
> > On Thu, Aug 02, 2018 at 02:23:23AM -0600, Jan Beulich wrote:
> > > >>> On 02.08.18 at 09:46, <kevin.tian@intel.com> wrote:
> > > >> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> > > >> Sent: Wednesday, August 1, 2018 7:04 PM
> > > >> --- a/docs/misc/xen-command-line.markdown
> > > >> +++ b/docs/misc/xen-command-line.markdown
> > > >> @@ -1150,12 +1150,18 @@ detection of systems known to misbehave
> > > >> upon accesses to that port.
> > > >>
> > > >> > `dom0-passthrough`
> > > >>
> > > >> +> **WARNING: This command line option is deprecated, and
> > superseded
> > > >> by
> > > >> +> _dom0-iommu=none_ - using both options in combination is
> > > >> undefined.**
> > > >> +
> > > >
> > > > in patch description you said 'supersede'... is it better to say that
> > > > new dom0_iommu is favored if both options are specified than
> > > > saying 'undefined'?
> > >
> > > That would complicate handling (perhaps just slightly, but anyway),
> > > since we'd have to maintain a second boolean. Without that the
> > > order on the command line determines behavior. (And I see that in
> > > the end you've figured that out.)
> > >
> > > >> @@ -1198,6 +1204,32 @@ detection of systems known to misbehave
> > upon
> > > >> accesses to that port.
> > > >>
> > > >> >> Enable IOMMU debugging code (implies `verbose`).
> > > >>
> > > >> +### dom0-iommu
> > > >> +> `= List of [ none | strict | relaxed ]`
> > > >> +
> > > >> +> Sub-options are of boolean kind and can be prefixed with `no-` to
> > effect
> > > >> the
> > > >> +> inverse meaning.
> > > >
> > > > not important comment, but doesn't "no-none" sound weird? :-)
> > >
> > > Only positive (true) values should be permitted for I think all of
> > > these. I didn't look at the patch yes, so perhaps that's already
> > > the case.
> >
> > For the current set of options introduced in this patch none, strict
> > or relaxed it doesn't make much sense to allow the no- prefix.
> >
> > For options added in later patches (inclusive and reserved) it makes
> > sense to allow the no- prefix, so that a user can do
> > 'dom0-iommu=no-inclusive' in order to change the default value.
> >
>
> But what does that mean? 'no-inclusive' clearly means you don't get the inclusive map, but what do you get instead? Strict? None?
IMO you always either get no iommu mappings at all (none), only Dom0
assigned RAM regions (strict) or all RAM except the regions used by
Xen (relaxed). Those options control what RAM gets mapped into the
iommu page tables.
Then you can use inclusive to even get more mappings of non-RAM
regions, but that can be used in conjunction with either strict or
relaxed and should allow the usage of the no- prefix.
So if you use dom0-iommu=no-inclusive you get the default relaxed RAM
mappings and that's all.
I hope this makes sense, I will try to clarify the documentation.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-03 8:52 ` Roger Pau Monné
@ 2018-08-03 9:05 ` Paul Durrant
2018-08-03 9:08 ` Roger Pau Monné
0 siblings, 1 reply; 29+ messages in thread
From: Paul Durrant @ 2018-08-03 9:05 UTC (permalink / raw)
To: Roger Pau Monne
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
> -----Original Message-----
> From: Roger Pau Monne
> Sent: 03 August 2018 09:52
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian <kevin.tian@intel.com>;
> Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>;
> George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> option
>
> On Fri, Aug 03, 2018 at 09:35:58AM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On
> Behalf
> > > Of Roger Pau Monné
> > > Sent: 03 August 2018 09:14
> > > To: Jan Beulich <JBeulich@suse.com>
> > > Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> > > <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> > > <George.Dunlap@citrix.com>; Andrew Cooper
> > > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Tim
> > > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > > option
> > >
> > > On Thu, Aug 02, 2018 at 02:23:23AM -0600, Jan Beulich wrote:
> > > > >>> On 02.08.18 at 09:46, <kevin.tian@intel.com> wrote:
> > > > >> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> > > > >> Sent: Wednesday, August 1, 2018 7:04 PM
> > > > >> --- a/docs/misc/xen-command-line.markdown
> > > > >> +++ b/docs/misc/xen-command-line.markdown
> > > > >> @@ -1150,12 +1150,18 @@ detection of systems known to
> misbehave
> > > > >> upon accesses to that port.
> > > > >>
> > > > >> > `dom0-passthrough`
> > > > >>
> > > > >> +> **WARNING: This command line option is deprecated, and
> > > superseded
> > > > >> by
> > > > >> +> _dom0-iommu=none_ - using both options in combination is
> > > > >> undefined.**
> > > > >> +
> > > > >
> > > > > in patch description you said 'supersede'... is it better to say that
> > > > > new dom0_iommu is favored if both options are specified than
> > > > > saying 'undefined'?
> > > >
> > > > That would complicate handling (perhaps just slightly, but anyway),
> > > > since we'd have to maintain a second boolean. Without that the
> > > > order on the command line determines behavior. (And I see that in
> > > > the end you've figured that out.)
> > > >
> > > > >> @@ -1198,6 +1204,32 @@ detection of systems known to
> misbehave
> > > upon
> > > > >> accesses to that port.
> > > > >>
> > > > >> >> Enable IOMMU debugging code (implies `verbose`).
> > > > >>
> > > > >> +### dom0-iommu
> > > > >> +> `= List of [ none | strict | relaxed ]`
> > > > >> +
> > > > >> +> Sub-options are of boolean kind and can be prefixed with `no-` to
> > > effect
> > > > >> the
> > > > >> +> inverse meaning.
> > > > >
> > > > > not important comment, but doesn't "no-none" sound weird? :-)
> > > >
> > > > Only positive (true) values should be permitted for I think all of
> > > > these. I didn't look at the patch yes, so perhaps that's already
> > > > the case.
> > >
> > > For the current set of options introduced in this patch none, strict
> > > or relaxed it doesn't make much sense to allow the no- prefix.
> > >
> > > For options added in later patches (inclusive and reserved) it makes
> > > sense to allow the no- prefix, so that a user can do
> > > 'dom0-iommu=no-inclusive' in order to change the default value.
> > >
> >
> > But what does that mean? 'no-inclusive' clearly means you don't get the
> inclusive map, but what do you get instead? Strict? None?
>
> IMO you always either get no iommu mappings at all (none), only Dom0
> assigned RAM regions (strict) or all RAM except the regions used by
> Xen (relaxed). Those options control what RAM gets mapped into the
> iommu page tables.
>
> Then you can use inclusive to even get more mappings of non-RAM
> regions, but that can be used in conjunction with either strict or
> relaxed and should allow the usage of the no- prefix.
>
> So if you use dom0-iommu=no-inclusive you get the default relaxed RAM
> mappings and that's all.
>
> I hope this makes sense, I will try to clarify the documentation.
>
Actually I wonder whether we should rename 'inclusive' to 'reserved'. Essentially 'none', 'strict' or 'relaxed' are all about mappings of RAM, and then we need to decide whether to map the E820 reserved regions. So I think the inclusive map as it stands today is equivalent to 'relaxed' + 'reserved'.
Paul
> Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-03 9:05 ` Paul Durrant
@ 2018-08-03 9:08 ` Roger Pau Monné
2018-08-03 9:14 ` Paul Durrant
0 siblings, 1 reply; 29+ messages in thread
From: Roger Pau Monné @ 2018-08-03 9:08 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
On Fri, Aug 03, 2018 at 10:05:19AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne
> > Sent: 03 August 2018 09:52
> > To: Paul Durrant <Paul.Durrant@citrix.com>
> > Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian <kevin.tian@intel.com>;
> > Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>;
> > George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > option
> >
> > On Fri, Aug 03, 2018 at 09:35:58AM +0100, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On
> > Behalf
> > > > Of Roger Pau Monné
> > > > Sent: 03 August 2018 09:14
> > > > To: Jan Beulich <JBeulich@suse.com>
> > > > Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> > > > <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> > > > <George.Dunlap@citrix.com>; Andrew Cooper
> > > > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> > Tim
> > > > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > > > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > > > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > > > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > > > option
> > > >
> > > > On Thu, Aug 02, 2018 at 02:23:23AM -0600, Jan Beulich wrote:
> > > > > >>> On 02.08.18 at 09:46, <kevin.tian@intel.com> wrote:
> > > > > >> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> > > > > >> Sent: Wednesday, August 1, 2018 7:04 PM
> > > > > >> --- a/docs/misc/xen-command-line.markdown
> > > > > >> +++ b/docs/misc/xen-command-line.markdown
> > > > > >> @@ -1150,12 +1150,18 @@ detection of systems known to
> > misbehave
> > > > > >> upon accesses to that port.
> > > > > >>
> > > > > >> > `dom0-passthrough`
> > > > > >>
> > > > > >> +> **WARNING: This command line option is deprecated, and
> > > > superseded
> > > > > >> by
> > > > > >> +> _dom0-iommu=none_ - using both options in combination is
> > > > > >> undefined.**
> > > > > >> +
> > > > > >
> > > > > > in patch description you said 'supersede'... is it better to say that
> > > > > > new dom0_iommu is favored if both options are specified than
> > > > > > saying 'undefined'?
> > > > >
> > > > > That would complicate handling (perhaps just slightly, but anyway),
> > > > > since we'd have to maintain a second boolean. Without that the
> > > > > order on the command line determines behavior. (And I see that in
> > > > > the end you've figured that out.)
> > > > >
> > > > > >> @@ -1198,6 +1204,32 @@ detection of systems known to
> > misbehave
> > > > upon
> > > > > >> accesses to that port.
> > > > > >>
> > > > > >> >> Enable IOMMU debugging code (implies `verbose`).
> > > > > >>
> > > > > >> +### dom0-iommu
> > > > > >> +> `= List of [ none | strict | relaxed ]`
> > > > > >> +
> > > > > >> +> Sub-options are of boolean kind and can be prefixed with `no-` to
> > > > effect
> > > > > >> the
> > > > > >> +> inverse meaning.
> > > > > >
> > > > > > not important comment, but doesn't "no-none" sound weird? :-)
> > > > >
> > > > > Only positive (true) values should be permitted for I think all of
> > > > > these. I didn't look at the patch yes, so perhaps that's already
> > > > > the case.
> > > >
> > > > For the current set of options introduced in this patch none, strict
> > > > or relaxed it doesn't make much sense to allow the no- prefix.
> > > >
> > > > For options added in later patches (inclusive and reserved) it makes
> > > > sense to allow the no- prefix, so that a user can do
> > > > 'dom0-iommu=no-inclusive' in order to change the default value.
> > > >
> > >
> > > But what does that mean? 'no-inclusive' clearly means you don't get the
> > inclusive map, but what do you get instead? Strict? None?
> >
> > IMO you always either get no iommu mappings at all (none), only Dom0
> > assigned RAM regions (strict) or all RAM except the regions used by
> > Xen (relaxed). Those options control what RAM gets mapped into the
> > iommu page tables.
> >
> > Then you can use inclusive to even get more mappings of non-RAM
> > regions, but that can be used in conjunction with either strict or
> > relaxed and should allow the usage of the no- prefix.
> >
> > So if you use dom0-iommu=no-inclusive you get the default relaxed RAM
> > mappings and that's all.
> >
> > I hope this makes sense, I will try to clarify the documentation.
> >
>
> Actually I wonder whether we should rename 'inclusive' to 'reserved'. Essentially 'none', 'strict' or 'relaxed' are all about mappings of RAM, and then we need to decide whether to map the E820 reserved regions. So I think the inclusive map as it stands today is equivalent to 'relaxed' + 'reserved'.
Hm, not exactly. inclusive (iommu_inclusive_mapping) right now maps
everything except unusable regions. That's way more than just mapping
reserved regions. If we want to keep this behaviour while introducing
an option to map only reserved regions we need both an inclusive and a
reserved option.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
2018-08-01 11:03 ` [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu Roger Pau Monne
2018-08-01 11:21 ` Paul Durrant
2018-08-02 7:58 ` Tian, Kevin
@ 2018-08-03 9:09 ` Julien Grall
2 siblings, 0 replies; 29+ messages in thread
From: Julien Grall @ 2018-08-03 9:09 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson, Tim Deegan, Jan Beulich
Hi Roger,
On 08/01/2018 12:03 PM, Roger Pau Monne wrote:
> Introduce a new dom0-iommu=inclusive generic option that supersedes
> iommu_inclusive_mapping. The prevcious behaviour is preserved and the
> option should only be enabled by default on Intel hardware.
>
> No functional change intended.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
For the Arm bits:
Acked-by: Julien Grall <julien.grall@arm.com>
Cheers,
> ---
> Changes since v1:
> - Use dom0-iommu instead of the iommu option.
> - Only enable by default on Intel hardware.
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> ---
> docs/misc/xen-command-line.markdown | 14 +++++++
> xen/drivers/passthrough/arm/iommu.c | 4 ++
> xen/drivers/passthrough/iommu.c | 5 +++
> xen/drivers/passthrough/vtd/extern.h | 2 -
> xen/drivers/passthrough/vtd/iommu.c | 6 ---
> xen/drivers/passthrough/vtd/x86/vtd.c | 58 +-------------------------
> xen/drivers/passthrough/x86/iommu.c | 60 +++++++++++++++++++++++++++
> xen/include/xen/iommu.h | 2 +
> 8 files changed, 86 insertions(+), 65 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index a2a07cc6c8..30d970bc2e 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1230,6 +1230,17 @@ detection of systems known to misbehave upon accesses to that port.
> >> Controls whether to setup DMA remappings for all the host RAM except regions
> >> in use by Xen.
>
> +> `inclusive`
> +
> +> Default: `true` on Intel hardware, `false` otherwise
> +
> +>> Use this to work around firmware issues providing incorrect RMRR or 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. Also note that if `strict` mode is enabled
> +>> then conventional RAM pages not assigned to dom0 will not be mapped.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= <integer>`
>
> @@ -1242,6 +1253,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=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/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/iommu.c b/xen/drivers/passthrough/iommu.c
> index 88e23bbd04..6611e13cc2 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -74,6 +74,7 @@ bool_t __read_mostly amd_iommu_perdev_intremap = 1;
> custom_param("dom0-iommu", parse_dom0_iommu_param);
> bool __hwdom_initdata iommu_dom0_strict;
> bool __read_mostly iommu_dom0_passthrough;
> +int8_t __hwdom_initdata iommu_dom0_inclusive = -1;
>
> DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
>
> @@ -158,6 +159,8 @@ static int __init parse_dom0_iommu_param(const char *s)
> iommu_dom0_strict = val;
> else if ( !strncmp(s, "relaxed", ss - s) )
> iommu_dom0_strict = !val;
> + else if ( !strncmp(s, "inclusive", ss - s) )
> + iommu_dom0_inclusive = val;
> else
> rc = -EINVAL;
>
> @@ -240,6 +243,8 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
> }
>
> hd->platform_ops->hwdom_init(d);
> +
> + arch_iommu_hwdom_init(d);
> }
>
> void iommu_teardown(struct domain *d)
> diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
> index fb7edfaef9..91cadc602e 100644
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -99,6 +99,4 @@ void pci_vtd_quirk(const struct pci_dev *);
> bool_t platform_supports_intremap(void);
> bool_t platform_supports_x2apic(void);
>
> -void vtd_set_hwdom_mapping(struct domain *d);
> -
> #endif // _VTD_EXTERN_H_
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index 8ac774215b..c880b0ce21 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1304,12 +1304,6 @@ static void __hwdom_init intel_iommu_hwdom_init(struct domain *d)
> {
> struct acpi_drhd_unit *drhd;
>
> - if ( !iommu_dom0_passthrough && is_pv_domain(d) )
> - {
> - /* Set up 1:1 page table for hardware domain. */
> - vtd_set_hwdom_mapping(d);
> - }
> -
> setup_hwdom_pci_devices(d, setup_hwdom_device);
> setup_hwdom_rmrr(d);
>
> diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
> index 00a9891005..20323051d0 100644
> --- a/xen/drivers/passthrough/vtd/x86/vtd.c
> +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
> @@ -25,7 +25,6 @@
> #include <xen/irq.h>
> #include <xen/numa.h>
> #include <asm/fixmap.h>
> -#include <asm/setup.h>
> #include "../iommu.h"
> #include "../dmar.h"
> #include "../vtd.h"
> @@ -35,8 +34,7 @@
> * iommu_inclusive_mapping: when set, all memory below 4GB is included in dom0
> * 1:1 iommu mappings except xen and unusable regions.
> */
> -static bool_t __hwdom_initdata iommu_inclusive_mapping = 1;
> -boolean_param("iommu_inclusive_mapping", iommu_inclusive_mapping);
> +boolean_param("iommu_inclusive_mapping", iommu_dom0_inclusive);
>
> void *map_vtd_domain_page(u64 maddr)
> {
> @@ -108,57 +106,3 @@ void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
> spin_unlock(&d->event_lock);
> }
>
> -void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
> -{
> - unsigned long i, top, max_pfn;
> -
> - BUG_ON(!is_hardware_domain(d));
> -
> - max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> - top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> -
> - for ( i = 0; i < top; i++ )
> - {
> - unsigned long pfn = pdx_to_pfn(i);
> - bool map;
> - int rc;
> -
> - /*
> - * Set up 1:1 mapping for dom0. Default to include only
> - * conventional RAM areas and let RMRRs include needed reserved
> - * regions. When set, the inclusive mapping additionally maps in
> - * every pfn up to 4GB except those that fall in unusable ranges.
> - */
> - if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
> - continue;
> -
> - if ( iommu_inclusive_mapping && pfn <= max_pfn )
> - map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> - else
> - map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> -
> - if ( !map )
> - continue;
> -
> - /* Exclude Xen bits */
> - if ( xen_in_range(pfn) )
> - continue;
> -
> - /*
> - * If dom0-strict mode is enabled then exclude conventional RAM
> - * and let the common code map dom0's pages.
> - */
> - if ( iommu_dom0_strict &&
> - page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> - continue;
> -
> - rc = iommu_map_page(d, pfn, pfn, IOMMUF_readable|IOMMUF_writable);
> - if ( rc )
> - printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed: %d\n",
> - d->domain_id, rc);
> -
> - if (!(i & 0xfffff))
> - process_pending_softirqs();
> - }
> -}
> -
> diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
> index 68182afd91..bf6edf4c04 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -20,6 +20,8 @@
> #include <xen/softirq.h>
> #include <xsm/xsm.h>
>
> +#include <asm/setup.h>
> +
> void iommu_update_ire_from_apic(
> unsigned int apic, unsigned int reg, unsigned int value)
> {
> @@ -132,6 +134,64 @@ void arch_iommu_domain_destroy(struct domain *d)
> {
> }
>
> +void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> +{
> + unsigned long i, top, max_pfn;
> +
> + BUG_ON(!is_hardware_domain(d));
> +
> + /* Set the default value of inclusive depending on the hardware. */
> + if ( iommu_dom0_inclusive == -1 )
> + iommu_dom0_inclusive = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
> +
> + max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> + top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> +
> + for ( i = 0; i < top; i++ )
> + {
> + unsigned long pfn = pdx_to_pfn(i);
> + bool map;
> + int rc;
> +
> + /*
> + * Set up 1:1 mapping for dom0. Default to include only
> + * conventional RAM areas and let RMRRs include needed reserved
> + * regions. When set, the inclusive mapping additionally maps in
> + * every pfn up to 4GB except those that fall in unusable ranges.
> + */
> + if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
> + continue;
> +
> + if ( iommu_dom0_inclusive && pfn <= max_pfn )
> + map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> + else
> + map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> +
> + if ( !map )
> + continue;
> +
> + /* Exclude Xen bits */
> + if ( xen_in_range(pfn) )
> + continue;
> +
> + /*
> + * If dom0-strict mode is enabled then exclude conventional RAM
> + * and let the common code map dom0's pages.
> + */
> + if ( iommu_dom0_strict &&
> + page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> + continue;
> +
> + rc = iommu_map_page(d, pfn, pfn, IOMMUF_readable|IOMMUF_writable);
> + if ( rc )
> + printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
> + d->domain_id, rc);
> +
> + if (!(i & 0xfffff))
> + process_pending_softirqs();
> + }
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index c0c6975ac4..99e5b89c0f 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -37,6 +37,7 @@ extern bool_t iommu_debug;
> extern bool_t amd_iommu_perdev_intremap;
>
> extern bool iommu_dom0_strict, iommu_dom0_passthrough;
> +extern int8_t iommu_dom0_inclusive;
>
> extern unsigned int iommu_dev_iotlb_timeout;
>
> @@ -51,6 +52,7 @@ void arch_iommu_domain_destroy(struct domain *d);
> int arch_iommu_domain_init(struct domain *d);
> int arch_iommu_populate_page_table(struct domain *d);
> void arch_iommu_check_autotranslated_hwdom(struct domain *d);
> +void arch_iommu_hwdom_init(struct domain *d);
>
> int iommu_construct(struct domain *d);
>
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-03 9:08 ` Roger Pau Monné
@ 2018-08-03 9:14 ` Paul Durrant
2018-08-03 9:32 ` Roger Pau Monné
0 siblings, 1 reply; 29+ messages in thread
From: Paul Durrant @ 2018-08-03 9:14 UTC (permalink / raw)
To: Roger Pau Monne
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
> -----Original Message-----
> From: Roger Pau Monne
> Sent: 03 August 2018 10:09
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian <kevin.tian@intel.com>;
> Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>;
> George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> option
>
> On Fri, Aug 03, 2018 at 10:05:19AM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Roger Pau Monne
> > > Sent: 03 August 2018 09:52
> > > To: Paul Durrant <Paul.Durrant@citrix.com>
> > > Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian
> <kevin.tian@intel.com>;
> > > Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> <wei.liu2@citrix.com>;
> > > George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> > > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Tim
> > > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > > option
> > >
> > > On Fri, Aug 03, 2018 at 09:35:58AM +0100, Paul Durrant wrote:
> > > > > -----Original Message-----
> > > > > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org]
> On
> > > Behalf
> > > > > Of Roger Pau Monné
> > > > > Sent: 03 August 2018 09:14
> > > > > To: Jan Beulich <JBeulich@suse.com>
> > > > > Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> > > > > <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George
> Dunlap
> > > > > <George.Dunlap@citrix.com>; Andrew Cooper
> > > > > <Andrew.Cooper3@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>;
> > > Tim
> > > > > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>;
> Suravee
> > > > > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > > > > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > > > > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-
> iommu
> > > > > option
> > > > >
> > > > > On Thu, Aug 02, 2018 at 02:23:23AM -0600, Jan Beulich wrote:
> > > > > > >>> On 02.08.18 at 09:46, <kevin.tian@intel.com> wrote:
> > > > > > >> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> > > > > > >> Sent: Wednesday, August 1, 2018 7:04 PM
> > > > > > >> --- a/docs/misc/xen-command-line.markdown
> > > > > > >> +++ b/docs/misc/xen-command-line.markdown
> > > > > > >> @@ -1150,12 +1150,18 @@ detection of systems known to
> > > misbehave
> > > > > > >> upon accesses to that port.
> > > > > > >>
> > > > > > >> > `dom0-passthrough`
> > > > > > >>
> > > > > > >> +> **WARNING: This command line option is deprecated, and
> > > > > superseded
> > > > > > >> by
> > > > > > >> +> _dom0-iommu=none_ - using both options in combination is
> > > > > > >> undefined.**
> > > > > > >> +
> > > > > > >
> > > > > > > in patch description you said 'supersede'... is it better to say that
> > > > > > > new dom0_iommu is favored if both options are specified than
> > > > > > > saying 'undefined'?
> > > > > >
> > > > > > That would complicate handling (perhaps just slightly, but anyway),
> > > > > > since we'd have to maintain a second boolean. Without that the
> > > > > > order on the command line determines behavior. (And I see that in
> > > > > > the end you've figured that out.)
> > > > > >
> > > > > > >> @@ -1198,6 +1204,32 @@ detection of systems known to
> > > misbehave
> > > > > upon
> > > > > > >> accesses to that port.
> > > > > > >>
> > > > > > >> >> Enable IOMMU debugging code (implies `verbose`).
> > > > > > >>
> > > > > > >> +### dom0-iommu
> > > > > > >> +> `= List of [ none | strict | relaxed ]`
> > > > > > >> +
> > > > > > >> +> Sub-options are of boolean kind and can be prefixed with `no-
> ` to
> > > > > effect
> > > > > > >> the
> > > > > > >> +> inverse meaning.
> > > > > > >
> > > > > > > not important comment, but doesn't "no-none" sound weird? :-)
> > > > > >
> > > > > > Only positive (true) values should be permitted for I think all of
> > > > > > these. I didn't look at the patch yes, so perhaps that's already
> > > > > > the case.
> > > > >
> > > > > For the current set of options introduced in this patch none, strict
> > > > > or relaxed it doesn't make much sense to allow the no- prefix.
> > > > >
> > > > > For options added in later patches (inclusive and reserved) it makes
> > > > > sense to allow the no- prefix, so that a user can do
> > > > > 'dom0-iommu=no-inclusive' in order to change the default value.
> > > > >
> > > >
> > > > But what does that mean? 'no-inclusive' clearly means you don't get the
> > > inclusive map, but what do you get instead? Strict? None?
> > >
> > > IMO you always either get no iommu mappings at all (none), only Dom0
> > > assigned RAM regions (strict) or all RAM except the regions used by
> > > Xen (relaxed). Those options control what RAM gets mapped into the
> > > iommu page tables.
> > >
> > > Then you can use inclusive to even get more mappings of non-RAM
> > > regions, but that can be used in conjunction with either strict or
> > > relaxed and should allow the usage of the no- prefix.
> > >
> > > So if you use dom0-iommu=no-inclusive you get the default relaxed RAM
> > > mappings and that's all.
> > >
> > > I hope this makes sense, I will try to clarify the documentation.
> > >
> >
> > Actually I wonder whether we should rename 'inclusive' to 'reserved'.
> Essentially 'none', 'strict' or 'relaxed' are all about mappings of RAM, and then
> we need to decide whether to map the E820 reserved regions. So I think the
> inclusive map as it stands today is equivalent to 'relaxed' + 'reserved'.
>
> Hm, not exactly. inclusive (iommu_inclusive_mapping) right now maps
> everything except unusable regions. That's way more than just mapping
> reserved regions. If we want to keep this behaviour while introducing
> an option to map only reserved regions we need both an inclusive and a
> reserved option.
>
Ok, how about:
inclusive -> all E820 ranges except unusable or ram
reserved -> all E820 reserved ranges
then
strict -> all ram ranges belonging to dom0
relaxed -> all ram ranges
none -> no ram ranges
The problem then is what does, say, reserved + no-inclusive mean? I guess we could have a flag for each non ram E820 range type?
Paul
> Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-03 9:14 ` Paul Durrant
@ 2018-08-03 9:32 ` Roger Pau Monné
2018-08-03 9:37 ` Paul Durrant
2018-08-06 3:19 ` Tian, Kevin
0 siblings, 2 replies; 29+ messages in thread
From: Roger Pau Monné @ 2018-08-03 9:32 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
On Fri, Aug 03, 2018 at 10:14:49AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne
> > Sent: 03 August 2018 10:09
> > To: Paul Durrant <Paul.Durrant@citrix.com>
> > Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian <kevin.tian@intel.com>;
> > Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>;
> > George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > option
> >
> > On Fri, Aug 03, 2018 at 10:05:19AM +0100, Paul Durrant wrote:
> > > Actually I wonder whether we should rename 'inclusive' to 'reserved'.
> > Essentially 'none', 'strict' or 'relaxed' are all about mappings of RAM, and then
> > we need to decide whether to map the E820 reserved regions. So I think the
> > inclusive map as it stands today is equivalent to 'relaxed' + 'reserved'.
> >
> > Hm, not exactly. inclusive (iommu_inclusive_mapping) right now maps
> > everything except unusable regions. That's way more than just mapping
> > reserved regions. If we want to keep this behaviour while introducing
> > an option to map only reserved regions we need both an inclusive and a
> > reserved option.
> >
>
> Ok, how about:
>
> inclusive -> all E820 ranges except unusable or ram
inclusive ATM also maps holes. So it would be all memory ranges except
those marked as unusable or in use by Xen.
> reserved -> all E820 reserved ranges
>
> then
>
> strict -> all ram ranges belonging to dom0
> relaxed -> all ram ranges
> none -> no ram ranges
>
> The problem then is what does, say, reserved + no-inclusive mean? I guess we could have a flag for each non ram E820 range type?
reserved + no-inclusive would make sense for a PV Dom0 running on
Intel hardware in order to map only the reserved regions instead of
mapping almost everything below 4GB by default.
What about the following description of the options, do you think it's
clear enough?
> `= List of [ none | strict | relaxed | inclusive | reserved ]`
* `none`: disables DMA remapping for Dom0.
The following two options control how RAM regions are mapped in the iommu for
Dom0:
* `strict`: sets up DMA remapping only for the memory Dom0 actually got
assigned.
* `relaxed`: sets DMA remapping for all the host RAM except regions in use by
Xen. This is the default iommu behaviour.
Note that all the above options are mutually exclusive. Specifying more than
one on the `dom0-iommu` command line will result in undefined behavior.
The following options control whether non-RAM regions are added to the Dom0
iommu tables. Note that they can be prefixed with `no-` to effect the inverse
meaning:
* `inclusive`: sets up DMA remapping for all the non-RAM memory 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.
* `reserved`: sets up DMA remapping for all the reserved regions in the memory
map for Dom0. Use this to work around firmware issues providing incorrect
RMRR or IVMD entries. Rather than only mapping RAM pages for IOMMU accesses
for Dom0, all memory regions marked as reserved in the memory map that don't
overlap with any MMIO region from emulated devices will be identity mapped.
This option maps a subset of the memory that would be mapped when using the
`inclusive` option. This option is available to a PVH Dom0 and is enabled by
default on Intel hardware.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-03 9:32 ` Roger Pau Monné
@ 2018-08-03 9:37 ` Paul Durrant
2018-08-06 3:19 ` Tian, Kevin
1 sibling, 0 replies; 29+ messages in thread
From: Paul Durrant @ 2018-08-03 9:37 UTC (permalink / raw)
To: Roger Pau Monne
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
> -----Original Message-----
[snip]
> > The problem then is what does, say, reserved + no-inclusive mean? I guess
> we could have a flag for each non ram E820 range type?
>
> reserved + no-inclusive would make sense for a PV Dom0 running on
> Intel hardware in order to map only the reserved regions instead of
> mapping almost everything below 4GB by default.
Ok, that makes sense.
>
> What about the following description of the options, do you think it's
> clear enough?
>
> > `= List of [ none | strict | relaxed | inclusive | reserved ]`
>
> * `none`: disables DMA remapping for Dom0.
>
> The following two options control how RAM regions are mapped in the
> iommu for
> Dom0:
>
> * `strict`: sets up DMA remapping only for the memory Dom0 actually got
> assigned.
>
> * `relaxed`: sets DMA remapping for all the host RAM except regions in use
> by
> Xen. This is the default iommu behaviour.
>
> Note that all the above options are mutually exclusive. Specifying more than
> one on the `dom0-iommu` command line will result in undefined behavior.
>
> The following options control whether non-RAM regions are added to the
> Dom0
> iommu tables. Note that they can be prefixed with `no-` to effect the inverse
> meaning:
>
> * `inclusive`: sets up DMA remapping for all the non-RAM memory 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.
>
> * `reserved`: sets up DMA remapping for all the reserved regions in the
> memory
> map for Dom0. Use this to work around firmware issues providing incorrect
> RMRR or IVMD entries. Rather than only mapping RAM pages for IOMMU
> accesses
> for Dom0, all memory regions marked as reserved in the memory map that
> don't
> overlap with any MMIO region from emulated devices will be identity
> mapped.
> This option maps a subset of the memory that would be mapped when
> using the
> `inclusive` option. This option is available to a PVH Dom0 and is enabled by
> default on Intel hardware.
>
With that explanation, I think it is clear enough :-)
Cheers,
Paul
> Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
2018-08-02 7:58 ` Tian, Kevin
@ 2018-08-03 10:39 ` Roger Pau Monné
0 siblings, 0 replies; 29+ messages in thread
From: Roger Pau Monné @ 2018-08-03 10:39 UTC (permalink / raw)
To: Tian, Kevin
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich, xen-devel
On Thu, Aug 02, 2018 at 07:58:48AM +0000, Tian, Kevin wrote:
> > +void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> > +{
> > + unsigned long i, top, max_pfn;
> > +
> > + BUG_ON(!is_hardware_domain(d));
> > +
> > + /* Set the default value of inclusive depending on the hardware. */
> > + if ( iommu_dom0_inclusive == -1 )
> > + iommu_dom0_inclusive = boot_cpu_data.x86_vendor ==
> > X86_VENDOR_INTEL;
> > +
>
> I don't like above style.
>
> btw It's better to set it in intel_iommu_hwdom_init.
Then I will also have to set the AMD default in amd_iommu_hwdom_init.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-03 9:32 ` Roger Pau Monné
2018-08-03 9:37 ` Paul Durrant
@ 2018-08-06 3:19 ` Tian, Kevin
2018-08-06 7:51 ` Paul Durrant
1 sibling, 1 reply; 29+ messages in thread
From: Tian, Kevin @ 2018-08-06 3:19 UTC (permalink / raw)
To: Roger Pau Monné, Paul Durrant
Cc: Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
> From: Roger Pau Monné [mailto:roger.pau@citrix.com]
> Sent: Friday, August 3, 2018 5:33 PM
>
> On Fri, Aug 03, 2018 at 10:14:49AM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Roger Pau Monne
> > > Sent: 03 August 2018 10:09
> > > To: Paul Durrant <Paul.Durrant@citrix.com>
> > > Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian
> <kevin.tian@intel.com>;
> > > Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> <wei.liu2@citrix.com>;
> > > George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> > > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Tim
> > > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > > option
> > >
> > > On Fri, Aug 03, 2018 at 10:05:19AM +0100, Paul Durrant wrote:
> > > > Actually I wonder whether we should rename 'inclusive' to 'reserved'.
> > > Essentially 'none', 'strict' or 'relaxed' are all about mappings of RAM,
> and then
> > > we need to decide whether to map the E820 reserved regions. So I think
> the
> > > inclusive map as it stands today is equivalent to 'relaxed' + 'reserved'.
> > >
> > > Hm, not exactly. inclusive (iommu_inclusive_mapping) right now maps
> > > everything except unusable regions. That's way more than just mapping
> > > reserved regions. If we want to keep this behaviour while introducing
> > > an option to map only reserved regions we need both an inclusive and a
> > > reserved option.
> > >
> >
> > Ok, how about:
> >
> > inclusive -> all E820 ranges except unusable or ram
>
> inclusive ATM also maps holes. So it would be all memory ranges except
> those marked as unusable or in use by Xen.
>
> > reserved -> all E820 reserved ranges
> >
> > then
> >
> > strict -> all ram ranges belonging to dom0
> > relaxed -> all ram ranges
> > none -> no ram ranges
> >
> > The problem then is what does, say, reserved + no-inclusive mean? I
> guess we could have a flag for each non ram E820 range type?
>
> reserved + no-inclusive would make sense for a PV Dom0 running on
> Intel hardware in order to map only the reserved regions instead of
> mapping almost everything below 4GB by default.
>
> What about the following description of the options, do you think it's
> clear enough?
>
> > `= List of [ none | strict | relaxed | inclusive | reserved ]`
>
> * `none`: disables DMA remapping for Dom0.
>
> The following two options control how RAM regions are mapped in the
> iommu for
> Dom0:
>
> * `strict`: sets up DMA remapping only for the memory Dom0 actually got
> assigned.
>
> * `relaxed`: sets DMA remapping for all the host RAM except regions in use
> by
> Xen. This is the default iommu behaviour.
>
> Note that all the above options are mutually exclusive. Specifying more
> than
> one on the `dom0-iommu` command line will result in undefined behavior.
>
> The following options control whether non-RAM regions are added to the
> Dom0
> iommu tables. Note that they can be prefixed with `no-` to effect the
> inverse
> meaning:
>
> * `inclusive`: sets up DMA remapping for all the non-RAM memory 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.
>
> * `reserved`: sets up DMA remapping for all the reserved regions in the
> memory
> map for Dom0. Use this to work around firmware issues providing
> incorrect
> RMRR or IVMD entries. Rather than only mapping RAM pages for IOMMU
> accesses
> for Dom0, all memory regions marked as reserved in the memory map
> that don't
> overlap with any MMIO region from emulated devices will be identity
> mapped.
> This option maps a subset of the memory that would be mapped when
> using the
> `inclusive` option. This option is available to a PVH Dom0 and is enabled
> by
> default on Intel hardware.
>
above makes it clear now. Just a side question. Is there also value of applying
'reserved' option to PV Dom0?
Thanks,
Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-06 3:19 ` Tian, Kevin
@ 2018-08-06 7:51 ` Paul Durrant
2018-08-06 9:09 ` Roger Pau Monné
0 siblings, 1 reply; 29+ messages in thread
From: Paul Durrant @ 2018-08-06 7:51 UTC (permalink / raw)
To: Kevin Tian, Roger Pau Monne
Cc: Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
> -----Original Message-----
> From: Tian, Kevin [mailto:kevin.tian@intel.com]
> Sent: 06 August 2018 04:19
> To: Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant
> <Paul.Durrant@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> Subject: RE: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> option
>
> > From: Roger Pau Monné [mailto:roger.pau@citrix.com]
> > Sent: Friday, August 3, 2018 5:33 PM
> >
> > On Fri, Aug 03, 2018 at 10:14:49AM +0100, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Roger Pau Monne
> > > > Sent: 03 August 2018 10:09
> > > > To: Paul Durrant <Paul.Durrant@citrix.com>
> > > > Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian
> > <kevin.tian@intel.com>;
> > > > Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> > <wei.liu2@citrix.com>;
> > > > George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> > > > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> > Tim
> > > > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > > > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > > > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > > > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-
> iommu
> > > > option
> > > >
> > > > On Fri, Aug 03, 2018 at 10:05:19AM +0100, Paul Durrant wrote:
> > > > > Actually I wonder whether we should rename 'inclusive' to 'reserved'.
> > > > Essentially 'none', 'strict' or 'relaxed' are all about mappings of RAM,
> > and then
> > > > we need to decide whether to map the E820 reserved regions. So I
> think
> > the
> > > > inclusive map as it stands today is equivalent to 'relaxed' + 'reserved'.
> > > >
> > > > Hm, not exactly. inclusive (iommu_inclusive_mapping) right now maps
> > > > everything except unusable regions. That's way more than just
> mapping
> > > > reserved regions. If we want to keep this behaviour while introducing
> > > > an option to map only reserved regions we need both an inclusive and a
> > > > reserved option.
> > > >
> > >
> > > Ok, how about:
> > >
> > > inclusive -> all E820 ranges except unusable or ram
> >
> > inclusive ATM also maps holes. So it would be all memory ranges except
> > those marked as unusable or in use by Xen.
> >
> > > reserved -> all E820 reserved ranges
> > >
> > > then
> > >
> > > strict -> all ram ranges belonging to dom0
> > > relaxed -> all ram ranges
> > > none -> no ram ranges
> > >
> > > The problem then is what does, say, reserved + no-inclusive mean? I
> > guess we could have a flag for each non ram E820 range type?
> >
> > reserved + no-inclusive would make sense for a PV Dom0 running on
> > Intel hardware in order to map only the reserved regions instead of
> > mapping almost everything below 4GB by default.
> >
> > What about the following description of the options, do you think it's
> > clear enough?
> >
> > > `= List of [ none | strict | relaxed | inclusive | reserved ]`
> >
> > * `none`: disables DMA remapping for Dom0.
> >
> > The following two options control how RAM regions are mapped in the
> > iommu for
> > Dom0:
> >
> > * `strict`: sets up DMA remapping only for the memory Dom0 actually got
> > assigned.
> >
> > * `relaxed`: sets DMA remapping for all the host RAM except regions in use
> > by
> > Xen. This is the default iommu behaviour.
> >
> > Note that all the above options are mutually exclusive. Specifying more
> > than
> > one on the `dom0-iommu` command line will result in undefined behavior.
> >
> > The following options control whether non-RAM regions are added to the
> > Dom0
> > iommu tables. Note that they can be prefixed with `no-` to effect the
> > inverse
> > meaning:
> >
> > * `inclusive`: sets up DMA remapping for all the non-RAM memory 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.
> >
> > * `reserved`: sets up DMA remapping for all the reserved regions in the
> > memory
> > map for Dom0. Use this to work around firmware issues providing
> > incorrect
> > RMRR or IVMD entries. Rather than only mapping RAM pages for IOMMU
> > accesses
> > for Dom0, all memory regions marked as reserved in the memory map
> > that don't
> > overlap with any MMIO region from emulated devices will be identity
> > mapped.
> > This option maps a subset of the memory that would be mapped when
> > using the
> > `inclusive` option. This option is available to a PVH Dom0 and is enabled
> > by
> > default on Intel hardware.
> >
>
> above makes it clear now. Just a side question. Is there also value of applying
> 'reserved' option to PV Dom0?
Absolutely. I don't think the text implies it's not available for PV dom0, just that (unlike 'inclusive') it is also available to PVH dom0.
Paul
>
> Thanks,
> Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option
2018-08-06 7:51 ` Paul Durrant
@ 2018-08-06 9:09 ` Roger Pau Monné
0 siblings, 0 replies; 29+ messages in thread
From: Roger Pau Monné @ 2018-08-06 9:09 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson,
Brian Woods
On Mon, Aug 06, 2018 at 08:51:55AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Tian, Kevin [mailto:kevin.tian@intel.com]
> > Sent: 06 August 2018 04:19
> > To: Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant
> > <Paul.Durrant@citrix.com>
> > Cc: Jan Beulich <JBeulich@suse.com>; Stefano Stabellini
> > <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> > <George.Dunlap@citrix.com>; Andrew Cooper
> > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > Subject: RE: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > option
> >
> > > From: Roger Pau Monné [mailto:roger.pau@citrix.com]
> > > Sent: Friday, August 3, 2018 5:33 PM
> > >
> > > On Fri, Aug 03, 2018 at 10:14:49AM +0100, Paul Durrant wrote:
> > > > > -----Original Message-----
> > > > > From: Roger Pau Monne
> > > > > Sent: 03 August 2018 10:09
> > > > > To: Paul Durrant <Paul.Durrant@citrix.com>
> > > > > Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian
> > > <kevin.tian@intel.com>;
> > > > > Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> > > <wei.liu2@citrix.com>;
> > > > > George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> > > > > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> > > Tim
> > > > > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > > > > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > > > > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > > > > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-
> > iommu
> > > > > option
> > > > >
> > > > > On Fri, Aug 03, 2018 at 10:05:19AM +0100, Paul Durrant wrote:
> > > > > > Actually I wonder whether we should rename 'inclusive' to 'reserved'.
> > > > > Essentially 'none', 'strict' or 'relaxed' are all about mappings of RAM,
> > > and then
> > > > > we need to decide whether to map the E820 reserved regions. So I
> > think
> > > the
> > > > > inclusive map as it stands today is equivalent to 'relaxed' + 'reserved'.
> > > > >
> > > > > Hm, not exactly. inclusive (iommu_inclusive_mapping) right now maps
> > > > > everything except unusable regions. That's way more than just
> > mapping
> > > > > reserved regions. If we want to keep this behaviour while introducing
> > > > > an option to map only reserved regions we need both an inclusive and a
> > > > > reserved option.
> > > > >
> > > >
> > > > Ok, how about:
> > > >
> > > > inclusive -> all E820 ranges except unusable or ram
> > >
> > > inclusive ATM also maps holes. So it would be all memory ranges except
> > > those marked as unusable or in use by Xen.
> > >
> > > > reserved -> all E820 reserved ranges
> > > >
> > > > then
> > > >
> > > > strict -> all ram ranges belonging to dom0
> > > > relaxed -> all ram ranges
> > > > none -> no ram ranges
> > > >
> > > > The problem then is what does, say, reserved + no-inclusive mean? I
> > > guess we could have a flag for each non ram E820 range type?
> > >
> > > reserved + no-inclusive would make sense for a PV Dom0 running on
> > > Intel hardware in order to map only the reserved regions instead of
> > > mapping almost everything below 4GB by default.
> > >
> > > What about the following description of the options, do you think it's
> > > clear enough?
> > >
> > > > `= List of [ none | strict | relaxed | inclusive | reserved ]`
> > >
> > > * `none`: disables DMA remapping for Dom0.
> > >
> > > The following two options control how RAM regions are mapped in the
> > > iommu for
> > > Dom0:
> > >
> > > * `strict`: sets up DMA remapping only for the memory Dom0 actually got
> > > assigned.
> > >
> > > * `relaxed`: sets DMA remapping for all the host RAM except regions in use
> > > by
> > > Xen. This is the default iommu behaviour.
> > >
> > > Note that all the above options are mutually exclusive. Specifying more
> > > than
> > > one on the `dom0-iommu` command line will result in undefined behavior.
> > >
> > > The following options control whether non-RAM regions are added to the
> > > Dom0
> > > iommu tables. Note that they can be prefixed with `no-` to effect the
> > > inverse
> > > meaning:
> > >
> > > * `inclusive`: sets up DMA remapping for all the non-RAM memory 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.
> > >
> > > * `reserved`: sets up DMA remapping for all the reserved regions in the
> > > memory
> > > map for Dom0. Use this to work around firmware issues providing
> > > incorrect
> > > RMRR or IVMD entries. Rather than only mapping RAM pages for IOMMU
> > > accesses
> > > for Dom0, all memory regions marked as reserved in the memory map
> > > that don't
> > > overlap with any MMIO region from emulated devices will be identity
> > > mapped.
> > > This option maps a subset of the memory that would be mapped when
> > > using the
> > > `inclusive` option. This option is available to a PVH Dom0 and is enabled
> > > by
> > > default on Intel hardware.
> > >
> >
> > above makes it clear now. Just a side question. Is there also value of applying
> > 'reserved' option to PV Dom0?
>
> Absolutely. I don't think the text implies it's not available for PV dom0, just that (unlike 'inclusive') it is also available to PVH dom0.
For a PV Dom0 using just:
dom0-iommu=reserved
On Intel hardware doesn't change the behaviour, since reserved is a
subset of inclusive. OTOH, using something like:
dom0-iommu=no-inclusive,reserved
Will have an effect even on Intel hardware, and the memory mapped will
be limited to reserved regions only.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2018-08-06 9:09 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-01 11:03 [PATCH v2 0/5] x86/iommu: PVH Dom0 workarounds for missing RMRR entries Roger Pau Monne
2018-08-01 11:03 ` [PATCH v2 1/5] iommu/vtd: cleanup vtd_set_hwdom_mapping after ia64 removal Roger Pau Monne
2018-08-01 11:16 ` Paul Durrant
2018-08-02 7:36 ` Tian, Kevin
2018-08-01 11:03 ` [PATCH v2 2/5] iommu: introduce dom0-iommu option Roger Pau Monne
2018-08-01 11:15 ` Paul Durrant
2018-08-01 11:16 ` Andrew Cooper
2018-08-02 7:46 ` Tian, Kevin
2018-08-02 8:23 ` Jan Beulich
2018-08-03 8:14 ` Roger Pau Monné
2018-08-03 8:35 ` Paul Durrant
2018-08-03 8:52 ` Roger Pau Monné
2018-08-03 9:05 ` Paul Durrant
2018-08-03 9:08 ` Roger Pau Monné
2018-08-03 9:14 ` Paul Durrant
2018-08-03 9:32 ` Roger Pau Monné
2018-08-03 9:37 ` Paul Durrant
2018-08-06 3:19 ` Tian, Kevin
2018-08-06 7:51 ` Paul Durrant
2018-08-06 9:09 ` Roger Pau Monné
2018-08-01 11:03 ` [PATCH v2 3/5] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu Roger Pau Monne
2018-08-01 11:21 ` Paul Durrant
2018-08-02 7:58 ` Tian, Kevin
2018-08-03 10:39 ` Roger Pau Monné
2018-08-03 9:09 ` Julien Grall
2018-08-01 11:03 ` [PATCH v2 4/5] dom0/pvh: change the order of the MMCFG initialization Roger Pau Monne
2018-08-01 11:03 ` [PATCH v2 5/5] x86/iommu: add PVH support to the inclusive options Roger Pau Monne
2018-08-01 11:27 ` Paul Durrant
2018-08-02 8:03 ` Tian, Kevin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.