Re: [PATCH] KVM: nVMX: Don't load L1's host state when freeing a vCPU

[Sean Christopherson <seanjc@google.com>]

On Sat, Aug 01, 2026, Hyunwoo Kim wrote:
> Don't load L1's host state when kicking a vCPU out of nested guest mode as
> part of freeing the vCPU, as loading host state processes vmcs12's VM-Exit
> MSR load list, i.e. reads an (index, value) pair out of guest memory and
> feeds it to kvm_emulate_msr_write() with host_initiated=false. Letting the
> guest emulate WRMSR against VM-scope state that KVM is actively tearing
> down goes sideways in at least two ways.
> 
> Writing HV_X64_MSR_ICR sends an IPI, which for a non-shorthand,
> non-broadcast destination walks kvm->arch.apic_map to dereference the
> target's local APIC. kvm_free_lapic() neither rebuilds nor dirties the map,
> and the map is freed only after all vCPUs are destroyed, i.e. the map still
> points at the already-freed local APIC of a previously destroyed vCPU. This
> requires userspace to expose Hyper-V's CPUID to the guest. Writing
> MSR_KVM_SYSTEM_TIME_NEW activates the kvmclock gfn=>pfn cache, which leaves
> the cache's list entry, resident in the about-to-be-freed vCPU, linked into
> kvm->gpc_list; the next vCPU to manipulate the list writes through that
> entry.

Wouldn't this also require an "unclean" shutdown of the VM, because the VM would
still need live memslots in order to process the MSR load/store lists.  I wonder
if that's an avenue to a short-term stopgap "fix" as well as long-term hardening.
E.g. if KVM were to nuke memslots as part of kvm_destroy_vm(), I think that would
plug this particular hole?

> The vCPU will never run again, so nothing can observe the loaded host
> state. Simply restore KVM's MMU pointers so that they aren't left pointing
> at the nested MMU, and bail. nSVM does the same, i.e. doesn't emulate a
> VM-Exit when forcibly leaving nested mode, and performs only the equivalent
> MMU cleanup.

I don't have the links off-hand, but nSVM's behavior of not emulating VM-Exit
has also led to problems (I think we've failed to account for things that are
handled by the VM-Exit path, on multiple occassions).  That said, emulating a
VM-Exit while a vCPU is being destroyed is beyond awful, e.g. it requires
loading+putting the vCPU, which is its own gigantic can of worms.

> Bail just before the branch that splits the success and VM-Fail paths, as
> leaving guest mode, canceling the VMX-preemption timer, and switching back
> to vmcs01 are all needed by the free path. Canceling the timer is in fact
> the only reason the free path goes through an emulated VM-Exit, see commit
> b4b65b5642d6 ("KVM: x86: cleanup freeing of nested state").

IIRC, we've accumulated more horrors since then.  I completely agree this code
is buggy and needs to be fixed, but I don't want to take a quick-and-dirty fix,
at least not without an exit strategy, which would/should force us to assess
exactly what is/isn't needed from the __nested_vmx_vmexit() flow. 

> Fixes: b4b65b5642d6 ("KVM: x86: cleanup freeing of nested state")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 12 ++++++++++++
>  arch/x86/kvm/vmx/vmx.h    |  3 +++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index ddf6df7bee93b2..8d58547acb1887 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -384,6 +384,8 @@ static void free_nested(struct kvm_vcpu *vcpu)
>   */
>  void nested_vmx_free_vcpu(struct kvm_vcpu *vcpu)
>  {
> +	to_vmx(vcpu)->nested.vcpu_is_dying = true;
> +
>  	vcpu_load(vcpu);
>  	vmx_leave_nested(vcpu);
>  	vcpu_put(vcpu);
> @@ -5173,6 +5175,16 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
>  	/* in case we halted in L2 */
>  	kvm_set_mp_state(vcpu, KVM_MP_STATE_RUNNABLE);
>  
> +	/*
> +	 * Don't emulate guest-controlled state, e.g. vmcs12's VM-Exit MSR load
> +	 * list, when freeing the vCPU.  Bail only after leaving guest mode,
> +	 * canceling the preemption timer, and switching back to vmcs01.
> +	 */
> +	if (vmx->nested.vcpu_is_dying) {
> +		nested_ept_uninit_mmu_context(vcpu);
> +		return;
> +	}
> +
>  	if (likely(!vmx->fail)) {
>  		if (vm_exit_reason != -1)
>  			trace_kvm_nested_vmexit_inject(vmcs12->vm_exit_reason,
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index dc8517f15bc463..2bacd3fe4c7ded 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -76,6 +76,9 @@ struct nested_vmx {
>  	gpa_t vmxon_ptr;
>  	bool pml_full;
>  
> +	/* Set when freeing the vCPU, to suppress emulation of guest state. */
> +	bool vcpu_is_dying;
> +
>  	/* The guest-physical address of the current VMCS L1 keeps for L2 */
>  	gpa_t current_vmptr;
>  	/*
> -- 
> 2.43.0
>