Re: [PATCH v3 7/7] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Sleeping in RCU read-side critical section due to TLB flush
--

--- Patch [7]: [PATCH v3 7/7] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status ---
commit 7254897a5d4882376acaff67e062dd687ad49ea0
Author: Woodhouse, David <[email protected]>
KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status

This commit transitions the guest steal time and preempted status recording
to use the gfn_to_pfn_cache, resolving mmap_lock contention associated with
userfaultfd. It replaces explicit asm exception fixups and unsafe_put_user()
calls with plain C accesses protected by rcu_read_lock().

> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -2064,42 +2062,27 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
>  	if (WARN_ON_ONCE(current->mm != vcpu->kvm->mm))
>  		return;
>  
> -	slots = kvm_memslots(vcpu->kvm);
> +	/* We rely on the fact that it fits in a single page. */
> +	BUILD_BUG_ON((sizeof(*st) - 1) & KVM_STEAL_VALID_BITS);
>  
> -	if (unlikely(slots->generation != ghc->generation ||
> -		     gpa != ghc->gpa ||
> -		     kvm_is_error_hva(ghc->hva) || !ghc->memslot)) {
> -		/* We rely on the fact that it fits in a single page. */
> -		BUILD_BUG_ON((sizeof(*st) - 1) & KVM_STEAL_VALID_BITS);
> +	rcu_read_lock();
> +	while (!kvm_gpc_check(gpc, sizeof(*st))) {
[ ... ]
> +		if (kvm_gpc_refresh(gpc, sizeof(*st)))
>  			return;
> +
> +		rcu_read_lock();
>  	}
>  
> -	st = (struct kvm_steal_time __user *)ghc->hva;
> +	st = gpc->khva;
> +
[ ... ]
>  	if (guest_pv_has(vcpu, KVM_FEATURE_PV_TLB_FLUSH)) {
> -		u8 st_preempted = 0;
[ ... ]
> +		u8 st_preempted = xchg(&st->preempted, 0);
>  
>  		vcpu->arch.st.preempted = 0;
>  
> @@ -2107,39 +2090,33 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
>  				       st_preempted & KVM_VCPU_FLUSH_TLB);
>  		if (st_preempted & KVM_VCPU_FLUSH_TLB)
>  			kvm_vcpu_flush_tlb_guest(vcpu);

[Severity: Critical]
Can this sleep in an RCU read-side critical section?

When TDP is disabled and shadow paging is in use, kvm_vcpu_flush_tlb_guest()
calls kvm_mmu_sync_roots(), which can yield the CPU.

Looking at the call chain:

kvm_vcpu_flush_tlb_guest()
  -> kvm_mmu_sync_roots()
    -> mmu_sync_children()
      -> cond_resched_rwlock_write(&vcpu->kvm->mmu_lock)

Because this TLB flush is now called while holding rcu_read_lock() for the
gfn_to_pfn_cache access, will this trigger a scheduling while atomic panic
and potential RCU stalls when a guest requests a TLB flush?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7