Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Jim Mattson <jmattson@google.com>
Cc: Tina Zhang <zhang_wei@open-hieco.net>,
	mlevitsk@redhat.com,  Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
	 zhouyanjing@hygon.cn, linux-kselftest@vger.kernel.org,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] KVM: nSVM: Expose DecodeAssists to L1
Date: Tue, 7 Jul 2026 07:41:23 -0700	[thread overview]
Message-ID: <ak0QE7mP_TeZmzeS@google.com> (raw)
In-Reply-To: <CALMp9eTr0iJ5QgbZJ0YtOZg89XVFSesbyoGGdtkz+f8c60DcDA@mail.gmail.com>

On Mon, Jul 06, 2026, Jim Mattson wrote:
> On Mon, Jul 6, 2026 at 8:21 PM Sean Christopherson <seanjc@google.com> wrote:
> > > For KVM-synthesized/emulated #PF/#NPF nested VM-Exits there is no fresh
> > > hardware GuestInstrBytes state to propagate to VMCB12. So I agree that it
> > > should be simpler to avoid plumbing instruction bytes from the emulator, and
> > > instead fetch the bytes late when constructing the nested VM-Exit for L1.
> > >
> > > I'll try this direction first.
> >
> > I'd rather not, at least not as the primary way.  It's going to raise a different
> > set of issues, e.g. how to behave if reading guest memory fails.  That's probably
> > unavoidable, e.g. if reading the INVPCID descriptor fails, then KVM doesn't even
> > have a pre-decoded instruction to work with, but it should ideally be a last
> > resort.
> >
> > E.g. where the NULL case triggers an on-demand read of guest memory.
> >
> > diff --git arch/x86/kvm/svm/nested.c arch/x86/kvm/svm/nested.c
> > index ba985a02208a..812af3a8d8b9 100644
> > --- arch/x86/kvm/svm/nested.c
> > +++ arch/x86/kvm/svm/nested.c
> > @@ -42,6 +42,7 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu,
> >         struct vcpu_svm *svm = to_svm(vcpu);
> >         struct vmcb *vmcb = svm->vmcb;
> >         u64 fault_stage;
> > +       u8 *insn_bytes;
> >
> >         /*
> >          * For hardware NPF exits, the GUEST_FAULT_STAGE bits are only
> > @@ -68,7 +69,14 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu,
> >                                     (fault->error_code & ~PFERR_GUEST_FAULT_STAGE_MASK);
> >         vmcb->control.exit_info_2 = fault->address;
> >
> > -       nested_svm_vmexit(svm);
> > +       if (from_hardware)
> > +               insn_bytes = svm->nested.vmcb02.ptr->control.insn_bytes;
> > +       else if (vcpu->arch.emulate_ctxt->eip == kvm_rip_read(vcpu))
> > +               insn_bytes = vcpu->arch.emulate_ctxt->fetch.data
> 
> Won't this be shy of the architected 15 bytes if RIP is within 15
> bytes of a page crossing and the instruction ends before the page
> crossing?

Ugh, x86 is so annoying.  Though we need to plumb the number of bytes even when
when pulling from vmcb02.

Expose kvm_fetch_guest_virt() and wire up a wrapper for the emulator, then this?

	int insn_len = 0, len;
	u8 *insn = NULL;
	u64 fault_stage;
	

	/*
	 * For hardware NPF exits, the GUEST_FAULT_STAGE bits are only
	 * available in the hardware exit_info_1, since the guest_mmu
	 * walker doesn't know whether the faulting GPA was a page table
	 * page or final page from L2's perspective.
	 */
	if (from_hardware)
		fault_stage = vmcb->control.exit_info_1 &
			      PFERR_GUEST_FAULT_STAGE_MASK;
	else
		fault_stage = fault->error_code & PFERR_GUEST_FAULT_STAGE_MASK;

	/*
	 * All nested page faults should be annotated as occurring on the
	 * final translation *or* the page walk. Arbitrarily choose "final"
	 * if KVM is buggy and enumerated both or neither.
	 */
	if (WARN_ON_ONCE(hweight64(fault_stage) != 1))
		fault_stage = PFERR_GUEST_FINAL_MASK;

	vmcb->control.exit_code = SVM_EXIT_NPF;
	vmcb->control.exit_info_1 = fault_stage |
				    (fault->error_code & ~PFERR_GUEST_FAULT_STAGE_MASK);
	vmcb->control.exit_info_2 = fault->address;

	if (fault->error_code & PFERR_FETCH_MASK)
		goto synthesize_vmexit;

	if (from_hardware) {
		insn = svm->nested.vmcb02.ptr->control.insn_bytes;
		insn_len = svm->nested.vmcb02.ptr->control.insn_len;
		goto synthesize_vmexit;
	}
	
	if (WARN_ON_ONCE(!ctxt))
		goto synthesize_vmexit;

	insn = ctxt->fetch.data;
	if (ctxt->eip == kvm_rip_read(vcpu))
		insn_len = ctxt->fetch.end - ctxt->fetch.ptr;

	for (len = X86_MAX_INSTRUCTION_LENGTH - insn_len; len >= 0; len--) {
		if (!kvm_fetch_guest_virt(vcpu, kvm_rip_read(vcpu) + insn_len,
					  &insn[insn_len], len, NULL))
			break;
	}

