* [PATCH v2] VT-d PI: disable VT-d PI when APICv is disabled
@ 2017-06-07 9:29 Chao Gao
2017-06-07 12:31 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Chao Gao @ 2017-06-07 9:29 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, Jan Beulich, Chao Gao
From the context calling pi_desc_init(), we can conclude the current
implementation of VT-d PI depends on CPU-side PI. If we disable APICv
but enable VT-d PI explicitly in xen boot command line, we would get
an assertion failure.
This patch disables VT-d PI when APICv is disabled and adds some
related description to docs.
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
v2:
- add missing S-o-b
- comments changes
- change bool_t to bool and move 'opt_apicv_enabled' declaration to vmcs.h
---
docs/misc/xen-command-line.markdown | 6 ++++--
xen/arch/x86/hvm/vmx/vmcs.c | 2 +-
xen/drivers/passthrough/vtd/iommu.c | 5 ++++-
xen/include/asm-x86/hvm/vmx/vmcs.h | 2 ++
4 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 44d9985..a5c261d 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -173,7 +173,8 @@ to boot on systems with the following errata:
Permit Xen to use APIC Virtualisation Extensions. This is an optimisation
available as part of VT-x, and allows hardware to take care of the guests APIC
-handling, rather than requiring emulation in Xen.
+handling, rather than requiring emulation in Xen. IOMMU-side interrupt posting
+relies on this extensions, see **iommu** parameter below.
### apic\_verbosity
> `= verbose | debug`
@@ -1001,7 +1002,8 @@ debug hypervisor only).
> Default: `false`
>> Control the use of interrupt posting, which depends on the availability of
->> interrupt remapping.
+>> interrupt remapping and CPU-side interrupt posting, which in turn requires
+>> **APIC Virtualization Extensions** above is not disabled.
> `qinval` (VT-d)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 8103b20..61c51de 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -47,7 +47,7 @@ boolean_param("vpid", opt_vpid_enabled);
static bool_t __read_mostly opt_unrestricted_guest_enabled = 1;
boolean_param("unrestricted_guest", opt_unrestricted_guest_enabled);
-static bool_t __read_mostly opt_apicv_enabled = 1;
+bool __read_mostly opt_apicv_enabled = true;
boolean_param("apicv", opt_apicv_enabled);
/*
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 19328f6..7cdb4e0 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -32,6 +32,7 @@
#include <xen/keyhandler.h>
#include <asm/msi.h>
#include <asm/irq.h>
+#include <asm/hvm/vmx/vmcs.h>
#include <asm/hvm/vmx/vmx.h>
#include <asm/p2m.h>
#include <mach_apic.h>
@@ -2266,8 +2267,10 @@ int __init intel_vtd_setup(void)
* 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.
+ * VT-d PI implementation relies on APICv. Thus, disable
+ * VT-d PI when APICv is disabled.
*/
- if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 )
+ if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 || !opt_apicv_enabled )
iommu_intpost = 0;
if ( !vtd_ept_page_compatible(iommu) )
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 9507bd2..48ad045 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -21,6 +21,8 @@
#include <asm/hvm/io.h>
#include <irq_vectors.h>
+extern bool opt_apicv_enabled;
+
extern void vmcs_dump_vcpu(struct vcpu *v);
extern void setup_vmcs_dump(void);
extern int vmx_cpu_up_prepare(unsigned int cpu);
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] VT-d PI: disable VT-d PI when APICv is disabled
2017-06-07 9:29 [PATCH v2] VT-d PI: disable VT-d PI when APICv is disabled Chao Gao
@ 2017-06-07 12:31 ` Jan Beulich
2017-06-07 15:06 ` Chao Gao
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2017-06-07 12:31 UTC (permalink / raw)
To: Chao Gao; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, xen-devel
>>> On 07.06.17 at 11:29, <chao.gao@intel.com> wrote:
> @@ -2266,8 +2267,10 @@ int __init intel_vtd_setup(void)
> * 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.
> + * VT-d PI implementation relies on APICv. Thus, disable
> + * VT-d PI when APICv is disabled.
> */
> - if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 )
> + if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 || !opt_apicv_enabled )
> iommu_intpost = 0;
I'm sorry for thinking of this only now, but why do you check the
command line option here instead of whether APIC-V is actually
in use? I guess there's not going to be hardware supporting VT-d
PI but not APIC-V (albeit one is a chipset feature aiui and the
other a CPU one, so they are kind of independent), but I could
easily imagine hypervisors supporting them entirely independently.
Hence I think you would be better off checking the relevant
secondary exec bit(s).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] VT-d PI: disable VT-d PI when APICv is disabled
2017-06-07 12:31 ` Jan Beulich
@ 2017-06-07 15:06 ` Chao Gao
2017-06-07 15:34 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Chao Gao @ 2017-06-07 15:06 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, xen-devel
On Wed, Jun 07, 2017 at 06:31:18AM -0600, Jan Beulich wrote:
>>>> On 07.06.17 at 11:29, <chao.gao@intel.com> wrote:
>> @@ -2266,8 +2267,10 @@ int __init intel_vtd_setup(void)
>> * 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.
>> + * VT-d PI implementation relies on APICv. Thus, disable
>> + * VT-d PI when APICv is disabled.
>> */
>> - if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 )
>> + if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 || !opt_apicv_enabled )
>> iommu_intpost = 0;
>
>I'm sorry for thinking of this only now, but why do you check the
>command line option here instead of whether APIC-V is actually
>in use? I guess there's not going to be hardware supporting VT-d
>PI but not APIC-V (albeit one is a chipset feature aiui and the
>other a CPU one, so they are kind of independent), but I could
>easily imagine hypervisors supporting them entirely independently.
>Hence I think you would be better off checking the relevant
>secondary exec bit(s).
Those relevant structure hasn't been initialized here. But your
thought is definitely right. How about clear iommu_intpost in
vmx_init_vmcs_config() when finding cpu-side interrupt posting
can't be enabled?
Thanks
Chao
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] VT-d PI: disable VT-d PI when APICv is disabled
2017-06-07 15:06 ` Chao Gao
@ 2017-06-07 15:34 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2017-06-07 15:34 UTC (permalink / raw)
To: Chao Gao; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, xen-devel
>>> On 07.06.17 at 17:06, <chao.gao@intel.com> wrote:
> On Wed, Jun 07, 2017 at 06:31:18AM -0600, Jan Beulich wrote:
>>>>> On 07.06.17 at 11:29, <chao.gao@intel.com> wrote:
>>> @@ -2266,8 +2267,10 @@ int __init intel_vtd_setup(void)
>>> * 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.
>>> + * VT-d PI implementation relies on APICv. Thus, disable
>>> + * VT-d PI when APICv is disabled.
>>> */
>>> - if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 )
>>> + if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 || !opt_apicv_enabled )
>>> iommu_intpost = 0;
>>
>>I'm sorry for thinking of this only now, but why do you check the
>>command line option here instead of whether APIC-V is actually
>>in use? I guess there's not going to be hardware supporting VT-d
>>PI but not APIC-V (albeit one is a chipset feature aiui and the
>>other a CPU one, so they are kind of independent), but I could
>>easily imagine hypervisors supporting them entirely independently.
>>Hence I think you would be better off checking the relevant
>>secondary exec bit(s).
>
> Those relevant structure hasn't been initialized here. But your
> thought is definitely right. How about clear iommu_intpost in
> vmx_init_vmcs_config() when finding cpu-side interrupt posting
> can't be enabled?
If clearing the flag is all it takes - sure.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-06-07 15:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-07 9:29 [PATCH v2] VT-d PI: disable VT-d PI when APICv is disabled Chao Gao
2017-06-07 12:31 ` Jan Beulich
2017-06-07 15:06 ` Chao Gao
2017-06-07 15:34 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).