Re: [PATCH v2 3/8] KVM: nSVM: Track fresh VMCB02 DecodeAssist bytes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The patch fails to set `vmcb02_insn_bytes_fresh` to `true` when reflecting a host-intercepted Guest Page Fault (`#PF`) to L1, discarding hardware-provided DecodeAssist bytes.

Pre-existing issues:
- [High] KVM incorrectly identifies synthesized nested page faults (NPFs) as hardware-generated if they occur during emulation of an original hardware NPF, passing a stale `exit_info_2` (GPA) to L1.
--

--- Patch [3]: [PATCH v2 3/8] KVM: nSVM: Track fresh VMCB02 DecodeAssist bytes ---
commit 1b65a65904c11af4f22d9f6d0ea2c488ec16575d
Author: Tina Zhang <zhang_wei@open-hieco.net>

KVM: nSVM: Track fresh VMCB02 DecodeAssist bytes

Track whether VMCB02 contains DecodeAssist instruction bytes from the
hardware VM-Exit currently being reflected to L1. Clear the fields before
each nested run so stale bytes cannot be mistaken for fresh hardware state.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index b340dc9991adb..70e2ee3af78b2 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -33,13 +33,21 @@
>  
>  #define CC KVM_NESTED_VMENTER_CONSISTENCY_CHECK
>  
> +static void nested_svm_clear_insn_bytes(struct vmcb *vmcb)
> +{
> +	vmcb->control.insn_len = 0;
> +	memset(vmcb->control.insn_bytes, 0,
> +	       sizeof(vmcb->control.insn_bytes));
> +}
> +
>  static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu,
>  				       struct x86_exception *fault)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	struct vmcb *vmcb = svm->vmcb;
> +	bool from_hardware = vmcb->control.exit_code == SVM_EXIT_NPF;
>  
> -	if (vmcb->control.exit_code != SVM_EXIT_NPF) {
> +	if (!from_hardware) {

[Severity: High]
This isn't a bug introduced by this patch, but does evaluating the original
hardware exit code correctly identify emulator-synthesized nested page faults?

If an L2 guest accesses an unmapped MMIO region, it causes a hardware NPF.
L0 intercepts it and starts emulation. If the emulator then performs a guest
page table walk or instruction fetch which faults in the nested NPT, the
emulator injects a new NPF to L1 by calling nested_svm_inject_npf_exit().

Because the VMCB exit code still contains the original hardware NPF exit code,
this check evaluates to true. This causes KVM to skip updating the faulting
GPA with the new faulting address, misleading the L1 hypervisor with the
original MMIO GPA and incorrectly marking stale instruction bytes as fresh.

[ ... ]
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 5137416be593d..fa65f6a1a59ba 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -238,6 +238,13 @@ struct svm_nested_state {
>  	 * on its side.
>  	 */
>  	bool force_msr_bitmap_recalc;
> +
> +	/*
> +	 * True if VMCB02's DecodeAssist instruction bytes belong to the hardware
> +	 * VM-Exit currently being reflected to L1.  KVM-synthesized exits leave
> +	 * it clear.
> +	 */
> +	bool vmcb02_insn_bytes_fresh;

[Severity: High]
Does KVM need to set vmcb02_insn_bytes_fresh to true when reflecting a 
host-intercepted Guest Page Fault (#PF) to L1?

If L0 KVM intercepts a #PF from L2 (e.g., for shadow paging or async PF),
it processes the fault and later queues it for reflection to L1 if L1 also
intercepts it. KVM synthesizes the exit via
nested_svm_inject_exception_vmexit():

arch/x86/kvm/svm/nested.c:nested_svm_inject_exception_vmexit() {
	...
	vmcb->control.exit_code = SVM_EXIT_EXCP_BASE + ex->vector;
	...
}

However, this path fails to set vmcb02_insn_bytes_fresh to true. Since the
flag defaults to false when entering L2, it remains false, and any
hardware-provided DecodeAssist bytes for the #PF are discarded. This could
break instruction decoding and emulation logic in the L1 hypervisor.

>  };
>  
>  struct vcpu_sev_es_state {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783999988.git.zhang_wei@open-hieco.net?part=3