[kvm-unit-tests PATCH] nVMX: Fix testing failure for canonical checks when forced emulation is not available

[kvm-unit-tests PATCH] nVMX: Fix testing failure for canonical checks when forced emulation is not available

[Chenyi Qiang]

Use the _safe() variant instead of _fep_safe() to avoid failure if the
forced emulated is not available.

Fixes: 05fbb364b5b2 ("nVMX: add a test for canonical checks of various host state vmcs12 fields")
Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
---
 x86/vmx_tests.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 2f178227..01a15b7c 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -10881,12 +10881,11 @@ static int set_host_value(u64 vmcs_field, u64 value)
 	case HOST_BASE_GDTR:
 		sgdt(&dt_ptr);
 		dt_ptr.base = value;
-		lgdt(&dt_ptr);
-		return lgdt_fep_safe(&dt_ptr);
+		return lgdt_safe(&dt_ptr);
 	case HOST_BASE_IDTR:
 		sidt(&dt_ptr);
 		dt_ptr.base = value;
-		return lidt_fep_safe(&dt_ptr);
+		return lidt_safe(&dt_ptr);
 	case HOST_BASE_TR:
 		/* Set the base and clear the busy bit */
 		set_gdt_entry(FIRST_SPARE_SEL, value, 0x200, 0x89, 0);
-- 
2.43.5

Re: [kvm-unit-tests PATCH] nVMX: Fix testing failure for canonical checks when forced emulation is not available

[Sean Christopherson]

On Fri, May 23, 2025, Chenyi Qiang wrote:
> Use the _safe() variant instead of _fep_safe() to avoid failure if the
> forced emulated is not available.
> 
> Fixes: 05fbb364b5b2 ("nVMX: add a test for canonical checks of various host state vmcs12 fields")
> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> ---
>  x86/vmx_tests.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 2f178227..01a15b7c 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -10881,12 +10881,11 @@ static int set_host_value(u64 vmcs_field, u64 value)
>  	case HOST_BASE_GDTR:
>  		sgdt(&dt_ptr);
>  		dt_ptr.base = value;
> -		lgdt(&dt_ptr);
> -		return lgdt_fep_safe(&dt_ptr);
> +		return lgdt_safe(&dt_ptr);
>  	case HOST_BASE_IDTR:
>  		sidt(&dt_ptr);
>  		dt_ptr.base = value;
> -		return lidt_fep_safe(&dt_ptr);
> +		return lidt_safe(&dt_ptr);

Hmm, the main purpose of this particular test is to verify KVM's emulation of the
canonical checks, so it probably makes sense to force emulation when possible.

It's not the most performant approach, but how about this?

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 2f178227..fe53e989 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -10881,12 +10881,13 @@ static int set_host_value(u64 vmcs_field, u64 value)
        case HOST_BASE_GDTR:
                sgdt(&dt_ptr);
                dt_ptr.base = value;
-               lgdt(&dt_ptr);
-               return lgdt_fep_safe(&dt_ptr);
+               return is_fep_available() ? lgdt_fep_safe(&dt_ptr) :
+                                           lgdt_safe(&dt_ptr);
        case HOST_BASE_IDTR:
                sidt(&dt_ptr);
                dt_ptr.base = value;
-               return lidt_fep_safe(&dt_ptr);
+               return is_fep_available() ? lidt_fep_safe(&dt_ptr) :
+                                           lidt_safe(&dt_ptr);
        case HOST_BASE_TR:
                /* Set the base and clear the busy bit */
                set_gdt_entry(FIRST_SPARE_SEL, value, 0x200, 0x89, 0);

Re: [kvm-unit-tests PATCH] nVMX: Fix testing failure for canonical checks when forced emulation is not available

[Chenyi Qiang]

