[PATCH] x86/hvm: Fix the unknown nested vmexit reason 80000021 bug

[PATCH] x86/hvm: Fix the unknown nested vmexit reason 80000021 bug

[Liang Li]

This bug will be trigged when NMI happen in the L2 guest. The current
code handles the NMI incorrectly. According to Intel SDM 31.7.1.2
(Resuming Guest Software after Handling an Exception), If bit 31 of the
IDT-vectoring information fields is set, and the virtual NMIs VM-execution
control is 1, while bits 10:8 in the IDT-vectoring information field is
2, bit 3 in the interruptibility-state field should be cleared to avoid
the next VM entry fail.

Signed-off-by: Liang Li <liang.z.li@intel.com>
---
 xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index e1c55ce..b1f2df8 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2621,7 +2621,8 @@ static void vmx_idtv_reinject(unsigned long idtv_info)
          * Clear NMI-blocking interruptibility info if an NMI delivery faulted.
          * Re-delivery will re-set it (see SDM 3B 25.7.1.2).
          */
-        if ( (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8) )
+        if ( cpu_has_vmx_vnmi && ((idtv_info & INTR_INFO_INTR_TYPE_MASK) ==
+                                 (X86_EVENTTYPE_NMI<<8)) )
         {
             unsigned long intr_info;
 
@@ -2772,8 +2773,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
     hvm_maybe_deassert_evtchn_irq();
 
     __vmread(IDT_VECTORING_INFO, &idtv_info);
-    if ( !nestedhvm_vcpu_in_guestmode(v) && 
-         exit_reason != EXIT_REASON_TASK_SWITCH )
+    if ( exit_reason != EXIT_REASON_TASK_SWITCH )
         vmx_idtv_reinject(idtv_info);
 
     switch ( exit_reason )
-- 
1.9.1

Re: [PATCH] x86/hvm: Fix the unknown nested vmexit reason 80000021 bug

[Andrew Cooper]

On 07/04/15 14:27, Liang Li wrote:
> This bug will be trigged when NMI happen in the L2 guest. The current
> code handles the NMI incorrectly. According to Intel SDM 31.7.1.2
> (Resuming Guest Software after Handling an Exception), If bit 31 of the
> IDT-vectoring information fields is set, and the virtual NMIs VM-execution
> control is 1, while bits 10:8 in the IDT-vectoring information field is
> 2, bit 3 in the interruptibility-state field should be cleared to avoid
> the next VM entry fail.
>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> ---
>  xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index e1c55ce..b1f2df8 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2621,7 +2621,8 @@ static void vmx_idtv_reinject(unsigned long idtv_info)
>           * Clear NMI-blocking interruptibility info if an NMI delivery faulted.
>           * Re-delivery will re-set it (see SDM 3B 25.7.1.2).
>           */
> -        if ( (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8) )
> +        if ( cpu_has_vmx_vnmi && ((idtv_info & INTR_INFO_INTR_TYPE_MASK) ==
> +                                 (X86_EVENTTYPE_NMI<<8)) )

This would be easier to read as

if ( cpu_has_vmx_vnmi &&
     (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8)) )

~Andrew

