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

[Amit Machhiwal <amachhiw@linux.ibm.com>]

On 2026/10/07 06:47 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] kvm: deadlock in kvm_test_age_rmapp() due to missing preempt_disable()
> --
> 
> commit 6b1add31442320e6307f450d080fd72f64ee2368
> Author: Amit Machhiwal <amachhiw@linux.ibm.com>
> 
> KVM: PPC: Book3S HV: Add preempt_disable() around virtual-mode HPTE bit-lock users
> 
> This commit enforces that threads acquiring HPTE or RMAP bit-locks run with
> preemption disabled to prevent CPU hangs. It adds preempt_disable() and
> preempt_enable() pairs around various bit-lock acquisition paths.
> 
> > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > index 2ccb3d138f46c..e8f73c8f97803 100644
> > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > @@ -890,6 +913,7 @@ static bool kvm_age_rmapp(struct kvm *kvm, struct kvm_memory_slot *memslot,
> >  
> >  	rmapp = &memslot->arch.rmap[gfn - memslot->base_gfn];
> >   retry:
> > +	preempt_disable();
> >  	lock_rmap(rmapp);
> >  	if (*rmapp & KVMPPC_RMAP_REFERENCED) {
> >  		*rmapp &= ~KVMPPC_RMAP_REFERENCED;
> 
> [Severity: High]
> Does kvm_test_age_rmapp() also need to disable preemption before acquiring
> the rmap lock?
> 
> Looking at arch/powerpc/kvm/book3s_64_mmu_hv.c:kvm_test_age_rmapp():
> 
> 	lock_rmap(rmapp);
> 	if (*rmapp & KVMPPC_RMAP_REFERENCED)
> 		goto out;

This is not an issue on powerpc; here is why.

kvm_test_age_rmapp() has exactly one call site:

    kvm_test_age_gfn_hv()                 [book3s_64_mmu_hv.c:1020]
      <- kvm_test_age_gfn()               [book3s.c:896]
        <- kvm_mmu_notifier_test_young()  [virt/kvm/kvm_main.c:864]

Inside kvm_handle_hva_range(), the handler is invoked only after
KVM_MMU_LOCK(kvm) at kvm_main.c:626, which on powerpc expands to
spin_lock(&kvm->mmu_lock).  mmu_lock is a spinlock_t
(include/linux/kvm_host.h:774), and spin_lock() calls preempt_disable()
internally.  By the time kvm_test_age_rmapp() acquires lock_rmap(), preemption
is already disabled by the outer mmu_lock acquisition.

The lockless path in kvm_handle_hva_range() (line 625, gated on
CONFIG_KVM_MMU_LOCKLESS_AGING) would bypass KVM_MMU_LOCK and reach the handler
with preemption still enabled.  However, CONFIG_KVM_MMU_LOCKLESS_AGING is
selected only by x86 and s390; powerpc does not select it, so this branch is
dead code on powerpc at compile time.

No additional preempt_disable() is needed in kvm_test_age_rmapp().

~ Amit

> 
> This MMU notifier callback still acquires the KVMPPC_RMAP_LOCK_BIT without
> disabling preemption.
> 
> If a thread is preempted while holding this bit-lock, and another thread on
> the same CPU begins spinning for the same lock with preemption disabled
> (such as in the newly patched paths), couldn't this result in a permanent
> CPU hang?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261007182116.12479-1-amachhiw@linux.ibm.com?part=2