Re: [PATCH v5 6/8] KVM: nSVM: Fetch missing DecodeAssist bytes for synthesized #NPF/#PF

[Sean Christopherson <seanjc@google.com>]

On Sun, Sep 06, 2026, Jim Mattson wrote:
> On Sat, Sep 5, 2026 at 11:40 PM Tina Zhang <zhang_wei@open-hieco.net> wrote:
> > On 9/5/2026 8:39 AM, Jim Mattson wrote:
> > > On Mon, Aug 24, 2026 at 5:40 AM Tina Zhang <zhang_wei@open-hieco.net> wrote:
> > >>
> > >> For a synthesized #NPF, the emulator fetch cache is not guaranteed to
> > >> contain the full architected 15-byte DecodeAssist window, e.g. it may
> > >> contain only the bytes needed to decode the instruction.
> > >>
> > >> Keep preparation of synthesized state limited to capturing a matching
> > >> emulator fetch cache for #NPF.  When constructing VMCB12, copy those bytes
> > >> and fetch any missing tail through L2 guest page tables.  If no emulator
> > >> bytes are available, fetch the full window from L2 RIP, including for a
> > >> queued or synthesized #PF VM-Exit.  Stop at a translation fault, read
> > >> failure, non-canonical address, or CS limit overrun.
> > >>
> > >> For a non-64-bit L2, truncate each incremented linear address to 32 bits
> > >> so that a fetch whose CS.base makes it cross the 4GB boundary wraps as
> > >> required.
> > >>
> > >> Do not perform tail or fallback reads for SEV guests.  KVM cannot read
> > >> plaintext instruction bytes from encrypted guest memory, and the existing
> > >> SEV emulation path treats missing hardware DecodeAssist bytes as
> > >> unavailable instead of decoding guest memory.  For nested SEV, report only
> > >> matching emulator bytes already captured for a synthesized #NPF,
> > >> potentially a zero instruction-byte count.
> > >>
> > >> Signed-off-by: Tina Zhang <zhang_wei@open-hieco.net>
> > >> ---
> > >>   arch/x86/kvm/svm/nested.c | 58 ++++++++++++++++++++++++++++++++++++++-
> > >>   1 file changed, 57 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > >> index 635ff20cc431..c677ad5df8d6 100644
> > >> --- a/arch/x86/kvm/svm/nested.c
> > >> +++ b/arch/x86/kvm/svm/nested.c
> > >> @@ -87,6 +87,54 @@ static void nested_svm_clear_synthesized_insn_bytes(struct vcpu_svm *svm)
> > >>          svm->nested.synthesized_insn_bytes.insn_len = 0;
> > >>   }
> > >>
> > >> +static u8 nested_svm_fetch_insn_bytes(struct kvm_vcpu *vcpu, u8 *bytes,
> > >> +                                     u8 count, u8 max_bytes)
> > >> +{
> > >> +       struct kvm_pagewalk *gva_walk = &vcpu->arch.gva_walk;
> > >> +       u64 access = PFERR_FETCH_MASK;
> > >> +       gva_t rip = kvm_get_linear_rip(vcpu);
> > >> +       struct x86_exception e;
> > >> +
> > >> +       if (kvm_x86_call(get_cpl)(vcpu) == 3)
> > >> +               access |= PFERR_USER_MASK;
> > >> +
> > >> +       if (!is_64_bit_mode(vcpu)) {
> > >> +               u32 eip = kvm_rip_read(vcpu);
> > >> +               u32 limit = to_svm(vcpu)->vmcb->save.cs.limit;
> > >> +
> > >> +               if (eip > limit)
> > >> +                       return 0;
> > >> +               max_bytes = min_t(u64, max_bytes, (u64)limit - eip + 1);
> > >> +       }
> > >> +
> > >> +       count = min(count, max_bytes);
> > >
> > > Ugh. Pasting together two partial reads performed at different times
> > > is egregious.

I don't love it either, but IMO (obviously) it's better than potentially reporting
completely different bytes than what KVM emulated, especially when KVM emulated
using the buffer provided by the CPU.

And practically speaking, KVM will always be splicing together two partial reads
when the instruction splits a page boundary, which is the most common case where
KVM will even need to read more bytes at this phase.

> > > This function should read all 15 bytes in one go. That
> > > pretty much renders the emulator's fetch cache useless, except when it
> > > contains the necessary 15 bytes.
> >
> > This patch was based on the discussion from the first version of this
> > series[1].  My understanding from that exchange was that preserving the
> > bytes used by the emulator and fetching the missing tail later was the
> > intended approach, as it retains the bytes actually used to decode the
> > instruction.
> >
> > Did I misunderstand the conclusion of that discussion?  If the
> > preference is now to avoid combining reads performed at different times,
> > I can change the next version to use the emulator fetch cache only when
> > it contains the full 15-byte window, and otherwise fetch all 15 bytes in
> > one operation.
> >
> > [1]
> > https://lore.kernel.org/kvm/20260629125205.52394-1-zhang_wei@open-hieco.net/T/#m3fa3f64ddd3284b312d3ddb44fd30a2e26708037
> 
> I still don't like it, but Sean overruled me, so I will be quiet now. :)

You can always appeal to Paolo.  I'm one of the District Courts, Paolo is the
Court of Appeals, and Linus is the Supreme Court.  :-D