On Mon, Aug 24, 2026 at 5:40 AM Tina Zhang <[email protected]> wrote:
>
> SVM DecodeAssists provides instruction bytes for data-access #NPF exits.
> Hardware-reflected VM-Exits can use fresh VMCB02 bytes, but a
> KVM-synthesized #NPF has no hardware byte state to propagate.
>
> Use the emulator fetch cache when the nested #NPF is the current emulator
> exception.  Store the bytes in a one-shot buffer that is consumed while
> constructing VMCB12 and cleared before the next nested run, preventing an
> unrelated nested VM-Exit from reusing stale emulator bytes.
>
> Signed-off-by: Tina Zhang <[email protected]>
> ---
>  arch/x86/kvm/svm/nested.c | 50 ++++++++++++++++++++++++++++++++++++++-
>  arch/x86/kvm/svm/svm.h    | 12 ++++++++++
>  2 files changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 2db0ec66e8dc..635ff20cc431 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -81,11 +81,45 @@ static void nested_svm_clear_vmcb02_insn_bytes(struct 
> vcpu_svm *svm)
>         svm->nested.vmcb02_insn_bytes_fresh = false;
>  }
>
> +static void nested_svm_clear_synthesized_insn_bytes(struct vcpu_svm *svm)
> +{
> +       svm->nested.synthesized_insn_bytes.prepared = false;
> +       svm->nested.synthesized_insn_bytes.insn_len = 0;
> +}
> +
> +static void nested_svm_prepare_synthesized_insn_bytes(struct kvm_vcpu *vcpu)
> +{
> +       struct vcpu_svm *svm = to_svm(vcpu);
> +       struct nested_svm_insn_bytes *synthesized =
> +               &svm->nested.synthesized_insn_bytes;
> +       struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
> +
> +       static_assert(sizeof(synthesized->insn_bytes) >=
> +                     sizeof(ctxt->fetch.data));
> +
> +       nested_svm_clear_synthesized_insn_bytes(svm);
> +
> +       if (!guest_cpu_cap_has(vcpu, X86_FEATURE_DECODEASSISTS))
> +               return;
> +
> +       if (!ctxt || ctxt->eip != kvm_rip_read(vcpu) ||
> +           ctxt->fetch.end < ctxt->fetch.data ||
> +           ctxt->fetch.end > ctxt->fetch.data + sizeof(ctxt->fetch.data))
> +               return;

Peeking into emulator internals violates encapsulation. Rather than
performing arithmetic on fetch.data and fetch.end, the emulator should
provide a function for accessing the fetch cache.

> +       synthesized->insn_len = ctxt->fetch.end - ctxt->fetch.data;
> +       memcpy(synthesized->insn_bytes, ctxt->fetch.data,
> +              synthesized->insn_len);
> +       synthesized->prepared = true;
> +}
> +
>  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);
> +       struct nested_svm_insn_bytes *synthesized =
> +               &svm->nested.synthesized_insn_bytes;
>
>         nested_svm_clear_insn_bytes(vmcb12);
>
> @@ -93,11 +127,20 @@ static void nested_svm_update_vmcb12_insn_bytes(struct 
> kvm_vcpu *vcpu,
>             !nested_svm_vmexit_has_insn_bytes(vmcb02))
>                 goto out;
>
> -       if (svm->nested.vmcb02_insn_bytes_fresh)
> +       if (svm->nested.vmcb02_insn_bytes_fresh) {
>                 nested_svm_copy_insn_bytes(vmcb12, vmcb02);
> +               goto out;
> +       }
> +
> +       if (synthesized->prepared) {
> +               vmcb12->control.insn_len = synthesized->insn_len;
> +               memcpy(vmcb12->control.insn_bytes, synthesized->insn_bytes,
> +                      vmcb12->control.insn_len);

These instruction bytes may be short. Although this is fixed in the
next commit, introducing a bug in one patch and fixing it in the next
is bad practice.

> +       }
>
>  out:
>         svm->nested.vmcb02_insn_bytes_fresh = false;
> +       nested_svm_clear_synthesized_insn_bytes(svm);
>  }
>
>  static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu,
> @@ -106,6 +149,8 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu 
> *vcpu,
>  {
>         struct vcpu_svm *svm = to_svm(vcpu);
>         struct vmcb *vmcb = svm->vmcb;
> +       struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
> +       bool from_emulation = ctxt && fault == &ctxt->exception;
>         u64 fault_stage;
>
>         /*
> @@ -134,6 +179,8 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu 
> *vcpu,
>         vmcb->control.exit_info_2 = fault->address;
>
>         nested_svm_set_vmcb02_insn_bytes_fresh(svm, from_hardware);
> +       if (from_emulation && !(fault->error_code & PFERR_FETCH_MASK))
> +               nested_svm_prepare_synthesized_insn_bytes(vcpu);
>         nested_svm_vmexit(svm);
>  }
>
> @@ -937,6 +984,7 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm 
> *svm)
>          * Clear stale DecodeAssist data before L2 runs.
>          */
>         nested_svm_clear_vmcb02_insn_bytes(svm);
> +       nested_svm_clear_synthesized_insn_bytes(svm);
>
>         if (guest_cpu_cap_has(vcpu, X86_FEATURE_VGIF) &&
>             (vmcb12_ctrl->int_ctl & V_GIF_ENABLE_MASK))
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 610def16f700..9344707e9ffd 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -209,6 +209,12 @@ struct vmcb_ctrl_area_cached {
>         };
>  };
>
> +struct nested_svm_insn_bytes {
> +       bool prepared;
> +       u8 insn_len;
> +       u8 insn_bytes[X86_MAX_INSTRUCTION_LENGTH];
> +};
> +
>  struct svm_nested_state {
>         struct kvm_vmcb_info vmcb02;
>         u64 hsave_msr;
> @@ -245,6 +251,12 @@ struct svm_nested_state {
>
>         /* True if VMCB02 has instruction bytes from the current hardware 
> exit. */
>         bool vmcb02_insn_bytes_fresh;
> +
> +       /*
> +        * Cached instruction bytes for the current synthesized nested #NPF.
> +        * Valid until the corresponding nested VM-Exit is constructed.
> +        */
> +       struct nested_svm_insn_bytes synthesized_insn_bytes;
>  };

I think the implementatiom would be much cleaner if you wrote the
synthesized instruction bytes directly to the vmcb02, rather than
going through this staging buffer.

>  struct vcpu_sev_es_state {
> --
> 2.43.7
>

Reply via email to