On Sat, Sep 12, 2026 at 11:44 PM 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.
>
> Add kvm_fetch_guest_virt() as a wrapper around the generic guest virtual
> memory helper that applies instruction-fetch permissions. 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.
>
> Limit each read to the current page so that linear address boundaries can
> be checked before continuing. Outside 64-bit mode, truncate each address
> to 32 bits and limit the window at the code-segment limit. In 64-bit mode,
> stop at a non-canonical address.
>
> 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 | 64 +++++++++++++++++++++++++++++++++++++--
> arch/x86/kvm/x86.c | 23 +++++++++++---
> arch/x86/kvm/x86.h | 3 ++
> 3 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index fba230058870..5cb6a9d6907b 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -78,22 +78,80 @@ 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)
> +{
> + struct x86_exception e;
> + gva_t rip = kvm_get_linear_rip(vcpu);
> + u8 count = 0;
> +
> + max_bytes = nested_svm_get_insn_bytes_len(vcpu, max_bytes);
> +
> + while (count < max_bytes) {
> + gva_t addr = rip + count;
> + u8 chunk;
> +
> + if (!is_64_bit_mode(vcpu))
> + addr = (u32)addr;
> + else if (is_noncanonical_address(addr, vcpu, 0))
> + break;
> +
> + /*
> + * Read one page at a time to recheck the linear address at
> each
> + * boundary.
> + */
> + chunk = min_t(unsigned int, max_bytes - count,
> + PAGE_SIZE - offset_in_page(addr));
> + if (kvm_fetch_guest_virt(vcpu, addr, bytes + count,
> + chunk, &e) != X86EMUL_CONTINUE)
> + break;
> + count += chunk;
> + }
> +
> + return count;
> +}
Hmmm...You added the wrapper I suggested, but you still have the loop
I don't like. See my comments on the wrapper, below.
> 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 4b3681796c75..735b67781d55 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4848,9 +4848,24 @@ static int kvm_read_guest_virt_helper(gva_t addr, void
> *val, unsigned int bytes,
> }
>
> /* used for instruction fetching */
> -static int kvm_fetch_guest_virt(struct x86_emulate_ctxt *ctxt,
> - gva_t addr, void *val, unsigned int bytes,
> - struct x86_exception *exception)
> +int kvm_fetch_guest_virt(struct kvm_vcpu *vcpu, gva_t addr, void *val,
> + unsigned int bytes, struct x86_exception *exception)
> +{
> + u64 access = PFERR_FETCH_MASK;
> +
> + if (kvm_x86_call(get_cpl)(vcpu) == 3)
> + access |= PFERR_USER_MASK;
> +
> + memset(exception, 0, sizeof(*exception));
> + return kvm_read_guest_virt_helper(addr, val, bytes, vcpu, access,
> + exception);
> +}
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_fetch_guest_virt);
To make this work the way I envisioned it, you need to add an out
variable, unsigned int *bytes_read, to kvm_read_guest_virt_helper():
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4818,7 +4818,8 @@ 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;
@@ -4830,8 +4831,10 @@ static int kvm_read_guest_virt_helper(gva_t
addr, void *val, unsigned int bytes,
unsigned toread = min(bytes, (unsigned)PAGE_SIZE - offset);
int ret;
- if (gpa == INVALID_GPA)
- return X86EMUL_PROPAGATE_FAULT;
+ if (gpa == INVALID_GPA) {
+ r = X86EMUL_PROPAGATE_FAULT;
+ goto out;
+ }
ret = kvm_vcpu_read_guest_page(vcpu, gpa >> PAGE_SHIFT, data,
offset, toread);
if (ret < 0) {
@@ -4844,6 +4847,8 @@ static int kvm_read_guest_virt_helper(gva_t
addr, void *val, unsigned int bytes,
addr += toread;
}
out:
+ if (bytes_read)
+ *bytes_read = data - val;
return r;
}
Then, kvm_fetch_guest_virt() should return the number of bytes read
rather than an X86_EMUL_* code.
That should allow you to eliminate the loop in nested_svm_fetch_insn_bytes().
> +static int emulator_fetch_guest_virt(struct x86_emulate_ctxt *ctxt,
> + gva_t addr, void *val,
> + unsigned int bytes,
> + struct x86_exception *exception)
> {
> struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
> struct kvm_pagewalk *gva_walk = &vcpu->arch.gva_walk;
> @@ -5817,7 +5832,7 @@ static const struct x86_emulate_ops emulate_ops = {
> .write_gpr = emulator_write_gpr,
> .read_std = emulator_read_std,
> .write_std = emulator_write_std,
> - .fetch = kvm_fetch_guest_virt,
> + .fetch = emulator_fetch_guest_virt,
> .read_emulated = emulator_read_emulated,
> .write_emulated = emulator_write_emulated,
> .cmpxchg_emulated = emulator_cmpxchg_emulated,
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 0f5919b092e4..f03419a79a11 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -356,6 +356,9 @@ static inline void adjust_tsc_offset_host(struct kvm_vcpu
> *vcpu, s64 adjustment)
> int kvm_read_guest_virt(struct kvm_vcpu *vcpu,
> gva_t addr, void *val, unsigned int bytes,
> struct x86_exception *exception);
> +int kvm_fetch_guest_virt(struct kvm_vcpu *vcpu, gva_t addr, void *val,
> + unsigned int bytes,
> + struct x86_exception *exception);
>
> int kvm_write_guest_virt_system(struct kvm_vcpu *vcpu,
> gva_t addr, void *val, unsigned int bytes,
> --
> 2.43.7
>
