Re: [PATCH for-5.2] Revert series "spapr/xive: Allocate vCPU IPIs from the vCPU contexts"

[Greg Kurz <groug@kaod.org>]

On Mon, 16 Nov 2020 16:54:32 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 11/16/20 4:34 PM, Greg Kurz wrote:
> > This series was largely built on the assumption that IPI numbers are
> > numerically equal to vCPU ids. Even if this happens to be the case
> > in practice with the default machine settings, this ceases to be true
> > if VSMT is set to a different value than the number of vCPUs per core.
> > This causes bogus IPI numbers to be created in KVM from a guest stand
> > point. This leads to unknow results in the guest, including crashes
> > or missing vCPUs (see BugLink) and even non-fatal oopses in current
> > KVM that lacks a check before accessing misconfigured HW (see [1]).
> > 
> > A tentative patch was sent (see [2]) but it seems too complex to be
> > merged in an RC. Since the original changes are essentially an
> > optimization, it seems safer to revert them for now. The damage is
> > done by commit acbdb9956fe9 ("spapr/xive: Allocate IPIs independently
> > from the other sources") but the previous patches in the series are
> > really preparatory patches. So this reverts the whole series:
> > 
> > eab0a2d06e97 ("spapr/xive: Allocate vCPU IPIs from the vCPU contexts")
> > acbdb9956fe9 ("spapr/xive: Allocate IPIs independently from the other sources")
> 
> These are introducing the optimisation to allocate the vCPU IPI from the 
> running task, and, at the same time, the issue for guests using vSMT.
> 
> > fa94447a2cd6 ("spapr/xive: Use kvmppc_xive_source_reset() in post_load")
> > 235d3b116213 ("spapr/xive: Modify kvm_cpu_is_enabled() interface")
> 
> IMO, these two last patches are fine. 
> 

235d3b116213 is useless if you no longer want to feed kvm_cpu_is_enabled()
with IPI numbers ;-) , so it seems safer to keep it taking a CPU state
pointer.

Keeping fa94447a2cd6 without 235d3b116213 and fa94447a2cd6 wouldn't make
a lot of sense since the next try at implementing the optimization will
likely result in a different set of changes. It would certainly be more
beneficial to get the feature with a brand new series IMHO.

Cheers,

--
Greg

> C. 
> 
>