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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Concurrent doorbell requests from sibling vCPUs are silently lost due to unconditional clearing of `doorbell_request` on exit.
- [Low] Typo in variable name `cpu` instead of `vcpu` leads to an undeclared identifier and build failure.
--

--- Patch [1]: [PATCH] KVM: PPC: Book3S HV nestedv2: Don't drop pending doorbell across L2 entry ---
commit efc59a0f35293530907029b05624f01523b4f3ea
Author: Vaibhav Jain <[email protected]>

KVM: PPC: Book3S HV nestedv2: Don't drop pending doorbell across L2 entry

This patch changes how pending doorbells are tracked during nestedv2 entry.
Instead of unconditionally consuming the doorbell on entry, it tracks the
L2's DPDES state and only clears the request if it was actually handled by
the L2.

Fixes: 54ec2bd9e017 ("KVM: PPC: Book3S HV nestedv2: Fix doorbell emulation")

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c

[ ... ]

> @@ -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;

[Severity: Low]
Is there a typo in this assignment?

Using cpu instead of vcpu here appears to cause a build failure due to an
undeclared identifier.

> +	} else {
> +		vcpu->arch.doorbell_request = 0;

[Severity: High]
Could this unconditional clear overwrite concurrent doorbell requests from
sibling vCPUs?

If a sibling vCPU executes msgsndp and sets doorbell_request to 1 while the
target vCPU is executing the L2 guest, that update might be lost.

When the target vCPU exits with vcpu->arch.vcore->dpdes == 0, this path
would wipe out any concurrent doorbell_request set by the sibling during the
guest's execution or exit process. Would this cause guest SMT threads to
miss IPIs and hang?

> +	}
> +
>  	timer_rearm_host_dec(*tb);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1