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

[Gautam Menghani <gautam@linux.ibm.com>]

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 <vaibhav@linux.ibm.com>
> 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 <agraf@suse.de>.
>   */
>  
> +#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);


- Gautam