diff mbox

[v4] VT-d PI: disable VT-d PI when CPU-side PI isn't enabled

Message ID 1497342023-13680-1-git-send-email-chao.gao@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chao Gao June 13, 2017, 8:20 a.m. UTC
From the context calling pi_desc_init(), we can conclude the current
implementation of VT-d PI depends on CPU-side PI. If we enable VT-d PI
and disable CPU-side PI by disabling APICv explicitly in xen boot
command line, we would get an assertion failure.

This patch clears iommu_intpost once finding CPU-side PI won't be enabled.
It is safe for this is done before this flag starts taking effect. Also
take this chance to remove the useless check of "acknowledge interrupt on
exit", which is a minimal requirement which has been checked earlier.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
v4:
- Remove APICv stuff
- Remove a useless check of "acknowledge interrupt on exit"

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

---
 xen/arch/x86/hvm/vmx/vmcs.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Jan Beulich June 13, 2017, 8:29 a.m. UTC | #1
>>> On 13.06.17 at 10:20, <chao.gao@intel.com> wrote:
> From the context calling pi_desc_init(), we can conclude the current
> implementation of VT-d PI depends on CPU-side PI. If we enable VT-d PI
> and disable CPU-side PI by disabling APICv explicitly in xen boot
> command line, we would get an assertion failure.
> 
> This patch clears iommu_intpost once finding CPU-side PI won't be enabled.
> It is safe for this is done before this flag starts taking effect. Also
> take this chance to remove the useless check of "acknowledge interrupt on
> exit", which is a minimal requirement which has been checked earlier.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
> v4:
> - Remove APICv stuff
> - Remove a useless check of "acknowledge interrupt on exit"
> 
> 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
> 
> ---
>  xen/arch/x86/hvm/vmx/vmcs.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 8103b20..58f89df 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -345,11 +345,19 @@ static int vmx_init_vmcs_config(void)
>  
>      /*
>       * "Process posted interrupt" can be set only when "virtual-interrupt
> -     * delivery" and "acknowledge interrupt on exit" is set
> +     * delivery" and "acknowledge interrupt on exit" is set. For the latter
> +     * is a minimal requirement, only check the former, which is optional.
>       */
> -    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
> -          || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) )
> -        _vmx_pin_based_exec_control  &= ~ PIN_BASED_POSTED_INTERRUPT;
> +    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) )
> +        _vmx_pin_based_exec_control &= ~ PIN_BASED_POSTED_INTERRUPT;
> +
> +    if ( iommu_intpost
> +          && !(_vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) )

With the placement of the && here corrected (belongs on the
earlier line), which can be done while committing of no other
need for v5 arises,
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Jan Beulich Aug. 3, 2017, 9:44 a.m. UTC | #2
>>> On 13.06.17 at 10:29, <JBeulich@suse.com> wrote:
>>>> On 13.06.17 at 10:20, <chao.gao@intel.com> wrote:
>> From the context calling pi_desc_init(), we can conclude the current
>> implementation of VT-d PI depends on CPU-side PI. If we enable VT-d PI
>> and disable CPU-side PI by disabling APICv explicitly in xen boot
>> command line, we would get an assertion failure.
>> 
>> This patch clears iommu_intpost once finding CPU-side PI won't be enabled.
>> It is safe for this is done before this flag starts taking effect. Also
>> take this chance to remove the useless check of "acknowledge interrupt on
>> exit", which is a minimal requirement which has been checked earlier.
>> 
>> Signed-off-by: Chao Gao <chao.gao@intel.com>

Kevin, Jun?

>> ---
>> v4:
>> - Remove APICv stuff
>> - Remove a useless check of "acknowledge interrupt on exit"
>> 
>> 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
>> 
>> ---
>>  xen/arch/x86/hvm/vmx/vmcs.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>> 
>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
>> index 8103b20..58f89df 100644
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -345,11 +345,19 @@ static int vmx_init_vmcs_config(void)
>>  
>>      /*
>>       * "Process posted interrupt" can be set only when "virtual-interrupt
>> -     * delivery" and "acknowledge interrupt on exit" is set
>> +     * delivery" and "acknowledge interrupt on exit" is set. For the latter
>> +     * is a minimal requirement, only check the former, which is optional.
>>       */
>> -    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
>> -          || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) )
>> -        _vmx_pin_based_exec_control  &= ~ PIN_BASED_POSTED_INTERRUPT;
>> +    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) )
>> +        _vmx_pin_based_exec_control &= ~ PIN_BASED_POSTED_INTERRUPT;
>> +
>> +    if ( iommu_intpost
>> +          && !(_vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) )
> 
> With the placement of the && here corrected (belongs on the
> earlier line), which can be done while committing of no other
> need for v5 arises,
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Jan
Tian, Kevin Aug. 4, 2017, 2:14 a.m. UTC | #3
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Thursday, August 3, 2017 5:44 PM
> 
> >>> On 13.06.17 at 10:29, <JBeulich@suse.com> wrote:
> >>>> On 13.06.17 at 10:20, <chao.gao@intel.com> wrote:
> >> From the context calling pi_desc_init(), we can conclude the current
> >> implementation of VT-d PI depends on CPU-side PI. If we enable VT-d PI
> >> and disable CPU-side PI by disabling APICv explicitly in xen boot
> >> command line, we would get an assertion failure.
> >>
> >> This patch clears iommu_intpost once finding CPU-side PI won't be
> enabled.
> >> It is safe for this is done before this flag starts taking effect. Also
> >> take this chance to remove the useless check of "acknowledge interrupt
> on
> >> exit", which is a minimal requirement which has been checked earlier.
> >>
> >> Signed-off-by: Chao Gao <chao.gao@intel.com>
> 
> Kevin, Jun?
> 

thanks for remind.

Acked-by: Kevin Tian <kevin.tian@intel.com>
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 8103b20..58f89df 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -345,11 +345,19 @@  static int vmx_init_vmcs_config(void)
 
     /*
      * "Process posted interrupt" can be set only when "virtual-interrupt
-     * delivery" and "acknowledge interrupt on exit" is set
+     * delivery" and "acknowledge interrupt on exit" is set. For the latter
+     * is a minimal requirement, only check the former, which is optional.
      */
-    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
-          || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) )
-        _vmx_pin_based_exec_control  &= ~ PIN_BASED_POSTED_INTERRUPT;
+    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) )
+        _vmx_pin_based_exec_control &= ~ PIN_BASED_POSTED_INTERRUPT;
+
+    if ( iommu_intpost
+          && !(_vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) )
+    {
+        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 )