On Sat, Sep 5, 2026 at 11:40 PM Tina Zhang <[email protected]> wrote: > > > > On 9/5/2026 8:39 AM, Jim Mattson wrote: > > On Mon, Aug 24, 2026 at 5:40 AM Tina Zhang <[email protected]> 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 <[email protected]> > >> --- > >> 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/[email protected]/T/#m3fa3f64ddd3284b312d3ddb44fd30a2e26708037
I still don't like it, but Sean overruled me, so I will be quiet now. :)
