Re: [PATCH 2/3] KVM: PPC: Book3S HV: Add preempt_disable() around virtual-mode HPTE bit-lock users

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: ppc: hard deadlock due to asymmetric preemption in HPTE bit-lock
--

commit 4ab3d02887c651ee39ea07438e210fdda4821de1
Author: Amit Machhiwal <amachhiw@linux.ibm.com>

KVM: PPC: Book3S HV: Add preempt_disable() around virtual-mode HPTE bit-lock users
    
This commit wraps virtual-mode HPTE bit-lock users in
kvmppc_pseries_do_hcall() and kvmppc_handle_exit_hv() with
preempt_disable() and preempt_enable() pairs. This is intended to prevent
indefinite spinning if a vCPU thread is preempted while holding the bit-lock.

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index aa51968e206a7..52f72f30baf0a 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1212,9 +1212,11 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>  	case H_CLEAR_REF:
>  	case H_PROTECT:
>  	case H_BULK_REMOVE:
> +		preempt_disable();

[Severity: High]
Does disabling preemption here expose the system to a hard deadlock?

While this patch ensures the virtual-mode spin loop won't be preempted,
host-side HPTE bit-lock owners like kvm_unmap_rmapp() still acquire the
lock and execute their critical sections with preemption enabled.

For example, when called from kvmppc_core_flush_memslot_hv() under
slots_arch_lock mutex, preemption is not disabled:

arch/powerpc/kvm/book3s_64_mmu_hv.c:kvmppc_core_flush_memslot_hv() {
	for (n = memslot->npages; n; --n, ++gfn) {
		...
		if (*rmapp & KVMPPC_RMAP_PRESENT)
			kvm_unmap_rmapp(kvm, memslot, gfn);
		++rmapp;
	}
}

The lock is acquired and held without disabling preemption:

arch/powerpc/kvm/book3s_64_mmu_hv.c:kvm_unmap_rmapp() {
	if (!try_lock_hpte(hptep, HPTE_V_HVLOCK)) {
		/* unlock rmap before spinning on the HPTE lock */
		unlock_rmap(rmapp);
		while (be64_to_cpu(hptep[0]) & HPTE_V_HVLOCK)
			cpu_relax();
		continue;
	}

	kvmppc_unmap_hpte(kvm, i, memslot, rmapp, gfn);
	unlock_rmap(rmapp);
	__unlock_hpte(hptep, be64_to_cpu(hptep[0]));
}

If the host-side thread executing kvm_unmap_rmapp() is preempted while
holding HPTE_V_HVLOCK, and a vCPU thread on the same CPU subsequently enters
kvmppc_pseries_do_hpt_hcall(), the vCPU thread will spin indefinitely trying
to acquire the lock because preemption is now disabled here. The preempted
lock owner will never be scheduled on that CPU to release the lock.

>  		idx = srcu_read_lock(&kvm->srcu);
>  		ret = kvmppc_pseries_do_hpt_hcall(vcpu, req);
>  		srcu_read_unlock(&kvm->srcu, idx);
> +		preempt_enable();
>  		if (ret == H_TOO_HARD)
>  			return RESUME_HOST;
>  		break;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928113704.48912-4-amachhiw@linux.ibm.com?part=2