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

[Fuad Tabba]

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.

You're right about the acquire though, for the plain stores to
reset_state.{pc, r0, be} after the cmpxchg. I'll make it
cmpxchg_acquire() in v4 and reword both comments.

Cheers,
/fuad