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

[Jim Mattson]

On Sat, Sep 5, 2026 at 11:40 PM Tina Zhang <zhang_wei@xxxxxxxxxxxxxx> wrote:
>
>
>
> On 9/5/2026 8:39 AM, Jim Mattson wrote:
> > On Mon, Aug 24, 2026 at 5:40 AM Tina Zhang <zhang_wei@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> >> ---
> >>   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. 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@xxxxxxxxxxxxxx/T/#m3fa3f64ddd3284b312d3ddb44fd30a2e26708037

I still don't like it, but Sean overruled me, so I will be quiet now. :)