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