>          {
>              unsigned long intr_info;
>  
> @@ -2772,8 +2773,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>      hvm_maybe_deassert_evtchn_irq();
>  
>      __vmread(IDT_VECTORING_INFO, &idtv_info);
> -    if ( !nestedhvm_vcpu_in_guestmode(v) && 
> -         exit_reason != EXIT_REASON_TASK_SWITCH )
> +    if ( exit_reason != EXIT_REASON_TASK_SWITCH )
>          vmx_idtv_reinject(idtv_info);
>  
>      switch ( exit_reason )

Re: [PATCH] x86/hvm: Fix the unknown nested vmexit reason 80000021 bug

[Tian, Kevin]

> From: Li, Liang Z
> Sent: Tuesday, April 07, 2015 9:27 PM
> 
> This bug will be trigged when NMI happen in the L2 guest. The current
> code handles the NMI incorrectly. According to Intel SDM 31.7.1.2
> (Resuming Guest Software after Handling an Exception), If bit 31 of the
> IDT-vectoring information fields is set, and the virtual NMIs VM-execution
> control is 1, while bits 10:8 in the IDT-vectoring information field is
> 2, bit 3 in the interruptibility-state field should be cleared to avoid
> the next VM entry fail.
> 
> Signed-off-by: Liang Li <liang.z.li@intel.com>

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

> ---
>  xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index e1c55ce..b1f2df8 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2621,7 +2621,8 @@ static void vmx_idtv_reinject(unsigned long
> idtv_info)
>           * Clear NMI-blocking interruptibility info if an NMI delivery
> faulted.
>           * Re-delivery will re-set it (see SDM 3B 25.7.1.2).
>           */
> -        if ( (idtv_info & INTR_INFO_INTR_TYPE_MASK) ==
> (X86_EVENTTYPE_NMI<<8) )
> +        if ( cpu_has_vmx_vnmi && ((idtv_info &
> INTR_INFO_INTR_TYPE_MASK) ==
> +                                 (X86_EVENTTYPE_NMI<<8)) )
>          {
>              unsigned long intr_info;
> 
> @@ -2772,8 +2773,7 @@ void vmx_vmexit_handler(struct cpu_user_regs
> *regs)
>      hvm_maybe_deassert_evtchn_irq();
> 
>      __vmread(IDT_VECTORING_INFO, &idtv_info);
> -    if ( !nestedhvm_vcpu_in_guestmode(v) &&
> -         exit_reason != EXIT_REASON_TASK_SWITCH )
> +    if ( exit_reason != EXIT_REASON_TASK_SWITCH )
>          vmx_idtv_reinject(idtv_info);
> 
>      switch ( exit_reason )
> --
> 1.9.1

Re: [PATCH] x86/hvm: Fix the unknown nested vmexit reason 80000021 bug

[George Dunlap]

On Tue, Apr 7, 2015 at 5:11 PM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 07/04/15 14:27, Liang Li wrote:
>> This bug will be trigged when NMI happen in the L2 guest. The current
>> code handles the NMI incorrectly. According to Intel SDM 31.7.1.2
>> (Resuming Guest Software after Handling an Exception), If bit 31 of the
>> IDT-vectoring information fields is set, and the virtual NMIs VM-execution
>> control is 1, while bits 10:8 in the IDT-vectoring information field is
>> 2, bit 3 in the interruptibility-state field should be cleared to avoid
>> the next VM entry fail.
>>
>> Signed-off-by: Liang Li <liang.z.li@intel.com>
>> ---
>>  xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index e1c55ce..b1f2df8 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -2621,7 +2621,8 @@ static void vmx_idtv_reinject(unsigned long idtv_info)
>>           * Clear NMI-blocking interruptibility info if an NMI delivery faulted.
>>           * Re-delivery will re-set it (see SDM 3B 25.7.1.2).
>>           */
>> -        if ( (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8) )
>> +        if ( cpu_has_vmx_vnmi && ((idtv_info & INTR_INFO_INTR_TYPE_MASK) ==
>> +                                 (X86_EVENTTYPE_NMI<<8)) )
>
> This would be easier to read as
>
> if ( cpu_has_vmx_vnmi &&
>      (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8)) )

I was going to say something similar, but I think in the past Jan has
said that Liang's original is more in line with the coding style.

But I'll let him comment.

 -George

Re: [PATCH] x86/hvm: Fix the unknown nested vmexit reason 80000021 bug

[Jan Beulich]

>>> On 08.04.15 at 11:20, <George.Dunlap@eu.citrix.com> wrote:
> On Tue, Apr 7, 2015 at 5:11 PM, Andrew Cooper <andrew.cooper3@citrix.com> 
> wrote:
>> On 07/04/15 14:27, Liang Li wrote:
>>> This bug will be trigged when NMI happen in the L2 guest. The current
>>> code handles the NMI incorrectly. According to Intel SDM 31.7.1.2
>>> (Resuming Guest Software after Handling an Exception), If bit 31 of the
>>> IDT-vectoring information fields is set, and the virtual NMIs VM-execution
>>> control is 1, while bits 10:8 in the IDT-vectoring information field is
>>> 2, bit 3 in the interruptibility-state field should be cleared to avoid
>>> the next VM entry fail.
>>>
>>> Signed-off-by: Liang Li <liang.z.li@intel.com>
>>> ---
>>>  xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>>> index e1c55ce..b1f2df8 100644
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -2621,7 +2621,8 @@ static void vmx_idtv_reinject(unsigned long idtv_info)
>>>           * Clear NMI-blocking interruptibility info if an NMI delivery 
> faulted.
>>>           * Re-delivery will re-set it (see SDM 3B 25.7.1.2).
>>>           */
>>> -        if ( (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8) )
>>> +        if ( cpu_has_vmx_vnmi && ((idtv_info & INTR_INFO_INTR_TYPE_MASK) ==
>>> +                                 (X86_EVENTTYPE_NMI<<8)) )
>>
>> This would be easier to read as
>>
>> if ( cpu_has_vmx_vnmi &&
>>      (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8)) )
> 
> I was going to say something similar, but I think in the past Jan has
> said that Liang's original is more in line with the coding style.

No, my complaint here wouldn't be about coding style, but about the
hard-coded 8 - it's not been that long ago that I replaced may of
them, and I'd really like to see it replaced here too. Liang - can you
please submit an incremental change (as the original one got
committed already)? There should be several examples in VMX code
on how the 8 can be avoided.

Jan

Re: [PATCH] x86/hvm: Fix the unknown nested vmexit reason 80000021 bug

[Li, Liang Z]

> >> This would be easier to read as
> >>
> >> if ( cpu_has_vmx_vnmi &&
> >>      (idtv_info & INTR_INFO_INTR_TYPE_MASK) ==
> >> (X86_EVENTTYPE_NMI<<8)) )
> >
> > I was going to say something similar, but I think in the past Jan has
> > said that Liang's original is more in line with the coding style.
> 
> No, my complaint here wouldn't be about coding style, but about the hard-
> coded 8 - it's not been that long ago that I replaced may of them, and I'd
> really like to see it replaced here too. Liang - can you please submit an
> incremental change (as the original one got committed already)? There
> should be several examples in VMX code on how the 8 can be avoided.
> 
> Jan

Of course, with pleasure.

Liang