On Wed, Sep 23, 2026 at 7:03 AM Tina Zhang <[email protected]> wrote:
>
> Hardware-reflected #NPF and #PF VM-Exits can propagate instruction bytes
> from VMCB02, but KVM-synthesized exits have no hardware-provided state for
> the current exit.
>
> Extend kvm_read_guest_virt_helper() to optionally report the number of
> bytes read, including partial reads. Add kvm_fetch_guest_virt() to apply
> instruction-fetch permissions and return that count, reusing the generic
> helper's page-by-page read loop.
>
> When a synthesized data #NPF or #PF has no valid VMCB02 instruction bytes,
> fetch up to 15 bytes from L2's RIP while constructing VMCB12. Preserve
> the bytes already read if a subsequent read cannot be completed.
>
> For instruction fetches, truncate each linear address to 32 bits outside
> 64-bit mode and stop at a non-canonical address in 64-bit mode. Limit
> the fetch window at the code-segment limit outside 64-bit mode.
>
> Do not attempt the fallback for SEV guests, as KVM cannot read encrypted
> guest memory directly.
>
> Signed-off-by: Tina Zhang <[email protected]>
> ---
> arch/x86/kvm/svm/nested.c | 41 +++++++++++++++++++++++++---
> arch/x86/kvm/x86.c | 56 +++++++++++++++++++++++++++++++--------
> arch/x86/kvm/x86.h | 2 ++
> 3 files changed, 85 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index e839f61211c9..813b0fc663c6 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -78,22 +78,57 @@ static void
> nested_svm_invalidate_vmcb02_insn_bytes(struct vcpu_svm *svm)
> svm->nested.vmcb02_insn_bytes_valid = false;
> }
>
> +static u8 nested_svm_get_insn_bytes_len(struct kvm_vcpu *vcpu, u8 max_bytes)
> +{
> + 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);
> + }
> +
> + return max_bytes;
> +}
> +
> +static u8 nested_svm_fetch_insn_bytes(struct kvm_vcpu *vcpu, u8 *bytes,
> + u8 max_bytes)
> +{
> + gva_t rip = kvm_get_linear_rip(vcpu);
> +
> + max_bytes = nested_svm_get_insn_bytes_len(vcpu, max_bytes);
> +
> + return kvm_fetch_guest_virt(vcpu, rip, bytes, max_bytes);
> +}
> +
> static void nested_svm_update_vmcb12_insn_bytes(struct kvm_vcpu *vcpu,
> struct vmcb *vmcb12,
> const struct vmcb *vmcb02)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
> + const u8 max_bytes = sizeof(vmcb12->control.insn_bytes);
>
> if (!guest_cpu_cap_has(vcpu, X86_FEATURE_DECODEASSISTS))
> goto out;
>
> - if (!nested_svm_vmexit_supports_insn_bytes(vmcb02) ||
> - !svm->nested.vmcb02_insn_bytes_valid) {
> + if (!nested_svm_vmexit_supports_insn_bytes(vmcb02)) {
> nested_svm_invalidate_insn_bytes(vmcb12);
> goto out;
> }
>
> - nested_svm_copy_insn_bytes(vmcb12, vmcb02);
> + if (svm->nested.vmcb02_insn_bytes_valid) {
> + nested_svm_copy_insn_bytes(vmcb12, vmcb02);
> + goto out;
> + }
> +
> + if (!is_sev_guest(vcpu))
> + vmcb12->control.insn_len =
> + nested_svm_fetch_insn_bytes(vcpu,
> +
> vmcb12->control.insn_bytes,
> + max_bytes);
> + else
> + nested_svm_invalidate_insn_bytes(vmcb12);
>
> out:
> svm->nested.vmcb02_insn_bytes_valid = false;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1705e7be46ec..ad8b35f2219d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4772,20 +4772,34 @@ gpa_t kvm_mmu_gva_to_gpa_system(struct kvm_vcpu
> *vcpu, gva_t gva,
>
> static int kvm_read_guest_virt_helper(gva_t addr, void *val, unsigned int
> bytes,
> struct kvm_vcpu *vcpu, u64 access,
> - struct x86_exception *exception)
> + struct x86_exception *exception,
> + unsigned int *bytes_read)
> {
> struct kvm_pagewalk *gva_walk = &vcpu->arch.gva_walk;
> void *data = val;
> int r = X86EMUL_CONTINUE;
>
> while (bytes) {
> - gpa_t gpa = gva_walk->gva_to_gpa(vcpu, gva_walk, addr,
> access, exception);
> + gpa_t gpa;
> unsigned offset = addr & (PAGE_SIZE-1);
> unsigned toread = min(bytes, (unsigned)PAGE_SIZE - offset);
> int ret;
>
> - if (gpa == INVALID_GPA)
> - return X86EMUL_PROPAGATE_FAULT;
> + /* Apply address wrapping or canonicality checks before each
> fetch chunk. */
> + if (access & PFERR_FETCH_MASK) {
> + if (!is_64_bit_mode(vcpu))
> + addr = (u32)addr;
> + else if (is_noncanonical_address(addr, vcpu, 0)) {
> + r = X86EMUL_UNHANDLEABLE;
> + goto out;
> + }
> + }
I hadn't anticipated the need for this special-case, and it's more
intrusive than just returning the number of bytes read.
I see now that it's complicated to match the behavior of the
emulator's __do_insn_fetch_bytes(). You could clamp the number of
bytes to read up-front to deal with the end of the address space, but
you would still need a second call to handle the wrap. Something like
the following (mangled by my MUA):
static unsigned int __kvm_fetch_guest_virt(struct kvm_vcpu *vcpu, gva_t addr,
void *val, unsigned int bytes)
{
struct x86_exception exception = {};
unsigned int bytes_read;
u64 access = PFERR_FETCH_MASK;
if (kvm_x86_call(get_cpl)(vcpu) == 3)
access |= PFERR_USER_MASK;
kvm_read_guest_virt_helper(addr, val, bytes, vcpu, access,
&exception, &bytes_read);
return bytes_read;
}
unsigned int kvm_fetch_guest_virt(struct kvm_vcpu *vcpu, gva_t addr,
void *val, unsigned int bytes)
{
unsigned int first, bytes_read;
bool wraps;
u64 avail;
/* An instruction-sized fetch can wrap at most once. */
if (WARN_ON_ONCE(bytes > X86_MAX_INSTRUCTION_LENGTH))
return 0;
if (!is_64_bit_mode(vcpu)) {
addr = (u32)addr;
avail = BIT_ULL(32) - addr;
wraps = true;
} else if (is_noncanonical_address(addr, vcpu, 0)) {
return 0;
} else if (addr < BIT_ULL(63)) {
avail = BIT_ULL(vcpu_virt_addr_bits(vcpu) - 1) - addr;
wraps = false;
} else {
avail = -addr;
wraps = true;
}
first = min_t(u64, bytes, avail);
bytes_read = __kvm_fetch_guest_virt(vcpu, addr, val, first);
if (wraps && bytes_read == first && first < bytes)
bytes_read += __kvm_fetch_guest_virt(vcpu, 0, val + first,
bytes - first);
return bytes_read;
}
I think this solution may be worse than what you have now, but I will
defer to Sean. Maybe he will have a better idea. (Maybe my suggestion
to return the number of bytes read was misguided.)
Reviewed-by: Jim Mattson <[email protected]>