On 6/4/2025 7:20 AM, Sean Christopherson wrote:
> On Fri, May 23, 2025, Chenyi Qiang wrote:
>> Use the _safe() variant instead of _fep_safe() to avoid failure if the
>> forced emulated is not available.
>>
>> Fixes: 05fbb364b5b2 ("nVMX: add a test for canonical checks of various host state vmcs12 fields")
>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>> ---
>>  x86/vmx_tests.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> index 2f178227..01a15b7c 100644
>> --- a/x86/vmx_tests.c
>> +++ b/x86/vmx_tests.c
>> @@ -10881,12 +10881,11 @@ static int set_host_value(u64 vmcs_field, u64 value)
>>  	case HOST_BASE_GDTR:
>>  		sgdt(&dt_ptr);
>>  		dt_ptr.base = value;
>> -		lgdt(&dt_ptr);
>> -		return lgdt_fep_safe(&dt_ptr);
>> +		return lgdt_safe(&dt_ptr);
>>  	case HOST_BASE_IDTR:
>>  		sidt(&dt_ptr);
>>  		dt_ptr.base = value;
>> -		return lidt_fep_safe(&dt_ptr);
>> +		return lidt_safe(&dt_ptr);
> 
> Hmm, the main purpose of this particular test is to verify KVM's emulation of the
> canonical checks, so it probably makes sense to force emulation when possible.
> 
> It's not the most performant approach, but how about this?
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 2f178227..fe53e989 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -10881,12 +10881,13 @@ static int set_host_value(u64 vmcs_field, u64 value)
>         case HOST_BASE_GDTR:
>                 sgdt(&dt_ptr);
>                 dt_ptr.base = value;
> -               lgdt(&dt_ptr);
> -               return lgdt_fep_safe(&dt_ptr);
> +               return is_fep_available() ? lgdt_fep_safe(&dt_ptr) :
> +                                           lgdt_safe(&dt_ptr);
>         case HOST_BASE_IDTR:
>                 sidt(&dt_ptr);
>                 dt_ptr.base = value;
> -               return lidt_fep_safe(&dt_ptr);
> +               return is_fep_available() ? lidt_fep_safe(&dt_ptr) :
> +                                           lidt_safe(&dt_ptr);
>         case HOST_BASE_TR:
>                 /* Set the base and clear the busy bit */
>                 set_gdt_entry(FIRST_SPARE_SEL, value, 0x200, 0x89, 0);

The call of is_fep_available() itself will trigger the #UD exception:

Unhandled cpu exception 6 #UD at ip 000000000040efb5
error_code=0000      rflags=00010097      cs=00000008
rax=0000000000000000 rcx=00000000c0000101 rdx=000000000042d220
rbx=0000000000006c0c
rbp=000000000073bed0 rsi=ff45454545000000 rdi=0000000000000006
 r8=000000000043836e  r9=00000000000003f8 r10=000000000000000d
r11=00000000000071ba
r12=0000000000436daa r13=0000000000006c0c r14=000000000042d220
r15=0000000000420078
cr0=0000000080010031 cr2=ffffffffffffb000 cr3=0000000001007000
cr4=0000000000042020
cr8=0000000000000000
        STACK: @40efb5 40f0e9 40ff56 402039 403f11 4001bd

Maybe the result of is_fep_available() needs to be passed in from main()
function in some way instead of checking it in guest code.

Re: [kvm-unit-tests PATCH] nVMX: Fix testing failure for canonical checks when forced emulation is not available

[Sean Christopherson]

