Re: [PATCH v3 10/18] KVM: arm64: Handle PSCI calls for protected VMs at EL2

[Will Deacon]

On Thu, Sep 24, 2026 at 12:26:25PM +0100, Fuad Tabba wrote:
> On Thu, 24 Sep 2026 09:30:06 +0100, Will Deacon <will@xxxxxxxxxx> wrote:
> [...]
> > Sorry, but I'm really confused by this and it appears to be different to
> > what we've got in Android as well. Why do we need release semantics for
> > the store to 'hyp_vcpu->power_state' in pvm_psci_vcpu_off()? What is it
> > that we are publishing here? The comment talks about pkvm_reset_vcpu(),
> > but how is that relevant to the vCPU _off_ path? You say the comment is
> > wrong, but what _should_ it say?
> >
> > I'm a bit baffled!
> 
> It's not publishing data, it's handing reset_state back.

What's the difference? The usual pattern for acquire/release is:

<write data>
<store-release flag>

on one CPU and then on another:

<load-acquire flag>	// If this reads from the release above...
<read data>		// ... then this is guaranteed to read the written data

That's a message-passing shape and you would normally say that the first
CPU (the producer) is publishing the data to the other CPU (the consumer).

Is this what is happening with the 'reset_state' (data) and the
'power_state' (flag)? If not, then what is the shape?

> The CPU_ON winner writes reset_state.{pc, r0, be}, then reset_state.reset
> with a release. The target reads them and clears reset in pkvm_reset_vcpu()
> on its next run

By 'next run' you mean, at EL2 on the entry path into the guest following
a successful CPU_ON operation?

> and its CPU_OFF hands reset_state on to the next
> CPU_ON. The release on OFF orders those reads and that clear before
> OFF, and an acquire on the winner's cmpxchg orders its writes after
> it: release on the way out, acquire on the way in, like a lock.

That doesn't make sense to me, sorry. You're saying that the release
store in the EL2 CPU_OFF hypercall handler is ordering stores that were
made during the initial CPU_ON handling on that vCPU? Since then, we've
been in and out of the guest. We really shouldn't need extra barriers to
create order there.

> Without the release, the target's clear of reset can become visible
> after the next winner's reset = true, and the target's next
> pkvm_reset_vcpu() then reads a clear flag and returns -ECANCELED,
> leaving the vCPU stuck at ON_PENDING.

This needs a litmus test because I can't see it myself. As above,
CPU_OFF does not clear the reset state, so it's bizarre to put the
release there.

> The comment described the flag ordering, which holds through the
> winner's release on reset even with a relaxed cmpxchg, and named the
> cmpxchg as its pair when the cmpxchg wasn't an acquire. The acquire is
> for the winner's plain writes of pc/r0/be: nothing else orders them
> after the target's reads. v4 has cmpxchg_acquire() and the two
> comments name what's ordered and each other:
> 
>         /*
>          * Orders pkvm_reset_vcpu()'s accesses to reset_state before OFF. Pairs
>          * with the acquire cmpxchg in pvm_psci_vcpu_on().
>          */
>         smp_store_release(&hyp_vcpu->power_state, PSCI_0_2_AFFINITY_LEVEL_OFF);
> 
> and in pvm_psci_vcpu_on():
> 
>         /*
>          * vCPUs race to power on the same target. The acquire pairs with the
>          * release of OFF in pvm_psci_vcpu_off(): the target's accesses to
>          * reset_state in pkvm_reset_vcpu() precede the writes below.
>          */
>         power_state = cmpxchg_acquire(&target->power_state,
>                                       PSCI_0_2_AFFINITY_LEVEL_OFF,
>                                       PSCI_0_2_AFFINITY_LEVEL_ON_PENDING);

This confused me more :( Why are you talking about accesses preceding an
acquire? An acquire only orders later accesses.

I really think we need some litmus tests to understand the general ordering
problems we have here before adding the memory barriers.

Will