Re: [PATCH 10/17] KVM: nVMX: Implement cache for L1 MSR bitmap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: nVMX: __kvm_gpc_init() re-initializes mutex on every VMXON
- [Medium] kvm: nVMX: gfn_to_pfn_cache breaks read-only L1 MSR bitmaps
--

commit 6f0690b4a89f3e0c76a94240889c416f2d71a8ec
Author: Fred Griffoul <fgriffo@amazon.co.uk>

KVM: nVMX: Implement cache for L1 MSR bitmap

This commit introduces a gfn_to_pfn_cache to optimize L1 MSR bitmap accesses
during L2 VM-entry, replacing individual map and unmap operations. The cache
is initialized on VMXON and deactivated when VMX operation ends.

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 151873407abd3..5fe7e5d1f72d9 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c

[ ... ]

> @@ -759,11 +793,11 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
>  			return true;
>  	}
>  
> -	CLASS(kvm_vcpu_map_local_readonly, m)(vcpu, gpa_to_gfn(vmcs12->msr_bitmap));
> -	if (m.ret)
> +	idx = nested_gpc_lock(&vmx->nested.msr_bitmap_cache, vmcs12->msr_bitmap);
> +	if (idx < 0)

[Severity: Medium]
Does this change break support for read-only L1 MSR bitmaps?

The previous code used kvm_vcpu_map_local_readonly(), which maps guest pages
without requesting write access. By switching to nested_gpc_lock(), the
underlying page lookup hardcodes FOLL_WRITE.

If the L1 hypervisor provides a read-only MSR bitmap, GPC activation will
fail here, causing KVM to silently fall back to software emulation for all
L2 MSR accesses.

>  		return false;
>  
> -	msr_bitmap_l1 = (unsigned long *)m.map.hva;
> +	msr_bitmap_l1 = (unsigned long *)vmx->nested.msr_bitmap_cache.khva;
>  
>  	/*
>  	 * To keep the control flow simple, pay eight 8-byte writes (sixteen

[ ... ]

> @@ -5443,6 +5479,9 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu)
>  
>  	vmx->nested.vpid02 = allocate_vpid();
>  
> +	/* KVM only ever reads the L1 MSR bitmap, so never mark it dirty. */
> +	__kvm_gpc_init(&vmx->nested.msr_bitmap_cache, vcpu->kvm, true, NULL, 0);

[Severity: High]
Does this re-initialize an active mutex on consecutive VMX transitions?

enter_vmx_operation() is called when the L1 guest executes VMXON, and
__kvm_gpc_init() calls mutex_init(&gpc->refresh_lock). When the guest
executes VMXOFF, free_nested() deactivates the cache but does not destroy
the mutex.

A subsequent VMXON will re-initialize the already active mutex, which
triggers lockdep/debugobjects warnings and can cause a host panic if
panic_on_warn is set.

> +
>  	/*
>  	 * Clear last_vpid to ensure that the VPID is flushed on the first
>  	 * nested VM-Enter. Otherwise, stale TLB entries from a previous life of

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=10