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@xxxxxxxxx>
> >> ---
> >> 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).