On Wed, Jun 04, 2025, Chenyi Qiang wrote:
> 
> 
> On 6/4/2025 7:20 AM, Sean Christopherson wrote:
> > On Fri, May 23, 2025, Chenyi Qiang wrote:
> >> Use the _safe() variant instead of _fep_safe() to avoid failure if the
> >> forced emulated is not available.
> >>
> >> Fixes: 05fbb364b5b2 ("nVMX: add a test for canonical checks of various host state vmcs12 fields")
> >> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> >> ---
> >>  x86/vmx_tests.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> >> index 2f178227..01a15b7c 100644
> >> --- a/x86/vmx_tests.c
> >> +++ b/x86/vmx_tests.c
> >> @@ -10881,12 +10881,11 @@ static int set_host_value(u64 vmcs_field, u64 value)
> >>  	case HOST_BASE_GDTR:
> >>  		sgdt(&dt_ptr);
> >>  		dt_ptr.base = value;
> >> -		lgdt(&dt_ptr);
> >> -		return lgdt_fep_safe(&dt_ptr);
> >> +		return lgdt_safe(&dt_ptr);
> >>  	case HOST_BASE_IDTR:
> >>  		sidt(&dt_ptr);
> >>  		dt_ptr.base = value;
> >> -		return lidt_fep_safe(&dt_ptr);
> >> +		return lidt_safe(&dt_ptr);
> > 
> > Hmm, the main purpose of this particular test is to verify KVM's emulation of the
> > canonical checks, so it probably makes sense to force emulation when possible.
> > 
> > It's not the most performant approach, but how about this?
> > 
> > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> > index 2f178227..fe53e989 100644
> > --- a/x86/vmx_tests.c
> > +++ b/x86/vmx_tests.c
> > @@ -10881,12 +10881,13 @@ static int set_host_value(u64 vmcs_field, u64 value)
> >         case HOST_BASE_GDTR:
> >                 sgdt(&dt_ptr);
> >                 dt_ptr.base = value;
> > -               lgdt(&dt_ptr);
> > -               return lgdt_fep_safe(&dt_ptr);
> > +               return is_fep_available() ? lgdt_fep_safe(&dt_ptr) :
> > +                                           lgdt_safe(&dt_ptr);
> >         case HOST_BASE_IDTR:
> >                 sidt(&dt_ptr);
> >                 dt_ptr.base = value;
> > -               return lidt_fep_safe(&dt_ptr);
> > +               return is_fep_available() ? lidt_fep_safe(&dt_ptr) :
> > +                                           lidt_safe(&dt_ptr);
> >         case HOST_BASE_TR:
> >                 /* Set the base and clear the busy bit */
> >                 set_gdt_entry(FIRST_SPARE_SEL, value, 0x200, 0x89, 0);
> 
> The call of is_fep_available() itself will trigger the #UD exception:

Huh.  The #UD is expected, but KUT should handle the #UD

Gah, I thought it was working on my end, but it's not.  I just got a triple fault
instead of a nice error report, and didn't notice the return code (was running
manually).

Oh, duh.  Invoking is_fep_available() when restoring the original host value for
the IDT will triple fault due to IDT.base being 0xff55555555000000.

Hmm, that doesn't explain how you managed to get a stack trace though.  Can you
test the series I cc'd you on?  If it still fails, then something entirely
different is going on.

> Unhandled cpu exception 6 #UD at ip 000000000040efb5
> error_code=0000      rflags=00010097      cs=00000008
> rax=0000000000000000 rcx=00000000c0000101 rdx=000000000042d220
> rbx=0000000000006c0c
> rbp=000000000073bed0 rsi=ff45454545000000 rdi=0000000000000006
>  r8=000000000043836e  r9=00000000000003f8 r10=000000000000000d
> r11=00000000000071ba
> r12=0000000000436daa r13=0000000000006c0c r14=000000000042d220
> r15=0000000000420078
> cr0=0000000080010031 cr2=ffffffffffffb000 cr3=0000000001007000
> cr4=0000000000042020
> cr8=0000000000000000
>         STACK: @40efb5 40f0e9 40ff56 402039 403f11 4001bd
> 
> Maybe the result of is_fep_available() needs to be passed in from main()
> function in some way instead of checking it in guest code.

Ya, it's past time we give KUT the same treatment as KVM selftests and cache the
information during test setup.  setup_idt() is the obvious choice.  I already
posted a series (I meant to send this first, but got distracted).

Nit, this isn't guest code per se, because these writes are all in the "host",
i.e. in L1 (which is obviously _a_ guest, but not _the_ guest from these test's
perspective).

Re: [kvm-unit-tests PATCH] nVMX: Fix testing failure for canonical checks when forced emulation is not available

[Chenyi Qiang]

