Re: [PATCH 1/6] KVM: PPC: Book3S HV P9: Fix "lost kick" race

From: Fabiano Rosas <farosas@linux.ibm.com>
To: Nicholas Piggin <npiggin@gmail.com>, linuxppc-dev@lists.ozlabs.org
Cc: "Cédric Le Goater" <clg@kaod.org>, "Nicholas Piggin" <npiggin@gmail.com>
Subject: Re: [PATCH 1/6] KVM: PPC: Book3S HV P9: Fix "lost kick" race
Date: Wed, 09 Mar 2022 10:07:27 -0300	[thread overview]
Message-ID: <87bkyfb7g0.fsf@linux.ibm.com> (raw)
In-Reply-To: <20220303053315.1056880-2-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> When new work is created that requires attention from the hypervisor
> (e.g., to inject an interrupt into the guest), fast_vcpu_kick is used to
> pull the target vcpu out of the guest if it may have been running.
>
> Therefore the work creation side looks like this:
>
>   vcpu->arch.doorbell_request = 1;
>   kvmppc_fast_vcpu_kick_hv(vcpu) {
>     smp_mb();
>     cpu = vcpu->cpu;
>     if (cpu != -1)
>         send_ipi(cpu);
>   }
>
> And the guest entry side *should* look like this:
>
>   vcpu->cpu = smp_processor_id();
>   smp_mb();
>   if (vcpu->arch.doorbell_request) {
>     // do something (abort entry or inject doorbell etc)
>   }
>
> But currently the store and load are flipped, so it is possible for the
> entry to see no doorbell pending, and the doorbell creation misses the
> store to set cpu, resulting lost work (or at least delayed until the
> next guest exit).
>
> Fix this by reordering the entry operations and adding a smp_mb
> between them. The P8 path appears to have a similar race which is
> commented but not addressed yet.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>