From: Sean Christopherson <seanjc@google.com>
To: Chenyi Qiang <chenyi.qiang@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Maxim Levitsky <mlevitsk@redhat.com>,
kvm@vger.kernel.org
Subject: Re: [kvm-unit-tests PATCH] nVMX: Fix testing failure for canonical checks when forced emulation is not available
Date: Wed, 4 Jun 2025 12:41:55 -0700 [thread overview]
Message-ID: <aEChg7jMQKG3Zm6-@google.com> (raw)
In-Reply-To: <917b478e-14f3-437f-a748-0fdf423e9db7@intel.com>
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).
next prev parent reply other threads:[~2025-06-04 19:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-23 9:08 [kvm-unit-tests PATCH] nVMX: Fix testing failure for canonical checks when forced emulation is not available Chenyi Qiang
2025-06-03 23:20 ` Sean Christopherson
2025-06-04 2:15 ` Chenyi Qiang
2025-06-04 19:41 ` Sean Christopherson [this message]
2025-06-05 1:16 ` Chenyi Qiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aEChg7jMQKG3Zm6-@google.com \
--to=seanjc@google.com \
--cc=chenyi.qiang@intel.com \
--cc=kvm@vger.kernel.org \
--cc=mlevitsk@redhat.com \
--cc=pbonzini@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.