synthesize_vmexit:
	nested_svm_vmexit(svm, insn, insn_len);

> > +       else
> > +               insn_bytes = NULL;
> > +
> > +       nested_svm_vmexit(svm, insn_bytes);
> >  }
> >
> >  static u64 nested_svm_get_tdp_pdptr(struct kvm_vcpu *vcpu, int index)

  parent reply	other threads:[~2026-07-07 14:41 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29 12:52 [PATCH 0/2] KVM: nSVM: Expose DecodeAssists to L1 Tina Zhang
2026-06-29 12:52 ` [PATCH 1/2] KVM: nSVM: Virtualize DecodeAssists for nested guests Tina Zhang
2026-06-29 13:12   ` sashiko-bot
2026-06-30 15:11   ` Jim Mattson
2026-06-29 12:52 ` [PATCH 2/2] KVM: selftests: Add nested SVM DecodeAssists test Tina Zhang
2026-06-30 15:52 ` [PATCH 0/2] KVM: nSVM: Expose DecodeAssists to L1 Jim Mattson
2026-07-06 22:10   ` mlevitsk
2026-07-07  0:01     ` Tina Zhang
2026-07-07  1:32       ` Jim Mattson
2026-07-07  2:44         ` Tina Zhang
2026-07-07  3:21           ` Sean Christopherson
2026-07-07  3:25             ` Jim Mattson
2026-07-07  3:32               ` Jim Mattson
2026-07-07 14:42                 ` Sean Christopherson
2026-07-07 15:19                   ` Jim Mattson
2026-07-07 15:47                     ` Sean Christopherson
2026-07-07 18:56                       ` Jim Mattson
2026-07-07 14:41               ` Sean Christopherson [this message]
2026-07-07 15:24                 ` Jim Mattson
2026-07-07 15:37                   ` Sean Christopherson
2026-07-07 18:23                     ` Jim Mattson
2026-07-07 19:16                       ` Sean Christopherson
2026-07-08 23:21         ` Venkatesh Srinivas
2026-07-07 22:34 ` Yosry Ahmed
2026-07-08  0:53   ` Tina Zhang
2026-07-08 14:04     ` Sean Christopherson
2026-07-08 17:47     ` Yosry Ahmed
2026-07-09  1:00       ` Tina Zhang

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=ak0QE7mP_TeZmzeS@google.com \
    --to=seanjc@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=shuah@kernel.org \
    --cc=zhang_wei@open-hieco.net \
    --cc=zhouyanjing@hygon.cn \
    /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