[PATCH v3] VT-d PI: disable VT-d PI when APICv is disabled

[PATCH v3] VT-d PI: disable VT-d PI when APICv is disabled

[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.

With this patch, using 'xl dmesg' we can see the following output on a
machine with CPU-side PI disabled but IOMMU-side PI enabled:
...
(XEN) [   10.154585] Intel VT-d Snoop Control enabled.
(XEN) [   10.160565] Intel VT-d Dom0 DMA Passthrough not enabled.
(XEN) [   10.167514] Intel VT-d Queued Invalidation enabled.
(XEN) [   10.174024] Intel VT-d Interrupt Remapping enabled.
(XEN) [   10.180538] Intel VT-d Posted Interrupt enabled.
(XEN) [   10.186797] Intel VT-d Shared EPT tables enabled.
(XEN) [   10.205834] I/O virtualisation enabled
(XEN) [   10.211212]  - Dom0 mode: Relaxed
(XEN) [   10.216159] Interrupt remapping enabled
(XEN) [   10.221619] nr_sockets: 5
(XEN) [   10.225874] Enabled directed EOI with ioapic_ack_old on!
(XEN) [   10.233809] ENABLING IO-APIC IRQs
(XEN) [   10.238757]  -> Using old ACK method
(XEN) [   10.244476] ..TIMER: vector=0xF0 apic1=0 pin1=2 apic2=-1 pin2=-1
(XEN) [   10.454461] TSC deadline timer enabled
(XEN) [   10.506000] Defaulting to alternative key handling; send 'A' to switch to normal mode.
(XEN) [   10.516577] mwait-idle: MWAIT substates: 0x2020
(XEN) [   10.518052] mwait-idle: v0.4.1 model 0x55
(XEN) [   10.519520] mwait-idle: lapic_timer_reliable_states 0xffffffff
(XEN) [   10.521016] Intel VT-d Posted Interrupt is disabled for CPU-side Posted Interrupt is not enabled
(XEN) [   10.523872] VMX: Supported advanced features:
(XEN) [   10.525341]  - APIC MMIO access virtualisation
(XEN) [   10.526810]  - APIC TPR shadow
(XEN) [   10.528265]  - Extended Page Tables (EPT)
(XEN) [   10.529731]  - Virtual-Processor Identifiers (VPID)
(XEN) [   10.531207]  - Virtual NMI
(XEN) [   10.532657]  - MSR direct-access bitmap
...

At first, we announce VT-d PI is enabled, but then disabled it. Hope
this is acceptable.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
v3:
- check relevant bit directly other than checking the apicv option
- add sample of 'xl dmesg'

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         | 7 +++++++
 2 files changed, 11 insertions(+), 2 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..9293814 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -351,6 +351,13 @@ static int vmx_init_vmcs_config(void)
           || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) )
         _vmx_pin_based_exec_control  &= ~ PIN_BASED_POSTED_INTERRUPT;
 
+    if ( iommu_intpost && !cpu_has_vmx_posted_intr_processing )
+    {
+        printk("Intel VT-d Posted Interrupt is disabled for CPU-side Posted "
+               "Interrupt is not enabled\n");
+        iommu_intpost = 0;
+    }
+
     /* The IA32_VMX_VMFUNC MSR exists only when VMFUNC is available */
     if ( _vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS )
     {
-- 
1.8.3.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3] VT-d PI: disable VT-d PI when APICv is disabled

[Jan Beulich]

>>> On 09.06.17 at 08:22, <chao.gao@intel.com> wrote:
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -351,6 +351,13 @@ static int vmx_init_vmcs_config(void)
>            || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) )
>          _vmx_pin_based_exec_control  &= ~ PIN_BASED_POSTED_INTERRUPT;
>  
> +    if ( iommu_intpost && !cpu_has_vmx_posted_intr_processing )
> +    {
> +        printk("Intel VT-d Posted Interrupt is disabled for CPU-side Posted "
> +               "Interrupt is not enabled\n");
> +        iommu_intpost = 0;
> +    }

So simply clearing iommu_intpost here indeed looks to be fine (at
least for all current uses of the flag). However, previously you
had a dependency on APIC-V being enabled, and there not longer
is such a dependency with the check above -
posted_intr_processing is being forced to off only when
!vmx_virtual_intr_delivery or ACK_INTR_ON_EXIT not being set
(the latter btw is strange as a check, as that feature is being
requested as "minimum", not "optional").

And then I have a flow question here:
cpu_has_vmx_posted_intr_processing expands to a reference to
vmx_pin_based_exec_control, yet that variable is being written
only later in the function, so it would seem to me that now you
turn off iommu_intpost unconditionally. Am I overlooking anything?
Otherwise I think you need to use _vmx_pin_based_exec_control
here instead, just like the code visible in context does.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3] VT-d PI: disable VT-d PI when APICv is disabled

[Chao Gao]

On Fri, Jun 09, 2017 at 02:43:30AM -0600, Jan Beulich wrote:
>>>> On 09.06.17 at 08:22, <chao.gao@intel.com> wrote:
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -351,6 +351,13 @@ static int vmx_init_vmcs_config(void)
>>            || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) )
>>          _vmx_pin_based_exec_control  &= ~ PIN_BASED_POSTED_INTERRUPT;
>>  
>> +    if ( iommu_intpost && !cpu_has_vmx_posted_intr_processing )
>> +    {
>> +        printk("Intel VT-d Posted Interrupt is disabled for CPU-side Posted "
>> +               "Interrupt is not enabled\n");
>> +        iommu_intpost = 0;
>> +    }
>
>So simply clearing iommu_intpost here indeed looks to be fine (at
>least for all current uses of the flag). However, previously you
>had a dependency on APIC-V being enabled, and there not longer
>is such a dependency with the check above -
>posted_intr_processing is being forced to off only when
>!vmx_virtual_intr_delivery or ACK_INTR_ON_EXIT not being set
>(the latter btw is strange as a check, as that feature is being
>requested as "minimum", not "optional").

Yes. the relevant description is improper now. I will remove all of
them and think more about the ACK_INTR_ON_EXIT. If it can be
removed, I will take this change to do this.

>
>And then I have a flow question here:
>cpu_has_vmx_posted_intr_processing expands to a reference to
>vmx_pin_based_exec_control, yet that variable is being written
>only later in the function, so it would seem to me that now you
>turn off iommu_intpost unconditionally. Am I overlooking anything?
>Otherwise I think you need to use _vmx_pin_based_exec_control
>here instead, just like the code visible in context does.

Agree. It should be. Thanks you. I forgot to test with 'apicv=true'.
I only tested this patch with 'apicv=false'.

Thanks
chao

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel