Re: [PATCH] KVM: PPC: Book3S HV nestedv2: Don't drop pending doorbell across L2 entry

On Mon, Aug 10, 2026 at 04:54:18PM +0530, Gautam Menghani wrote:
> On Mon, Aug 03, 2026 at 09:14:25AM +0530, Vaibhav Jain wrote:
> > On nestedv2 the L1 converts a pending doorbell into guest DPDES state at
> > the top of kvmhv_vcpu_entry_nestedv2() and immediately forgets about it:
> > 
> > 	if (vcpu->arch.doorbell_request) {
> > 		vcpu->arch.doorbell_request = 0;
> > 		kvmppc_set_dpdes(vcpu, 1);
> > 	}
> > 
> > Clearing 'doorbell_request' at this point assumes that handing DPDES to the
> > L0 is equivalent to the L2 having taken the doorbell. That is not true, and
> > the doorbell can be lost in two ways:
> > 
> >   - The block runs before the lazy_irq_pending() check, so the doorbell is
> >     consumed even on the path that returns 0 without ever calling
> >     H_GUEST_RUN_VCPU.
> > 
> >   - DPDES stays pending in the L2 until it is actually delivered. The L2
> >     may exit for an unrelated reason (hcall, page fault, HDEC) with the
> >     doorbell still set, typically because it was running with MSR[EE]=0.
> >     Nothing reloads DPDES afterwards, so the L1 never learns this.
> > 
> > Once 'doorbell_request' has been cleared, the L1 has no record of the
> > pending doorbell. kvmppc_doorbell_pending() returns false, so
> > kvmppc_read_dpdes() reports the target thread as idle when a sibling vCPU
> > emulates 'mfspr DPDES', and the vCPU can be treated as having no work
> > pending and blocked. From the L2's point of view the doorbell is silently
> > lost, which shows up as an SMT guest hanging on a doorbell-based IPI.
> > 
> > Fix this by making 'doorbell_request' track the L2's DPDES rather than
> > being consumed by entry:
> > 
> >   - inject DPDES after the early-return paths and before
> >     kvmhv_nestedv2_flush_vcpu() serializes it into the vcpu run input
> >     buffer, and no longer clear 'doorbell_request' there,
> > 
> >   - after H_GUEST_RUN_VCPU, reload DPDES from the L0. The run output only
> >     carries the state the L0 chose to return and the 'valids' bitmap is
> >     zeroed on exit, so an explicit kvmhv_nestedv2_cached_reload() is
> >     needed to see the L2's current value,
> > 
> >   - if DPDES is still set the doorbell was not delivered, so keep
> >     'doorbell_request' pending so that it is re-injected on the next
> >     entry; otherwise clear it.
> > 
> > This keeps a pending doorbell visible to the L1 for as long as the L2 has
> > not consumed it, so vCPU wakeup and DPDES emulation on sibling vCPUs stay
> > consistent with the L2's actual state.
> > 
> > Fixes: 54ec2bd9e017 ("KVM: PPC: Book3S HV nestedv2: Fix doorbell emulation")
> > Signed-off-by: Vaibhav Jain <[email protected]>
> > Assisted-by: Claude:Opus-5
> > ---
> >  arch/powerpc/kvm/book3s_hv.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > index 61dbeea317f3..40f8717b8a7d 100644
> > --- a/arch/powerpc/kvm/book3s_hv.c
> > +++ b/arch/powerpc/kvm/book3s_hv.c
> > @@ -15,6 +15,7 @@
> >   * by Alexander Graf <[email protected]>.
> >   */
> >  
> > +#include "asm/guest-state-buffer.h"
> >  #include <linux/kvm_host.h>
> >  #include <linux/kernel.h>
> >  #include <linux/err.h>
> > @@ -4253,11 +4254,6 @@ static int kvmhv_vcpu_entry_nestedv2(struct kvm_vcpu *vcpu, u64 time_limit,
> >  	int trap;
> >  	long rc;
> >  
> > -	if (vcpu->arch.doorbell_request) {
> > -		vcpu->arch.doorbell_request = 0;
> > -		kvmppc_set_dpdes(vcpu, 1);
> > -	}
> > -
> >  	io = &vcpu->arch.nestedv2_io;
> >  
> >  	msr = mfmsr();
> > @@ -4265,6 +4261,9 @@ static int kvmhv_vcpu_entry_nestedv2(struct kvm_vcpu *vcpu, u64 time_limit,
> >  	if (lazy_irq_pending())
> >  		return 0;
> >  
> > +	if (vcpu->arch.doorbell_request)
> > +		kvmppc_set_dpdes(vcpu, 1);
> > +
> >  	rc = kvmhv_nestedv2_flush_vcpu(vcpu, time_limit);
> >  	if (rc < 0)
> >  		return -EINVAL;
> > @@ -4296,6 +4295,17 @@ static int kvmhv_vcpu_entry_nestedv2(struct kvm_vcpu *vcpu, u64 time_limit,
> >  	if (rc < 0)
> >  		return -EINVAL;
> >  
> > +	/* Check if privileged door bell was requested and handled */
> > +	if (vcpu->arch.vcore->dpdes) {
> > +		kvmhv_nestedv2_cached_reload(vcpu, KVMPPC_GSID_DPDES);
> > +		if (vcpu->arch.vcore->dpdes)
> > +			vcpu->arch.doorbell_request |= vcpu->arch.vcore->dpdes;
> > +		else
> > +			cpu->arch.doorbell_request = 0;
> 
> This is a compile error, should be "vcpu->"
> 
> 
> > +	} else {
> > +		vcpu->arch.doorbell_request = 0;
> > +	}
> > +
> 
> I think this entire block after vcpu exit can be reduced to just
> 
>     if (vcpu->arch.doorbell_request)
>         vcpu->arch.doorbell_request = kvmppc_get_dpdes(vcpu);
> 

Gave this a bit more thought - this can miss the doorbell that was sent
to this vcpu while the vcpu was running. I think the below code should
work fine:

     if (vcpu->arch.doorbell_request) {
             vcpu->arch.doorbell_request = 0;
             kvmppc_set_dpdes(vcpu, 1);
     }

     rc = plpar_guest_run_vcpu(...);

    /* Restore the doorbell status and also account for the doorbell
     * received when this vcpu was running */
     if (vcpu->arch.vcore->dpdes)
        vcpu->arch.doorbell_request |= kvmppc_get_dpdes(vcpu);


- Gautam