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

[Will Deacon <will@kernel.org>]

On Wed, Sep 23, 2026 at 10:51:34AM +0100, Fuad Tabba wrote:
> Hi Vincent,
> 
> > > +/*
> > > + * Returns true when handled at EL2, false when the host must stop scheduling
> > > + * the vCPU.
> > > + */
> > > +static bool pvm_psci_vcpu_off(struct pkvm_hyp_vcpu *hyp_vcpu)
> > > +{
> > > +        /* No other writer runs while this vCPU is ON and executing. */
> > > +        WARN_ON(READ_ONCE(hyp_vcpu->power_state) != PSCI_0_2_AFFINITY_LEVEL_ON);
> > > +
> > > +        /*
> > > +         * Orders pkvm_reset_vcpu()'s clear of reset_state.reset before OFF, so
> > > +         * a CPU_ON that wins on OFF republishes after it. Pairs with the
> > > +         * cmpxchg in pvm_psci_vcpu_on().
> > > +         */
> > > +        smp_store_release(&hyp_vcpu->power_state, PSCI_0_2_AFFINITY_LEVEL_OFF);
> >
> > Is there an issue either with the comment or with pvm_psci_vcpu_on()? the
> > cmpxchg is relaxed. I would have expected cmpxchg_acquire().
> 
> It's the comment. The ordering it describes holds with the relaxed
> cmpxchg. The release of OFF orders the clear before OFF. The winner's
> smp_store_release(&reset_state->reset, true) orders its cmpxchg before
> that store. So the clear can't land after the republish. The comment
> should name that store-release as the pair, not the cmpxchg.

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!

Will