From: Sean Christopherson <seanjc@google.com>
To: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Cc: Maxim Levitsky <mlevitsk@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/8] KVM: SVM: Re-inject INT3/INTO instead of retrying the instruction
Date: Mon, 4 Apr 2022 19:54:29 +0000 [thread overview]
Message-ID: <YktM9bnq5HaTMKkV@google.com> (raw)
In-Reply-To: <YktIGHM86jHkzGdF@google.com>
On Mon, Apr 04, 2022, Sean Christopherson wrote:
> On Mon, Apr 04, 2022, Maciej S. Szmigiero wrote:
> > > > > index 47e7427d0395..a770a1c7ddd2 100644
> > > > > --- a/arch/x86/kvm/svm/svm.h
> > > > > +++ b/arch/x86/kvm/svm/svm.h
> > > > > @@ -230,8 +230,8 @@ struct vcpu_svm {
> > > > > bool nmi_singlestep;
> > > > > u64 nmi_singlestep_guest_rflags;
> > > > > - unsigned int3_injected;
> > > > > - unsigned long int3_rip;
> > > > > + unsigned soft_int_injected;
> > > > > + unsigned long soft_int_linear_rip;
> > > > > /* optional nested SVM features that are enabled for this guest */
> > > > > bool nrips_enabled : 1;
> > > >
> > > >
> > > > I mostly agree with this patch, but think that it doesn't address the
> > > > original issue that Maciej wanted to address:
> > > >
> > > > Suppose that there is *no* instruction in L2 code which caused the software
> > > > exception, but rather L1 set arbitrary next_rip, and set EVENTINJ to software
> > > > exception with some vector, and that injection got interrupted.
> > > >
> > > > I don't think that this code will support this.
> > >
> > > Argh, you're right. Maciej's selftest injects without an instruction, but it doesn't
> > > configure the scenario where that injection fails due to an exception+VM-Exit that
> > > isn't intercepted by L1 and is handled by L0. The event_inj test gets the coverage
> > > for the latter, but always has a backing instruction.
> >
> > Still reviewing the whole patch set, but want to clear this point quickly:
> > The selftest does have an implicit intervening NPF (handled by L0) while
> > injecting the first L1 -> L2 event.
>
> I'll do some debug to figure out why the test passes for me. I'm guessing I either
> got lucky, e.g. IDT was faulted in already, or I screwed up and the test doesn't
> actually pass.
Well that was easy. My code is indeed flawed and skips the wrong instruction,
the skipped instruction just so happens to be a (spurious?) adjustment of RSP. The
L2 guest function never runs to completion and so the "bad" RSP is never consumed.
KVM: incomplete injection for L2, vector 32 @ 401c70. next_rip = 0
KVM: injecting for L2, vector 0 @ 401c70. next_rip = 401c74
0000000000401c70 <l2_guest_code>:
401c70: 48 83 ec 08 sub $0x8,%rsp
401c74: 83 3d 75 a7 0e 00 01 cmpl $0x1,0xea775(%rip) # 4ec3f0 <int_fired>
401c7b: 74 1e je 401c9b <l2_guest_code+0x2b>
401c7d: 45 31 c0 xor %r8d,%r8d
401c80: b9 32 00 00 00 mov $0x32,%ecx
401c85: ba 90 40 4b 00 mov $0x4b4090,%edx
401c8a: 31 c0 xor %eax,%eax
401c8c: be 02 00 00 00 mov $0x2,%esi
401c91: bf 02 00 00 00 mov $0x2,%edi
401c96: e8 05 ae 00 00 call 40caa0 <ucall>
401c9b: 0f 01 d9 vmmcall
401c9e: 0f 0b ud2
401ca0: 83 3d 4d a7 0e 00 01 cmpl $0x1,0xea74d(%rip) # 4ec3f4 <bp_fired>
401ca7: 74 1e je 401cc7 <l2_guest_code+0x57>
401ca9: 45 31 c0 xor %r8d,%r8d
401cac: b9 36 00 00 00 mov $0x36,%ecx
401cb1: ba b8 40 4b 00 mov $0x4b40b8,%edx
401cb6: 31 c0 xor %eax,%eax
401cb8: be 02 00 00 00 mov $0x2,%esi
401cbd: bf 02 00 00 00 mov $0x2,%edi
401cc2: e8 d9 ad 00 00 call 40caa0 <ucall>
401cc7: f4 hlt
401cc8: 48 83 c4 08 add $0x8,%rsp
401ccc: c3 ret
401ccd: 0f 1f 00 nopl (%rax)
I don't see why the compiler is creating room for a single variable, but it doesn't
really matter, the easiest way to detect this bug is to assert that the return RIP
in the INT 0x20 handler points at l2_guest_code, e.g. this fails:
diff --git a/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c b/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c
index d39be5d885c1..257aa2280b5c 100644
--- a/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c
+++ b/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c
@@ -40,9 +40,13 @@ static void guest_bp_handler(struct ex_regs *regs)
}
static unsigned int int_fired;
+static void l2_guest_code(void);
+
static void guest_int_handler(struct ex_regs *regs)
{
int_fired++;
+ GUEST_ASSERT_2(regs->rip == (unsigned long)l2_guest_code,
+ regs->rip, (unsigned long)l2_guest_code);
}
static void l2_guest_code(void)
next prev parent reply other threads:[~2022-04-04 21:23 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-02 1:08 [PATCH 0/8] KVM: SVM: Fix soft int/ex re-injection Sean Christopherson
2022-04-02 1:08 ` [PATCH 1/8] KVM: nSVM: Sync next_rip field from vmcb12 to vmcb02 Sean Christopherson
2022-04-04 9:54 ` Maxim Levitsky
2022-04-04 16:50 ` Maciej S. Szmigiero
2022-04-04 17:21 ` Sean Christopherson
2022-04-04 17:45 ` Maciej S. Szmigiero
2022-04-20 15:00 ` Paolo Bonzini
2022-04-20 15:05 ` Maciej S. Szmigiero
2022-04-20 16:15 ` Sean Christopherson
2022-04-20 16:33 ` Paolo Bonzini
2022-04-20 16:44 ` Sean Christopherson
2022-04-02 1:08 ` [PATCH 2/8] KVM: SVM: Downgrade BUG_ON() to WARN_ON() in svm_inject_irq() Sean Christopherson
2022-04-02 1:08 ` [PATCH 3/8] KVM: SVM: Unwind "speculative" RIP advancement if INTn injection "fails" Sean Christopherson
2022-04-04 10:03 ` Maxim Levitsky
2022-04-20 15:01 ` Paolo Bonzini
2022-04-02 1:08 ` [PATCH 4/8] KVM: SVM: Stuff next_rip on emualted INT3 injection if NRIPS is supported Sean Christopherson
2022-04-04 12:00 ` Maxim Levitsky
2022-04-02 1:09 ` [PATCH 5/8] KVM: SVM: Re-inject INT3/INTO instead of retrying the instruction Sean Christopherson
2022-04-04 12:12 ` Maxim Levitsky
2022-04-04 16:49 ` Sean Christopherson
2022-04-04 16:53 ` Maciej S. Szmigiero
2022-04-04 19:33 ` Sean Christopherson
2022-04-04 19:50 ` Maciej S. Szmigiero
2022-04-04 19:54 ` Sean Christopherson [this message]
2022-04-04 20:46 ` Maciej S. Szmigiero
2022-04-04 20:44 ` Maciej S. Szmigiero
2022-04-06 1:48 ` Sean Christopherson
2022-04-06 13:13 ` Maciej S. Szmigiero
2022-04-06 17:10 ` Sean Christopherson
2022-04-06 19:08 ` Maciej S. Szmigiero
2022-04-06 19:48 ` Sean Christopherson
2022-04-06 20:30 ` Maciej S. Szmigiero
2022-04-06 20:52 ` Sean Christopherson
2022-04-06 22:34 ` Maciej S. Szmigiero
2022-04-06 23:03 ` Sean Christopherson
2022-04-07 15:32 ` Maciej S. Szmigiero
2022-04-02 1:09 ` [PATCH 6/8] KVM: SVM: Re-inject INTn instead of retrying the insn on "failure" Sean Christopherson
2022-04-04 17:14 ` Sean Christopherson
2022-04-04 20:27 ` Maciej S. Szmigiero
2022-04-02 1:09 ` [PATCH 7/8] KVM: x86: Trace re-injected exceptions Sean Christopherson
2022-04-04 12:14 ` Maxim Levitsky
2022-04-04 16:14 ` Sean Christopherson
2022-04-02 1:09 ` [PATCH 8/8] KVM: selftests: nSVM: Add svm_nested_soft_inject_test Sean Christopherson
2022-04-04 12:27 ` Maxim Levitsky
2022-04-04 16:59 ` Maciej S. Szmigiero
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=YktM9bnq5HaTMKkV@google.com \
--to=seanjc@google.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mail@maciej.szmigiero.name \
--cc=mlevitsk@redhat.com \
--cc=pbonzini@redhat.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).