* [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables
@ 2020-03-09 10:39 Jan Beulich
2020-03-09 10:42 ` [Xen-devel] [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only Jan Beulich
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Jan Beulich @ 2020-03-09 10:39 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Wilk, George Dunlap, Andrew Cooper, Ian Jackson
A number of the command line controlled variables are x86-
or even x86-HVM-specific. Don't have those variables elsewhere
in the first place (in some cases replace them by a #define),
and as a result also don't silently accept such "iommu="
sub-options which in fact have no effect.
1: iommu_intremap is x86-only
2: iommu_intpost is x86/HVM-only
3: iommu_igfx is x86-only
4: iommu_qinval is x86-only
5: iommu_snoop is x86-only
The series contextually depends on "AMD/IOMMU: without XT,
x2APIC needs to be forced into physical mode"
v2 addresses review comments, at least as far as agreement was
reached. See individual patches.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Xen-devel] [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only
2020-03-09 10:39 [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Jan Beulich
@ 2020-03-09 10:42 ` Jan Beulich
2020-03-10 10:44 ` Paul Durrant
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only Jan Beulich
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2020-03-09 10:42 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Wilk, George Dunlap, Andrew Cooper, Paul Durrant,
Ian Jackson
Provide a #define for other cases; it didn't seem worthwhile to me to
introduce an IOMMU_INTREMAP Kconfig option at this point.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Refine doc adjustment.
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1299,6 +1299,8 @@ boolean (e.g. `iommu=no`) can override t
generation of IOMMUs only supported DMA remapping, and Interrupt Remapping
appeared in the second generation.
+ This option is only valid on x86.
+
* The `intpost` boolean controls the Posted Interrupt sub-feature. In
combination with APIC acceleration (VT-x APICV, SVM AVIC), the IOMMU can
be configured to deliver interrupts from assigned PCI devices directly
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -35,7 +35,6 @@ bool __read_mostly iommu_quarantine = tr
bool_t __read_mostly iommu_igfx = 1;
bool_t __read_mostly iommu_snoop = 1;
bool_t __read_mostly iommu_qinval = 1;
-enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
bool_t __read_mostly iommu_crash_disable;
static bool __hwdom_initdata iommu_hwdom_none;
@@ -90,8 +89,10 @@ static int __init parse_iommu_param(cons
iommu_snoop = val;
else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
iommu_qinval = val;
+#ifndef iommu_intremap
else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
+#endif
else if ( (val = parse_boolean("intpost", s, ss)) >= 0 )
iommu_intpost = val;
#ifdef CONFIG_KEXEC
@@ -474,8 +475,11 @@ int __init iommu_setup(void)
rc = iommu_hardware_setup();
iommu_enabled = (rc == 0);
}
+
+#ifndef iommu_intremap
if ( !iommu_enabled )
iommu_intremap = iommu_intremap_off;
+#endif
if ( (force_iommu && !iommu_enabled) ||
(force_intremap && !iommu_intremap) )
@@ -500,7 +504,9 @@ int __init iommu_setup(void)
printk(" - Dom0 mode: %s\n",
iommu_hwdom_passthrough ? "Passthrough" :
iommu_hwdom_strict ? "Strict" : "Relaxed");
+#ifndef iommu_intremap
printk("Interrupt remapping %sabled\n", iommu_intremap ? "en" : "dis");
+#endif
tasklet_init(&iommu_pt_cleanup_tasklet, iommu_free_pagetables, NULL);
}
@@ -558,7 +564,9 @@ void iommu_crash_shutdown(void)
if ( iommu_enabled )
iommu_get_ops()->crash_shutdown();
iommu_enabled = iommu_intpost = 0;
+#ifndef iommu_intremap
iommu_intremap = iommu_intremap_off;
+#endif
}
int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -27,6 +27,8 @@
const struct iommu_init_ops *__initdata iommu_init_ops;
struct iommu_ops __read_mostly iommu_ops;
+enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
+
int __init iommu_hardware_setup(void)
{
struct IO_APIC_route_entry **ioapic_entries = NULL;
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -55,21 +55,24 @@ static inline bool_t dfn_eq(dfn_t x, dfn
extern bool_t iommu_enable, iommu_enabled;
extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
extern bool_t iommu_snoop, iommu_qinval, iommu_intpost;
+
+#ifdef CONFIG_X86
extern enum __packed iommu_intremap {
/*
* In order to allow traditional boolean uses of the iommu_intremap
* variable, the "off" value has to come first (yielding a value of zero).
*/
iommu_intremap_off,
-#ifdef CONFIG_X86
/*
* Interrupt remapping enabled, but only able to generate interrupts
* with an 8-bit APIC ID.
*/
iommu_intremap_restricted,
-#endif
iommu_intremap_full,
} iommu_intremap;
+#else
+# define iommu_intremap false
+#endif
#if defined(CONFIG_IOMMU_FORCE_PT_SHARE)
#define iommu_hap_pt_share true
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
2020-03-09 10:39 [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Jan Beulich
2020-03-09 10:42 ` [Xen-devel] [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only Jan Beulich
@ 2020-03-09 10:43 ` Jan Beulich
2020-03-10 1:13 ` Tian, Kevin
2020-03-10 10:54 ` Paul Durrant
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 3/5] IOMMU: iommu_igfx is x86-only Jan Beulich
` (3 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: Jan Beulich @ 2020-03-09 10:43 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Wilk, George Dunlap, Andrew Cooper, Paul Durrant,
Ian Jackson
Provide a #define for all other cases.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Refine doc adjustment.
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1309,6 +1309,8 @@ boolean (e.g. `iommu=no`) can override t
This option depends on `intremap`, and is disabled by default due to some
corner cases in the implementation which have yet to be resolved.
+ This option is only valid on x86, and only builds of Xen with HVM support.
+
* The `crash-disable` boolean controls disabling IOMMU functionality (DMAR/IR/QI)
before switching to a crash kernel. This option is inactive by default and
is for compatibility with older kdump kernels only. Modern kernels copy
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -43,14 +43,6 @@ bool __read_mostly iommu_hwdom_passthrou
bool __hwdom_initdata iommu_hwdom_inclusive;
int8_t __hwdom_initdata iommu_hwdom_reserved = -1;
-/*
- * In the current implementation of VT-d posted interrupts, in some extreme
- * cases, the per cpu list which saves the blocked vCPU will be very long,
- * and this will affect the interrupt latency, so let this feature off by
- * default until we find a good solution to resolve it.
- */
-bool_t __read_mostly iommu_intpost;
-
#ifndef iommu_hap_pt_share
bool __read_mostly iommu_hap_pt_share = true;
#endif
@@ -93,8 +85,10 @@ static int __init parse_iommu_param(cons
else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
#endif
+#ifndef iommu_intpost
else if ( (val = parse_boolean("intpost", s, ss)) >= 0 )
iommu_intpost = val;
+#endif
#ifdef CONFIG_KEXEC
else if ( (val = parse_boolean("crash-disable", s, ss)) >= 0 )
iommu_crash_disable = val;
@@ -486,8 +480,10 @@ int __init iommu_setup(void)
panic("Couldn't enable %s and iommu=required/force\n",
!iommu_enabled ? "IOMMU" : "Interrupt Remapping");
+#ifndef iommu_intpost
if ( !iommu_intremap )
iommu_intpost = 0;
+#endif
printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
if ( !iommu_enabled )
@@ -563,10 +559,13 @@ void iommu_crash_shutdown(void)
if ( iommu_enabled )
iommu_get_ops()->crash_shutdown();
- iommu_enabled = iommu_intpost = 0;
+ iommu_enabled = false;
#ifndef iommu_intremap
iommu_intremap = iommu_intremap_off;
#endif
+#ifndef iommu_intpost
+ iommu_intpost = false;
+#endif
}
int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2297,13 +2297,15 @@ static int __init vtd_setup(void)
if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) )
iommu_intremap = iommu_intremap_off;
+#ifndef iommu_intpost
/*
* We cannot use posted interrupt if X86_FEATURE_CX16 is
* not supported, since we count on this feature to
* atomically update 16-byte IRTE in posted format.
*/
if ( !cap_intr_post(iommu->cap) || !iommu_intremap || !cpu_has_cx16 )
- iommu_intpost = 0;
+ iommu_intpost = false;
+#endif
if ( !vtd_ept_page_compatible(iommu) )
clear_iommu_hap_pt_share();
@@ -2330,7 +2332,9 @@ static int __init vtd_setup(void)
P(iommu_hwdom_passthrough, "Dom0 DMA Passthrough");
P(iommu_qinval, "Queued Invalidation");
P(iommu_intremap, "Interrupt Remapping");
+#ifndef iommu_intpost
P(iommu_intpost, "Posted Interrupt");
+#endif
P(iommu_hap_pt_share, "Shared EPT tables");
#undef P
@@ -2348,7 +2352,9 @@ static int __init vtd_setup(void)
iommu_hwdom_passthrough = false;
iommu_qinval = 0;
iommu_intremap = iommu_intremap_off;
- iommu_intpost = 0;
+#ifndef iommu_intpost
+ iommu_intpost = false;
+#endif
return ret;
}
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -29,6 +29,16 @@ struct iommu_ops __read_mostly iommu_ops
enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
+#ifndef iommu_intpost
+/*
+ * In the current implementation of VT-d posted interrupts, in some extreme
+ * cases, the per cpu list which saves the blocked vCPU will be very long,
+ * and this will affect the interrupt latency, so let this feature off by
+ * default until we find a good solution to resolve it.
+ */
+bool __read_mostly iommu_intpost;
+#endif
+
int __init iommu_hardware_setup(void)
{
struct IO_APIC_route_entry **ioapic_entries = NULL;
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -54,7 +54,7 @@ static inline bool_t dfn_eq(dfn_t x, dfn
extern bool_t iommu_enable, iommu_enabled;
extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
-extern bool_t iommu_snoop, iommu_qinval, iommu_intpost;
+extern bool_t iommu_snoop, iommu_qinval;
#ifdef CONFIG_X86
extern enum __packed iommu_intremap {
@@ -74,6 +74,12 @@ extern enum __packed iommu_intremap {
# define iommu_intremap false
#endif
+#if defined(CONFIG_X86) && defined(CONFIG_HVM)
+extern bool iommu_intpost;
+#else
+# define iommu_intpost false
+#endif
+
#if defined(CONFIG_IOMMU_FORCE_PT_SHARE)
#define iommu_hap_pt_share true
#elif defined(CONFIG_HVM)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Xen-devel] [PATCH v2 3/5] IOMMU: iommu_igfx is x86-only
2020-03-09 10:39 [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Jan Beulich
2020-03-09 10:42 ` [Xen-devel] [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only Jan Beulich
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only Jan Beulich
@ 2020-03-09 10:43 ` Jan Beulich
2020-03-10 10:46 ` Paul Durrant
2020-03-09 10:44 ` [Xen-devel] [PATCH v2 4/5] IOMMU: iommu_qinval " Jan Beulich
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2020-03-09 10:43 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Wilk, George Dunlap, Andrew Cooper, Paul Durrant,
Ian Jackson
In fact it's VT-d specific, but we don't have a way yet to build code
for just one vendor.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -32,7 +32,6 @@ bool_t __read_mostly iommu_enabled;
bool_t __read_mostly force_iommu;
bool_t __read_mostly iommu_verbose;
bool __read_mostly iommu_quarantine = true;
-bool_t __read_mostly iommu_igfx = 1;
bool_t __read_mostly iommu_snoop = 1;
bool_t __read_mostly iommu_qinval = 1;
bool_t __read_mostly iommu_crash_disable;
@@ -73,8 +72,10 @@ static int __init parse_iommu_param(cons
force_iommu = val;
else if ( (val = parse_boolean("quarantine", s, ss)) >= 0 )
iommu_quarantine = val;
+#ifdef CONFIG_X86
else if ( (val = parse_boolean("igfx", s, ss)) >= 0 )
iommu_igfx = val;
+#endif
else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
iommu_verbose = val;
else if ( (val = parse_boolean("snoop", s, ss)) >= 0 )
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -50,6 +50,8 @@ struct mapped_rmrr {
/* Possible unfiltered LAPIC/MSI messages from untrusted sources? */
bool __read_mostly untrusted_msi;
+bool __read_mostly iommu_igfx = true;
+
int nr_iommus;
static struct tasklet vtd_fault_tasklet;
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -53,7 +53,7 @@ static inline bool_t dfn_eq(dfn_t x, dfn
}
extern bool_t iommu_enable, iommu_enabled;
-extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
+extern bool force_iommu, iommu_quarantine, iommu_verbose;
extern bool_t iommu_snoop, iommu_qinval;
#ifdef CONFIG_X86
@@ -70,6 +70,7 @@ extern enum __packed iommu_intremap {
iommu_intremap_restricted,
iommu_intremap_full,
} iommu_intremap;
+extern bool iommu_igfx;
#else
# define iommu_intremap false
#endif
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Xen-devel] [PATCH v2 4/5] IOMMU: iommu_qinval is x86-only
2020-03-09 10:39 [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Jan Beulich
` (2 preceding siblings ...)
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 3/5] IOMMU: iommu_igfx is x86-only Jan Beulich
@ 2020-03-09 10:44 ` Jan Beulich
2020-03-10 10:47 ` Paul Durrant
2020-03-09 10:44 ` [Xen-devel] [PATCH v2 5/5] IOMMU: iommu_snoop " Jan Beulich
2020-03-10 1:19 ` [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Tian, Kevin
5 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2020-03-09 10:44 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Wilk, George Dunlap, Andrew Cooper, Paul Durrant,
Ian Jackson
In fact it's VT-d specific, but we don't have a way yet to build code
for just one vendor.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -33,7 +33,6 @@ bool_t __read_mostly force_iommu;
bool_t __read_mostly iommu_verbose;
bool __read_mostly iommu_quarantine = true;
bool_t __read_mostly iommu_snoop = 1;
-bool_t __read_mostly iommu_qinval = 1;
bool_t __read_mostly iommu_crash_disable;
static bool __hwdom_initdata iommu_hwdom_none;
@@ -75,13 +74,13 @@ static int __init parse_iommu_param(cons
#ifdef CONFIG_X86
else if ( (val = parse_boolean("igfx", s, ss)) >= 0 )
iommu_igfx = val;
+ else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
+ iommu_qinval = val;
#endif
else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
iommu_verbose = val;
else if ( (val = parse_boolean("snoop", s, ss)) >= 0 )
iommu_snoop = val;
- else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
- iommu_qinval = val;
#ifndef iommu_intremap
else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -51,6 +51,7 @@ struct mapped_rmrr {
bool __read_mostly untrusted_msi;
bool __read_mostly iommu_igfx = true;
+bool __read_mostly iommu_qinval = true;
int nr_iommus;
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -54,7 +54,7 @@ static inline bool_t dfn_eq(dfn_t x, dfn
extern bool_t iommu_enable, iommu_enabled;
extern bool force_iommu, iommu_quarantine, iommu_verbose;
-extern bool_t iommu_snoop, iommu_qinval;
+extern bool_t iommu_snoop;
#ifdef CONFIG_X86
extern enum __packed iommu_intremap {
@@ -70,7 +70,7 @@ extern enum __packed iommu_intremap {
iommu_intremap_restricted,
iommu_intremap_full,
} iommu_intremap;
-extern bool iommu_igfx;
+extern bool iommu_igfx, iommu_qinval;
#else
# define iommu_intremap false
#endif
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Xen-devel] [PATCH v2 5/5] IOMMU: iommu_snoop is x86-only
2020-03-09 10:39 [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Jan Beulich
` (3 preceding siblings ...)
2020-03-09 10:44 ` [Xen-devel] [PATCH v2 4/5] IOMMU: iommu_qinval " Jan Beulich
@ 2020-03-09 10:44 ` Jan Beulich
2020-03-10 10:49 ` Paul Durrant
2020-03-10 1:19 ` [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Tian, Kevin
5 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2020-03-09 10:44 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Wilk, George Dunlap, Andrew Cooper, Paul Durrant,
Ian Jackson
In fact it's VT-d specific, but we don't have a way yet to build code
for just one vendor. Provide a #define for the opposite case.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: The option isn't HVM-specific, after all.
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -32,7 +32,6 @@ bool_t __read_mostly iommu_enabled;
bool_t __read_mostly force_iommu;
bool_t __read_mostly iommu_verbose;
bool __read_mostly iommu_quarantine = true;
-bool_t __read_mostly iommu_snoop = 1;
bool_t __read_mostly iommu_crash_disable;
static bool __hwdom_initdata iommu_hwdom_none;
@@ -79,8 +78,10 @@ static int __init parse_iommu_param(cons
#endif
else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
iommu_verbose = val;
+#ifndef iommu_snoop
else if ( (val = parse_boolean("snoop", s, ss)) >= 0 )
iommu_snoop = val;
+#endif
#ifndef iommu_intremap
else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
@@ -488,7 +489,9 @@ int __init iommu_setup(void)
printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
if ( !iommu_enabled )
{
- iommu_snoop = 0;
+#ifndef iommu_snoop
+ iommu_snoop = false;
+#endif
iommu_hwdom_passthrough = false;
iommu_hwdom_strict = false;
}
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -52,6 +52,9 @@ bool __read_mostly untrusted_msi;
bool __read_mostly iommu_igfx = true;
bool __read_mostly iommu_qinval = true;
+#ifndef iommu_snoop
+bool __read_mostly iommu_snoop = true;
+#endif
int nr_iommus;
@@ -2288,8 +2291,10 @@ static int __init vtd_setup(void)
cap_sps_2mb(iommu->cap) ? ", 2MB" : "",
cap_sps_1gb(iommu->cap) ? ", 1GB" : "");
+#ifndef iommu_snoop
if ( iommu_snoop && !ecap_snp_ctl(iommu->ecap) )
- iommu_snoop = 0;
+ iommu_snoop = false;
+#endif
if ( iommu_hwdom_passthrough && !ecap_pass_thru(iommu->ecap) )
iommu_hwdom_passthrough = false;
@@ -2331,7 +2336,9 @@ static int __init vtd_setup(void)
}
#define P(p,s) printk("Intel VT-d %s %senabled.\n", s, (p)? "" : "not ")
+#ifndef iommu_snoop
P(iommu_snoop, "Snoop Control");
+#endif
P(iommu_hwdom_passthrough, "Dom0 DMA Passthrough");
P(iommu_qinval, "Queued Invalidation");
P(iommu_intremap, "Interrupt Remapping");
@@ -2351,7 +2358,9 @@ static int __init vtd_setup(void)
error:
iommu_enabled = 0;
- iommu_snoop = 0;
+#ifndef iommu_snoop
+ iommu_snoop = false;
+#endif
iommu_hwdom_passthrough = false;
iommu_qinval = 0;
iommu_intremap = iommu_intremap_off;
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -54,7 +54,6 @@ static inline bool_t dfn_eq(dfn_t x, dfn
extern bool_t iommu_enable, iommu_enabled;
extern bool force_iommu, iommu_quarantine, iommu_verbose;
-extern bool_t iommu_snoop;
#ifdef CONFIG_X86
extern enum __packed iommu_intremap {
@@ -70,9 +69,10 @@ extern enum __packed iommu_intremap {
iommu_intremap_restricted,
iommu_intremap_full,
} iommu_intremap;
-extern bool iommu_igfx, iommu_qinval;
+extern bool iommu_igfx, iommu_qinval, iommu_snoop;
#else
# define iommu_intremap false
+# define iommu_snoop false
#endif
#if defined(CONFIG_X86) && defined(CONFIG_HVM)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only Jan Beulich
@ 2020-03-10 1:13 ` Tian, Kevin
2020-03-10 9:39 ` Jan Beulich
2020-03-10 10:54 ` Paul Durrant
1 sibling, 1 reply; 17+ messages in thread
From: Tian, Kevin @ 2020-03-10 1:13 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Wilk,
George Dunlap, Andrew Cooper, Paul Durrant, Ian Jackson
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, March 9, 2020 6:43 PM
>
> Provide a #define for all other cases.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Refine doc adjustment.
>
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1309,6 +1309,8 @@ boolean (e.g. `iommu=no`) can override t
> This option depends on `intremap`, and is disabled by default due to some
> corner cases in the implementation which have yet to be resolved.
>
> + This option is only valid on x86, and only builds of Xen with HVM support.
> +
> * The `crash-disable` boolean controls disabling IOMMU functionality
> (DMAR/IR/QI)
> before switching to a crash kernel. This option is inactive by default and
> is for compatibility with older kdump kernels only. Modern kernels copy
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -43,14 +43,6 @@ bool __read_mostly iommu_hwdom_passthrou
> bool __hwdom_initdata iommu_hwdom_inclusive;
> int8_t __hwdom_initdata iommu_hwdom_reserved = -1;
>
> -/*
> - * In the current implementation of VT-d posted interrupts, in some extreme
> - * cases, the per cpu list which saves the blocked vCPU will be very long,
> - * and this will affect the interrupt latency, so let this feature off by
> - * default until we find a good solution to resolve it.
> - */
> -bool_t __read_mostly iommu_intpost;
> -
> #ifndef iommu_hap_pt_share
> bool __read_mostly iommu_hap_pt_share = true;
> #endif
> @@ -93,8 +85,10 @@ static int __init parse_iommu_param(cons
> else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
> iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
> #endif
> +#ifndef iommu_intpost
> else if ( (val = parse_boolean("intpost", s, ss)) >= 0 )
> iommu_intpost = val;
> +#endif
> #ifdef CONFIG_KEXEC
> else if ( (val = parse_boolean("crash-disable", s, ss)) >= 0 )
> iommu_crash_disable = val;
> @@ -486,8 +480,10 @@ int __init iommu_setup(void)
> panic("Couldn't enable %s and iommu=required/force\n",
> !iommu_enabled ? "IOMMU" : "Interrupt Remapping");
>
> +#ifndef iommu_intpost
> if ( !iommu_intremap )
> iommu_intpost = 0;
> +#endif
>
> printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
> if ( !iommu_enabled )
> @@ -563,10 +559,13 @@ void iommu_crash_shutdown(void)
>
> if ( iommu_enabled )
> iommu_get_ops()->crash_shutdown();
> - iommu_enabled = iommu_intpost = 0;
> + iommu_enabled = false;
> #ifndef iommu_intremap
> iommu_intremap = iommu_intremap_off;
> #endif
> +#ifndef iommu_intpost
> + iommu_intpost = false;
> +#endif
> }
>
> int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2297,13 +2297,15 @@ static int __init vtd_setup(void)
> if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) )
> iommu_intremap = iommu_intremap_off;
>
> +#ifndef iommu_intpost
> /*
> * We cannot use posted interrupt if X86_FEATURE_CX16 is
> * not supported, since we count on this feature to
> * atomically update 16-byte IRTE in posted format.
> */
> if ( !cap_intr_post(iommu->cap) || !iommu_intremap || !cpu_has_cx16 )
> - iommu_intpost = 0;
> + iommu_intpost = false;
> +#endif
>
> if ( !vtd_ept_page_compatible(iommu) )
> clear_iommu_hap_pt_share();
> @@ -2330,7 +2332,9 @@ static int __init vtd_setup(void)
> P(iommu_hwdom_passthrough, "Dom0 DMA Passthrough");
> P(iommu_qinval, "Queued Invalidation");
> P(iommu_intremap, "Interrupt Remapping");
> +#ifndef iommu_intpost
> P(iommu_intpost, "Posted Interrupt");
> +#endif
> P(iommu_hap_pt_share, "Shared EPT tables");
> #undef P
>
> @@ -2348,7 +2352,9 @@ static int __init vtd_setup(void)
> iommu_hwdom_passthrough = false;
> iommu_qinval = 0;
> iommu_intremap = iommu_intremap_off;
> - iommu_intpost = 0;
> +#ifndef iommu_intpost
> + iommu_intpost = false;
> +#endif
> return ret;
> }
>
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -29,6 +29,16 @@ struct iommu_ops __read_mostly iommu_ops
>
> enum iommu_intremap __read_mostly iommu_intremap =
> iommu_intremap_full;
>
> +#ifndef iommu_intpost
> +/*
> + * In the current implementation of VT-d posted interrupts, in some
> extreme
> + * cases, the per cpu list which saves the blocked vCPU will be very long,
> + * and this will affect the interrupt latency, so let this feature off by
> + * default until we find a good solution to resolve it.
> + */
Is above comment really VT-d specific? may take this chance to refine
it together.
> +bool __read_mostly iommu_intpost;
> +#endif
> +
> int __init iommu_hardware_setup(void)
> {
> struct IO_APIC_route_entry **ioapic_entries = NULL;
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -54,7 +54,7 @@ static inline bool_t dfn_eq(dfn_t x, dfn
>
> extern bool_t iommu_enable, iommu_enabled;
> extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
> -extern bool_t iommu_snoop, iommu_qinval, iommu_intpost;
> +extern bool_t iommu_snoop, iommu_qinval;
>
> #ifdef CONFIG_X86
> extern enum __packed iommu_intremap {
> @@ -74,6 +74,12 @@ extern enum __packed iommu_intremap {
> # define iommu_intremap false
> #endif
>
> +#if defined(CONFIG_X86) && defined(CONFIG_HVM)
> +extern bool iommu_intpost;
> +#else
> +# define iommu_intpost false
> +#endif
> +
> #if defined(CONFIG_IOMMU_FORCE_PT_SHARE)
> #define iommu_hap_pt_share true
> #elif defined(CONFIG_HVM)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables
2020-03-09 10:39 [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Jan Beulich
` (4 preceding siblings ...)
2020-03-09 10:44 ` [Xen-devel] [PATCH v2 5/5] IOMMU: iommu_snoop " Jan Beulich
@ 2020-03-10 1:19 ` Tian, Kevin
5 siblings, 0 replies; 17+ messages in thread
From: Tian, Kevin @ 2020-03-10 1:19 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Wilk,
George Dunlap, Andrew Cooper, Ian Jackson
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, March 9, 2020 6:40 PM
>
> A number of the command line controlled variables are x86-
> or even x86-HVM-specific. Don't have those variables elsewhere
> in the first place (in some cases replace them by a #define),
> and as a result also don't silently accept such "iommu="
> sub-options which in fact have no effect.
>
> 1: iommu_intremap is x86-only
> 2: iommu_intpost is x86/HVM-only
> 3: iommu_igfx is x86-only
> 4: iommu_qinval is x86-only
> 5: iommu_snoop is x86-only
>
> The series contextually depends on "AMD/IOMMU: without XT,
> x2APIC needs to be forced into physical mode"
>
> v2 addresses review comments, at least as far as agreement was
> reached. See individual patches.
>
Reviewed-by: Kevin Tian <kevin.tian@intel.com> for the whole
series.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
2020-03-10 1:13 ` Tian, Kevin
@ 2020-03-10 9:39 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2020-03-10 9:39 UTC (permalink / raw)
To: Tian, Kevin
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Wilk,
George Dunlap, Andrew Cooper, Paul Durrant, Ian Jackson,
xen-devel
On 10.03.2020 02:13, Tian, Kevin wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Monday, March 9, 2020 6:43 PM
>>
>> --- a/xen/drivers/passthrough/x86/iommu.c
>> +++ b/xen/drivers/passthrough/x86/iommu.c
>> @@ -29,6 +29,16 @@ struct iommu_ops __read_mostly iommu_ops
>>
>> enum iommu_intremap __read_mostly iommu_intremap =
>> iommu_intremap_full;
>>
>> +#ifndef iommu_intpost
>> +/*
>> + * In the current implementation of VT-d posted interrupts, in some
>> extreme
>> + * cases, the per cpu list which saves the blocked vCPU will be very long,
>> + * and this will affect the interrupt latency, so let this feature off by
>> + * default until we find a good solution to resolve it.
>> + */
>
> Is above comment really VT-d specific? may take this chance to refine
> it together.
At the moment it is, as we still don't have any AMD side counterpart
code.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only
2020-03-09 10:42 ` [Xen-devel] [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only Jan Beulich
@ 2020-03-10 10:44 ` Paul Durrant
0 siblings, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2020-03-10 10:44 UTC (permalink / raw)
To: 'Jan Beulich', xen-devel
Cc: 'Kevin Tian', 'Stefano Stabellini',
'Julien Grall', 'Wei Liu', 'Konrad Wilk',
'George Dunlap', 'Andrew Cooper',
'Ian Jackson'
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 09 March 2020 10:43
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien@xen.org>; Wei Liu <wl@xen.org>; Konrad Wilk <konrad.wilk@oracle.com>; George Dunlap
> <George.Dunlap@eu.citrix.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Ian Jackson
> <ian.jackson@citrix.com>; Paul Durrant <paul@xen.org>
> Subject: [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only
>
> Provide a #define for other cases; it didn't seem worthwhile to me to
> introduce an IOMMU_INTREMAP Kconfig option at this point.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
> ---
> v2: Refine doc adjustment.
>
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1299,6 +1299,8 @@ boolean (e.g. `iommu=no`) can override t
> generation of IOMMUs only supported DMA remapping, and Interrupt Remapping
> appeared in the second generation.
>
> + This option is only valid on x86.
> +
> * The `intpost` boolean controls the Posted Interrupt sub-feature. In
> combination with APIC acceleration (VT-x APICV, SVM AVIC), the IOMMU can
> be configured to deliver interrupts from assigned PCI devices directly
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -35,7 +35,6 @@ bool __read_mostly iommu_quarantine = tr
> bool_t __read_mostly iommu_igfx = 1;
> bool_t __read_mostly iommu_snoop = 1;
> bool_t __read_mostly iommu_qinval = 1;
> -enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
> bool_t __read_mostly iommu_crash_disable;
>
> static bool __hwdom_initdata iommu_hwdom_none;
> @@ -90,8 +89,10 @@ static int __init parse_iommu_param(cons
> iommu_snoop = val;
> else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
> iommu_qinval = val;
> +#ifndef iommu_intremap
> else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
> iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
> +#endif
> else if ( (val = parse_boolean("intpost", s, ss)) >= 0 )
> iommu_intpost = val;
> #ifdef CONFIG_KEXEC
> @@ -474,8 +475,11 @@ int __init iommu_setup(void)
> rc = iommu_hardware_setup();
> iommu_enabled = (rc == 0);
> }
> +
> +#ifndef iommu_intremap
> if ( !iommu_enabled )
> iommu_intremap = iommu_intremap_off;
> +#endif
>
> if ( (force_iommu && !iommu_enabled) ||
> (force_intremap && !iommu_intremap) )
> @@ -500,7 +504,9 @@ int __init iommu_setup(void)
> printk(" - Dom0 mode: %s\n",
> iommu_hwdom_passthrough ? "Passthrough" :
> iommu_hwdom_strict ? "Strict" : "Relaxed");
> +#ifndef iommu_intremap
> printk("Interrupt remapping %sabled\n", iommu_intremap ? "en" : "dis");
> +#endif
> tasklet_init(&iommu_pt_cleanup_tasklet, iommu_free_pagetables, NULL);
> }
>
> @@ -558,7 +564,9 @@ void iommu_crash_shutdown(void)
> if ( iommu_enabled )
> iommu_get_ops()->crash_shutdown();
> iommu_enabled = iommu_intpost = 0;
> +#ifndef iommu_intremap
> iommu_intremap = iommu_intremap_off;
> +#endif
> }
>
> int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -27,6 +27,8 @@
> const struct iommu_init_ops *__initdata iommu_init_ops;
> struct iommu_ops __read_mostly iommu_ops;
>
> +enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
> +
> int __init iommu_hardware_setup(void)
> {
> struct IO_APIC_route_entry **ioapic_entries = NULL;
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -55,21 +55,24 @@ static inline bool_t dfn_eq(dfn_t x, dfn
> extern bool_t iommu_enable, iommu_enabled;
> extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
> extern bool_t iommu_snoop, iommu_qinval, iommu_intpost;
> +
> +#ifdef CONFIG_X86
> extern enum __packed iommu_intremap {
> /*
> * In order to allow traditional boolean uses of the iommu_intremap
> * variable, the "off" value has to come first (yielding a value of zero).
> */
> iommu_intremap_off,
> -#ifdef CONFIG_X86
> /*
> * Interrupt remapping enabled, but only able to generate interrupts
> * with an 8-bit APIC ID.
> */
> iommu_intremap_restricted,
> -#endif
> iommu_intremap_full,
> } iommu_intremap;
> +#else
> +# define iommu_intremap false
> +#endif
>
> #if defined(CONFIG_IOMMU_FORCE_PT_SHARE)
> #define iommu_hap_pt_share true
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 3/5] IOMMU: iommu_igfx is x86-only
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 3/5] IOMMU: iommu_igfx is x86-only Jan Beulich
@ 2020-03-10 10:46 ` Paul Durrant
0 siblings, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2020-03-10 10:46 UTC (permalink / raw)
To: 'Jan Beulich', xen-devel
Cc: 'Kevin Tian', 'Stefano Stabellini',
'Julien Grall', 'Wei Liu', 'Konrad Wilk',
'George Dunlap', 'Andrew Cooper',
'Ian Jackson'
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 09 March 2020 10:44
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien@xen.org>; Wei Liu <wl@xen.org>; Konrad Wilk <konrad.wilk@oracle.com>; George Dunlap
> <George.Dunlap@eu.citrix.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Ian Jackson
> <ian.jackson@citrix.com>; Paul Durrant <paul@xen.org>
> Subject: [PATCH v2 3/5] IOMMU: iommu_igfx is x86-only
>
> In fact it's VT-d specific, but we don't have a way yet to build code
> for just one vendor.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
>
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -32,7 +32,6 @@ bool_t __read_mostly iommu_enabled;
> bool_t __read_mostly force_iommu;
> bool_t __read_mostly iommu_verbose;
> bool __read_mostly iommu_quarantine = true;
> -bool_t __read_mostly iommu_igfx = 1;
> bool_t __read_mostly iommu_snoop = 1;
> bool_t __read_mostly iommu_qinval = 1;
> bool_t __read_mostly iommu_crash_disable;
> @@ -73,8 +72,10 @@ static int __init parse_iommu_param(cons
> force_iommu = val;
> else if ( (val = parse_boolean("quarantine", s, ss)) >= 0 )
> iommu_quarantine = val;
> +#ifdef CONFIG_X86
> else if ( (val = parse_boolean("igfx", s, ss)) >= 0 )
> iommu_igfx = val;
> +#endif
> else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
> iommu_verbose = val;
> else if ( (val = parse_boolean("snoop", s, ss)) >= 0 )
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -50,6 +50,8 @@ struct mapped_rmrr {
> /* Possible unfiltered LAPIC/MSI messages from untrusted sources? */
> bool __read_mostly untrusted_msi;
>
> +bool __read_mostly iommu_igfx = true;
> +
> int nr_iommus;
>
> static struct tasklet vtd_fault_tasklet;
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -53,7 +53,7 @@ static inline bool_t dfn_eq(dfn_t x, dfn
> }
>
> extern bool_t iommu_enable, iommu_enabled;
> -extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
> +extern bool force_iommu, iommu_quarantine, iommu_verbose;
> extern bool_t iommu_snoop, iommu_qinval;
>
> #ifdef CONFIG_X86
> @@ -70,6 +70,7 @@ extern enum __packed iommu_intremap {
> iommu_intremap_restricted,
> iommu_intremap_full,
> } iommu_intremap;
> +extern bool iommu_igfx;
> #else
> # define iommu_intremap false
> #endif
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 4/5] IOMMU: iommu_qinval is x86-only
2020-03-09 10:44 ` [Xen-devel] [PATCH v2 4/5] IOMMU: iommu_qinval " Jan Beulich
@ 2020-03-10 10:47 ` Paul Durrant
0 siblings, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2020-03-10 10:47 UTC (permalink / raw)
To: 'Jan Beulich', xen-devel
Cc: 'Kevin Tian', 'Stefano Stabellini',
'Julien Grall', 'Wei Liu', 'Konrad Wilk',
'George Dunlap', 'Andrew Cooper',
'Ian Jackson'
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 09 March 2020 10:44
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien@xen.org>; Wei Liu <wl@xen.org>; Konrad Wilk <konrad.wilk@oracle.com>; George Dunlap
> <George.Dunlap@eu.citrix.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Ian Jackson
> <ian.jackson@citrix.com>; Paul Durrant <paul@xen.org>
> Subject: [PATCH v2 4/5] IOMMU: iommu_qinval is x86-only
>
> In fact it's VT-d specific, but we don't have a way yet to build code
> for just one vendor.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
>
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -33,7 +33,6 @@ bool_t __read_mostly force_iommu;
> bool_t __read_mostly iommu_verbose;
> bool __read_mostly iommu_quarantine = true;
> bool_t __read_mostly iommu_snoop = 1;
> -bool_t __read_mostly iommu_qinval = 1;
> bool_t __read_mostly iommu_crash_disable;
>
> static bool __hwdom_initdata iommu_hwdom_none;
> @@ -75,13 +74,13 @@ static int __init parse_iommu_param(cons
> #ifdef CONFIG_X86
> else if ( (val = parse_boolean("igfx", s, ss)) >= 0 )
> iommu_igfx = val;
> + else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
> + iommu_qinval = val;
> #endif
> else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
> iommu_verbose = val;
> else if ( (val = parse_boolean("snoop", s, ss)) >= 0 )
> iommu_snoop = val;
> - else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
> - iommu_qinval = val;
> #ifndef iommu_intremap
> else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
> iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -51,6 +51,7 @@ struct mapped_rmrr {
> bool __read_mostly untrusted_msi;
>
> bool __read_mostly iommu_igfx = true;
> +bool __read_mostly iommu_qinval = true;
>
> int nr_iommus;
>
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -54,7 +54,7 @@ static inline bool_t dfn_eq(dfn_t x, dfn
>
> extern bool_t iommu_enable, iommu_enabled;
> extern bool force_iommu, iommu_quarantine, iommu_verbose;
> -extern bool_t iommu_snoop, iommu_qinval;
> +extern bool_t iommu_snoop;
>
> #ifdef CONFIG_X86
> extern enum __packed iommu_intremap {
> @@ -70,7 +70,7 @@ extern enum __packed iommu_intremap {
> iommu_intremap_restricted,
> iommu_intremap_full,
> } iommu_intremap;
> -extern bool iommu_igfx;
> +extern bool iommu_igfx, iommu_qinval;
> #else
> # define iommu_intremap false
> #endif
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 5/5] IOMMU: iommu_snoop is x86-only
2020-03-09 10:44 ` [Xen-devel] [PATCH v2 5/5] IOMMU: iommu_snoop " Jan Beulich
@ 2020-03-10 10:49 ` Paul Durrant
0 siblings, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2020-03-10 10:49 UTC (permalink / raw)
To: 'Jan Beulich', xen-devel
Cc: 'Kevin Tian', 'Stefano Stabellini',
'Julien Grall', 'Wei Liu', 'Konrad Wilk',
'George Dunlap', 'Andrew Cooper',
'Ian Jackson'
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 09 March 2020 10:45
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien@xen.org>; Wei Liu <wl@xen.org>; Konrad Wilk <konrad.wilk@oracle.com>; George Dunlap
> <George.Dunlap@eu.citrix.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Ian Jackson
> <ian.jackson@citrix.com>; Paul Durrant <paul@xen.org>
> Subject: [PATCH v2 5/5] IOMMU: iommu_snoop is x86-only
>
> In fact it's VT-d specific, but we don't have a way yet to build code
> for just one vendor. Provide a #define for the opposite case.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
> ---
> v2: The option isn't HVM-specific, after all.
>
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -32,7 +32,6 @@ bool_t __read_mostly iommu_enabled;
> bool_t __read_mostly force_iommu;
> bool_t __read_mostly iommu_verbose;
> bool __read_mostly iommu_quarantine = true;
> -bool_t __read_mostly iommu_snoop = 1;
> bool_t __read_mostly iommu_crash_disable;
>
> static bool __hwdom_initdata iommu_hwdom_none;
> @@ -79,8 +78,10 @@ static int __init parse_iommu_param(cons
> #endif
> else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
> iommu_verbose = val;
> +#ifndef iommu_snoop
> else if ( (val = parse_boolean("snoop", s, ss)) >= 0 )
> iommu_snoop = val;
> +#endif
> #ifndef iommu_intremap
> else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
> iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
> @@ -488,7 +489,9 @@ int __init iommu_setup(void)
> printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
> if ( !iommu_enabled )
> {
> - iommu_snoop = 0;
> +#ifndef iommu_snoop
> + iommu_snoop = false;
> +#endif
> iommu_hwdom_passthrough = false;
> iommu_hwdom_strict = false;
> }
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -52,6 +52,9 @@ bool __read_mostly untrusted_msi;
>
> bool __read_mostly iommu_igfx = true;
> bool __read_mostly iommu_qinval = true;
> +#ifndef iommu_snoop
> +bool __read_mostly iommu_snoop = true;
> +#endif
>
> int nr_iommus;
>
> @@ -2288,8 +2291,10 @@ static int __init vtd_setup(void)
> cap_sps_2mb(iommu->cap) ? ", 2MB" : "",
> cap_sps_1gb(iommu->cap) ? ", 1GB" : "");
>
> +#ifndef iommu_snoop
> if ( iommu_snoop && !ecap_snp_ctl(iommu->ecap) )
> - iommu_snoop = 0;
> + iommu_snoop = false;
> +#endif
>
> if ( iommu_hwdom_passthrough && !ecap_pass_thru(iommu->ecap) )
> iommu_hwdom_passthrough = false;
> @@ -2331,7 +2336,9 @@ static int __init vtd_setup(void)
> }
>
> #define P(p,s) printk("Intel VT-d %s %senabled.\n", s, (p)? "" : "not ")
> +#ifndef iommu_snoop
> P(iommu_snoop, "Snoop Control");
> +#endif
> P(iommu_hwdom_passthrough, "Dom0 DMA Passthrough");
> P(iommu_qinval, "Queued Invalidation");
> P(iommu_intremap, "Interrupt Remapping");
> @@ -2351,7 +2358,9 @@ static int __init vtd_setup(void)
>
> error:
> iommu_enabled = 0;
> - iommu_snoop = 0;
> +#ifndef iommu_snoop
> + iommu_snoop = false;
> +#endif
> iommu_hwdom_passthrough = false;
> iommu_qinval = 0;
> iommu_intremap = iommu_intremap_off;
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -54,7 +54,6 @@ static inline bool_t dfn_eq(dfn_t x, dfn
>
> extern bool_t iommu_enable, iommu_enabled;
> extern bool force_iommu, iommu_quarantine, iommu_verbose;
> -extern bool_t iommu_snoop;
>
> #ifdef CONFIG_X86
> extern enum __packed iommu_intremap {
> @@ -70,9 +69,10 @@ extern enum __packed iommu_intremap {
> iommu_intremap_restricted,
> iommu_intremap_full,
> } iommu_intremap;
> -extern bool iommu_igfx, iommu_qinval;
> +extern bool iommu_igfx, iommu_qinval, iommu_snoop;
> #else
> # define iommu_intremap false
> +# define iommu_snoop false
> #endif
>
> #if defined(CONFIG_X86) && defined(CONFIG_HVM)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only Jan Beulich
2020-03-10 1:13 ` Tian, Kevin
@ 2020-03-10 10:54 ` Paul Durrant
2020-03-10 11:02 ` Jan Beulich
1 sibling, 1 reply; 17+ messages in thread
From: Paul Durrant @ 2020-03-10 10:54 UTC (permalink / raw)
To: 'Jan Beulich', xen-devel
Cc: 'Kevin Tian', 'Stefano Stabellini',
'Julien Grall', 'Wei Liu', 'Konrad Wilk',
'George Dunlap', 'Andrew Cooper',
'Ian Jackson'
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 09 March 2020 10:43
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien@xen.org>; Wei Liu <wl@xen.org>; Konrad Wilk <konrad.wilk@oracle.com>; George Dunlap
> <George.Dunlap@eu.citrix.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Ian Jackson
> <ian.jackson@citrix.com>; Paul Durrant <paul@xen.org>
> Subject: [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
>
> Provide a #define for all other cases.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Refine doc adjustment.
>
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1309,6 +1309,8 @@ boolean (e.g. `iommu=no`) can override t
> This option depends on `intremap`, and is disabled by default due to some
> corner cases in the implementation which have yet to be resolved.
>
> + This option is only valid on x86, and only builds of Xen with HVM support.
> +
> * The `crash-disable` boolean controls disabling IOMMU functionality (DMAR/IR/QI)
> before switching to a crash kernel. This option is inactive by default and
> is for compatibility with older kdump kernels only. Modern kernels copy
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -43,14 +43,6 @@ bool __read_mostly iommu_hwdom_passthrou
> bool __hwdom_initdata iommu_hwdom_inclusive;
> int8_t __hwdom_initdata iommu_hwdom_reserved = -1;
>
> -/*
> - * In the current implementation of VT-d posted interrupts, in some extreme
> - * cases, the per cpu list which saves the blocked vCPU will be very long,
> - * and this will affect the interrupt latency, so let this feature off by
> - * default until we find a good solution to resolve it.
> - */
> -bool_t __read_mostly iommu_intpost;
> -
> #ifndef iommu_hap_pt_share
> bool __read_mostly iommu_hap_pt_share = true;
> #endif
> @@ -93,8 +85,10 @@ static int __init parse_iommu_param(cons
> else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
> iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
> #endif
> +#ifndef iommu_intpost
> else if ( (val = parse_boolean("intpost", s, ss)) >= 0 )
> iommu_intpost = val;
> +#endif
> #ifdef CONFIG_KEXEC
> else if ( (val = parse_boolean("crash-disable", s, ss)) >= 0 )
> iommu_crash_disable = val;
> @@ -486,8 +480,10 @@ int __init iommu_setup(void)
> panic("Couldn't enable %s and iommu=required/force\n",
> !iommu_enabled ? "IOMMU" : "Interrupt Remapping");
>
> +#ifndef iommu_intpost
> if ( !iommu_intremap )
> iommu_intpost = 0;
Nit: 0 -> false
With that fixed...
Reviewed-by: Paul Durrant <paul@xen.org>
> +#endif
>
> printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
> if ( !iommu_enabled )
> @@ -563,10 +559,13 @@ void iommu_crash_shutdown(void)
>
> if ( iommu_enabled )
> iommu_get_ops()->crash_shutdown();
> - iommu_enabled = iommu_intpost = 0;
> + iommu_enabled = false;
> #ifndef iommu_intremap
> iommu_intremap = iommu_intremap_off;
> #endif
> +#ifndef iommu_intpost
> + iommu_intpost = false;
> +#endif
> }
>
> int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2297,13 +2297,15 @@ static int __init vtd_setup(void)
> if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) )
> iommu_intremap = iommu_intremap_off;
>
> +#ifndef iommu_intpost
> /*
> * We cannot use posted interrupt if X86_FEATURE_CX16 is
> * not supported, since we count on this feature to
> * atomically update 16-byte IRTE in posted format.
> */
> if ( !cap_intr_post(iommu->cap) || !iommu_intremap || !cpu_has_cx16 )
> - iommu_intpost = 0;
> + iommu_intpost = false;
> +#endif
>
> if ( !vtd_ept_page_compatible(iommu) )
> clear_iommu_hap_pt_share();
> @@ -2330,7 +2332,9 @@ static int __init vtd_setup(void)
> P(iommu_hwdom_passthrough, "Dom0 DMA Passthrough");
> P(iommu_qinval, "Queued Invalidation");
> P(iommu_intremap, "Interrupt Remapping");
> +#ifndef iommu_intpost
> P(iommu_intpost, "Posted Interrupt");
> +#endif
> P(iommu_hap_pt_share, "Shared EPT tables");
> #undef P
>
> @@ -2348,7 +2352,9 @@ static int __init vtd_setup(void)
> iommu_hwdom_passthrough = false;
> iommu_qinval = 0;
> iommu_intremap = iommu_intremap_off;
> - iommu_intpost = 0;
> +#ifndef iommu_intpost
> + iommu_intpost = false;
> +#endif
> return ret;
> }
>
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -29,6 +29,16 @@ struct iommu_ops __read_mostly iommu_ops
>
> enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
>
> +#ifndef iommu_intpost
> +/*
> + * In the current implementation of VT-d posted interrupts, in some extreme
> + * cases, the per cpu list which saves the blocked vCPU will be very long,
> + * and this will affect the interrupt latency, so let this feature off by
> + * default until we find a good solution to resolve it.
> + */
> +bool __read_mostly iommu_intpost;
> +#endif
> +
> int __init iommu_hardware_setup(void)
> {
> struct IO_APIC_route_entry **ioapic_entries = NULL;
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -54,7 +54,7 @@ static inline bool_t dfn_eq(dfn_t x, dfn
>
> extern bool_t iommu_enable, iommu_enabled;
> extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
> -extern bool_t iommu_snoop, iommu_qinval, iommu_intpost;
> +extern bool_t iommu_snoop, iommu_qinval;
>
> #ifdef CONFIG_X86
> extern enum __packed iommu_intremap {
> @@ -74,6 +74,12 @@ extern enum __packed iommu_intremap {
> # define iommu_intremap false
> #endif
>
> +#if defined(CONFIG_X86) && defined(CONFIG_HVM)
> +extern bool iommu_intpost;
> +#else
> +# define iommu_intpost false
> +#endif
> +
> #if defined(CONFIG_IOMMU_FORCE_PT_SHARE)
> #define iommu_hap_pt_share true
> #elif defined(CONFIG_HVM)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
2020-03-10 10:54 ` Paul Durrant
@ 2020-03-10 11:02 ` Jan Beulich
2020-03-10 12:20 ` Paul Durrant
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2020-03-10 11:02 UTC (permalink / raw)
To: paul
Cc: 'Kevin Tian', 'Stefano Stabellini',
'Julien Grall', 'Wei Liu', 'Konrad Wilk',
'George Dunlap', 'Andrew Cooper',
'Ian Jackson',
xen-devel
On 10.03.2020 11:54, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 09 March 2020 10:43
>>
>> @@ -486,8 +480,10 @@ int __init iommu_setup(void)
>> panic("Couldn't enable %s and iommu=required/force\n",
>> !iommu_enabled ? "IOMMU" : "Interrupt Remapping");
>>
>> +#ifndef iommu_intpost
>> if ( !iommu_intremap )
>> iommu_intpost = 0;
>
> Nit: 0 -> false
Hmm, I'm not touching this line, and the goal of the patch isn't
to (also) switch _all_ assignments to the variable. There is at
least one more (in vmcs.c), and doing the adjustment here (as
being not otherwise motivated, e.g. because of touching the line
anyway) would then, for consistency, seem to call for correcting
that other instance too. This, however, would seem too unrelated
a change to make here for my taste. Hence ...
> With that fixed...
>
> Reviewed-by: Paul Durrant <paul@xen.org>
... please clarify whether I may leave the line untouched.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
2020-03-10 11:02 ` Jan Beulich
@ 2020-03-10 12:20 ` Paul Durrant
2020-03-10 13:47 ` Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Paul Durrant @ 2020-03-10 12:20 UTC (permalink / raw)
To: 'Jan Beulich'
Cc: 'Kevin Tian', 'Stefano Stabellini',
'Julien Grall', 'Wei Liu', 'Konrad Wilk',
'George Dunlap', 'Andrew Cooper',
'Ian Jackson',
xen-devel
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 10 March 2020 11:02
> To: paul@xen.org
> Cc: xen-devel@lists.xenproject.org; 'Kevin Tian' <kevin.tian@intel.com>; 'Stefano Stabellini'
> <sstabellini@kernel.org>; 'Julien Grall' <julien@xen.org>; 'Wei Liu' <wl@xen.org>; 'Konrad Wilk'
> <konrad.wilk@oracle.com>; 'George Dunlap' <George.Dunlap@eu.citrix.com>; 'Andrew Cooper'
> <andrew.cooper3@citrix.com>; 'Ian Jackson' <ian.jackson@citrix.com>
> Subject: Re: [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
>
> On 10.03.2020 11:54, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: 09 March 2020 10:43
> >>
> >> @@ -486,8 +480,10 @@ int __init iommu_setup(void)
> >> panic("Couldn't enable %s and iommu=required/force\n",
> >> !iommu_enabled ? "IOMMU" : "Interrupt Remapping");
> >>
> >> +#ifndef iommu_intpost
> >> if ( !iommu_intremap )
> >> iommu_intpost = 0;
> >
> > Nit: 0 -> false
>
> Hmm, I'm not touching this line, and the goal of the patch isn't
> to (also) switch _all_ assignments to the variable.
Yes, but it is in context and you normally ask for fix-ups where they are in context. In this case it’s a pretty trivial addition to the patch.
> There is at
> least one more (in vmcs.c), and doing the adjustment here (as
> being not otherwise motivated, e.g. because of touching the line
> anyway) would then, for consistency, seem to call for correcting
> that other instance too.
No, I'm not suggesting a wholesale conversion (although I'm not against it)... just tidying as we go.
> This, however, would seem too unrelated
> a change to make here for my taste. Hence ...
>
> > With that fixed...
> >
> > Reviewed-by: Paul Durrant <paul@xen.org>
>
> ... please clarify whether I may leave the line untouched.
Since it's in context I'd prefer it fixed, but I'm not going to insist so you can keep the R-b.
Paul
>
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
2020-03-10 12:20 ` Paul Durrant
@ 2020-03-10 13:47 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2020-03-10 13:47 UTC (permalink / raw)
To: paul
Cc: 'Kevin Tian', 'Stefano Stabellini',
'Julien Grall', 'Wei Liu', 'Konrad Wilk',
'George Dunlap', 'Andrew Cooper',
'Ian Jackson',
xen-devel
On 10.03.2020 13:20, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 10 March 2020 11:02
>> To: paul@xen.org
>> Cc: xen-devel@lists.xenproject.org; 'Kevin Tian' <kevin.tian@intel.com>; 'Stefano Stabellini'
>> <sstabellini@kernel.org>; 'Julien Grall' <julien@xen.org>; 'Wei Liu' <wl@xen.org>; 'Konrad Wilk'
>> <konrad.wilk@oracle.com>; 'George Dunlap' <George.Dunlap@eu.citrix.com>; 'Andrew Cooper'
>> <andrew.cooper3@citrix.com>; 'Ian Jackson' <ian.jackson@citrix.com>
>> Subject: Re: [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
>>
>> On 10.03.2020 11:54, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: 09 March 2020 10:43
>>>>
>>>> @@ -486,8 +480,10 @@ int __init iommu_setup(void)
>>>> panic("Couldn't enable %s and iommu=required/force\n",
>>>> !iommu_enabled ? "IOMMU" : "Interrupt Remapping");
>>>>
>>>> +#ifndef iommu_intpost
>>>> if ( !iommu_intremap )
>>>> iommu_intpost = 0;
>>>
>>> Nit: 0 -> false
>>
>> Hmm, I'm not touching this line, and the goal of the patch isn't
>> to (also) switch _all_ assignments to the variable.
>
> Yes, but it is in context and you normally ask for fix-ups where
> they are in context. In this case it’s a pretty trivial addition to the patch.
Hmm, I now notice that in another place I already do such an in-context
adjustment, so I'll do so here too. Normally I (try to) restrict such
(requests) to lines touched anyway.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-03-10 13:48 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 10:39 [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables Jan Beulich
2020-03-09 10:42 ` [Xen-devel] [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only Jan Beulich
2020-03-10 10:44 ` Paul Durrant
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only Jan Beulich
2020-03-10 1:13 ` Tian, Kevin
2020-03-10 9:39 ` Jan Beulich
2020-03-10 10:54 ` Paul Durrant
2020-03-10 11:02 ` Jan Beulich
2020-03-10 12:20 ` Paul Durrant
2020-03-10 13:47 ` Jan Beulich
2020-03-09 10:43 ` [Xen-devel] [PATCH v2 3/5] IOMMU: iommu_igfx is x86-only Jan Beulich
2020-03-10 10:46 ` Paul Durrant
2020-03-09 10:44 ` [Xen-devel] [PATCH v2 4/5] IOMMU: iommu_qinval " Jan Beulich
2020-03-10 10:47 ` Paul Durrant
2020-03-09 10:44 ` [Xen-devel] [PATCH v2 5/5] IOMMU: iommu_snoop " Jan Beulich
2020-03-10 10:49 ` Paul Durrant
2020-03-10 1:19 ` [Xen-devel] [PATCH v2 0/5] IOMMU: restrict visibility/scope if certain variables 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.