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

[Fuad Tabba]

Hi Will,

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. 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, 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.
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.

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

Android has neither end (a WRITE_ONCE() of OFF and a relaxed cmpxchg),
so it has the lost flag above and the data race on pc/r0/be. Both stay
within the guest's own target vCPU, so I haven't treated it as urgent.
I'm working on a fix for Android separately.

Cheers,
/fuad