On 6/5/2025 3:41 AM, Sean Christopherson wrote:
> On Wed, Jun 04, 2025, Chenyi Qiang wrote:
>>
>>
>> On 6/4/2025 7:20 AM, Sean Christopherson wrote:
>>> On Fri, May 23, 2025, Chenyi Qiang wrote:
>>>> Use the _safe() variant instead of _fep_safe() to avoid failure if the
>>>> forced emulated is not available.
>>>>
>>>> Fixes: 05fbb364b5b2 ("nVMX: add a test for canonical checks of various host state vmcs12 fields")
>>>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>>>> ---
>>>>  x86/vmx_tests.c | 5 ++---
>>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>>>> index 2f178227..01a15b7c 100644
>>>> --- a/x86/vmx_tests.c
>>>> +++ b/x86/vmx_tests.c
>>>> @@ -10881,12 +10881,11 @@ static int set_host_value(u64 vmcs_field, u64 value)
>>>>  	case HOST_BASE_GDTR:
>>>>  		sgdt(&dt_ptr);
>>>>  		dt_ptr.base = value;
>>>> -		lgdt(&dt_ptr);
>>>> -		return lgdt_fep_safe(&dt_ptr);
>>>> +		return lgdt_safe(&dt_ptr);
>>>>  	case HOST_BASE_IDTR:
>>>>  		sidt(&dt_ptr);
>>>>  		dt_ptr.base = value;
>>>> -		return lidt_fep_safe(&dt_ptr);
>>>> +		return lidt_safe(&dt_ptr);
>>>
>>> Hmm, the main purpose of this particular test is to verify KVM's emulation of the
>>> canonical checks, so it probably makes sense to force emulation when possible.
>>>
>>> It's not the most performant approach, but how about this?
>>>
>>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>>> index 2f178227..fe53e989 100644
>>> --- a/x86/vmx_tests.c
>>> +++ b/x86/vmx_tests.c
>>> @@ -10881,12 +10881,13 @@ static int set_host_value(u64 vmcs_field, u64 value)
>>>         case HOST_BASE_GDTR:
>>>                 sgdt(&dt_ptr);
>>>                 dt_ptr.base = value;
>>> -               lgdt(&dt_ptr);
>>> -               return lgdt_fep_safe(&dt_ptr);
>>> +               return is_fep_available() ? lgdt_fep_safe(&dt_ptr) :
>>> +                                           lgdt_safe(&dt_ptr);
>>>         case HOST_BASE_IDTR:
>>>                 sidt(&dt_ptr);
>>>                 dt_ptr.base = value;
>>> -               return lidt_fep_safe(&dt_ptr);
>>> +               return is_fep_available() ? lidt_fep_safe(&dt_ptr) :
>>> +                                           lidt_safe(&dt_ptr);
>>>         case HOST_BASE_TR:
>>>                 /* Set the base and clear the busy bit */
>>>                 set_gdt_entry(FIRST_SPARE_SEL, value, 0x200, 0x89, 0);
>>
>> The call of is_fep_available() itself will trigger the #UD exception:
> 
> Huh.  The #UD is expected, but KUT should handle the #UD
> 
> Gah, I thought it was working on my end, but it's not.  I just got a triple fault
> instead of a nice error report, and didn't notice the return code (was running
> manually).
> 
> Oh, duh.  Invoking is_fep_available() when restoring the original host value for
> the IDT will triple fault due to IDT.base being 0xff55555555000000.
> 
> Hmm, that doesn't explain how you managed to get a stack trace though.  Can you
> test the series I cc'd you on?  If it still fails, then something entirely
> different is going on.

That series works for me. Now the test can pass on my side.

> 
>> Unhandled cpu exception 6 #UD at ip 000000000040efb5
>> error_code=0000      rflags=00010097      cs=00000008
>> rax=0000000000000000 rcx=00000000c0000101 rdx=000000000042d220
>> rbx=0000000000006c0c
>> rbp=000000000073bed0 rsi=ff45454545000000 rdi=0000000000000006
>>  r8=000000000043836e  r9=00000000000003f8 r10=000000000000000d
>> r11=00000000000071ba
>> r12=0000000000436daa r13=0000000000006c0c r14=000000000042d220
>> r15=0000000000420078
>> cr0=0000000080010031 cr2=ffffffffffffb000 cr3=0000000001007000
>> cr4=0000000000042020
>> cr8=0000000000000000
>>         STACK: @40efb5 40f0e9 40ff56 402039 403f11 4001bd
>>
>> Maybe the result of is_fep_available() needs to be passed in from main()
>> function in some way instead of checking it in guest code.
> 
> Ya, it's past time we give KUT the same treatment as KVM selftests and cache the
> information during test setup.  setup_idt() is the obvious choice.  I already
> posted a series (I meant to send this first, but got distracted).
> 
> Nit, this isn't guest code per se, because these writes are all in the "host",
> i.e. in L1 (which is obviously _a_ guest, but not _the_ guest from these test's
> perspective).

Got it! Thanks for the explanation.