Re: [PATCH v12 02/29] arm64/fpsimd: Update FA64 and ZT0 enables when loading SME state

[Fuad Tabba]

On Sat, 18 Jul 2026 at 01:35, Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> On Thu, Jul 16, 2026 at 04:28:02PM +0100, Mark Brown wrote:
> > On Thu, Jul 16, 2026 at 11:52:36AM +0100, Mark Rutland wrote:
>
> > > >           if (test_thread_flag(TIF_SME)) {
> > > > -                 unsigned long vq = sve_vq_from_vl(sme_vl);
> > > > -                 sysreg_clear_set_s(SYS_SMCR_EL1, SMCR_ELx_LEN, vq - 1);
> > > > +                 sysreg_cond_update_s(SYS_SMCR_EL1, task_smcr(current));
> > > > +                 isb();
> > > >           }
>
> > > What's the ISB for? That mysteriously appeared in v11 without
> > > explanation. It wasn't in the original code, prior versions of the
> > > series, or my suggested rework with the task_smcr() helper.
>
> > > I don't believe it's necessary to add an ISB here.
>
> > I can't remember or figure it out, I'll delete.
>
> Actually I remembered: while SCMR_EL1.LEN is self synchronising the
> "without the need for explict synchronization" wording is not present
> for SMCR_EL1.{FA64,EZT0} and this is no longer explicitly just an update
> of LEN.  It's possible I'm being overly paranoid here, I'll leave the
> isb() and add a comment for the next version.

I went through the ARM ARM (DDI 0487 M.c) on this and I don't think
you're being overly paranoid, I believe the isb() is needed here.

The self-synchronisation wording in the SMCR_EL1 description is
attached to the LEN field only: "An indirect read of SMCR_EL1.LEN
appears to occur in program order relative to a direct write of the
same register, without the need for explicit synchronization." There
is no equivalent wording for FA64 or EZT0, so they fall under the
general rule in D24.1.2.2: "Unless otherwise stated in its System
register definition, a direct write to a System register requires
synchronization before software can rely on the effects of that write
to affect instructions appearing in program order after the write."
Table D24-1 has direct write -> indirect read as "Required".

And task_fpsimd_load() does make indirect reads of both fields before
the next context synchronization event: sme_load_state() executes LDR
ZT0, whose execution at EL1 is trapped when SMCR_EL1.EZT0 is 0, and
when PSTATE.SM is set sve_load_state() executes WRFFR, which is
"illegal when executed in Streaming SVE mode, unless FEAT_SME_FA64 is
implemented and enabled" (from the WRFFR description). The MSR SVCR in
between doesn't provide the synchronization, since SVCR's
self-synchronisation wording covers reads of its own SM and ZA fields
only.

When the write does change FA64 or EZT0 (once KVM can disable them for
a guest, and possibly on the return from suspend path now that
sme_suspend_exit() no longer writes them), K1.2.4 "CONSTRAINED
UNPREDICTABLE behavior due to inadequate context synchronization"
applies without the isb(): "the behavior of the PE is consistent with
the unsynchronized control value being either the old value or the new
value", independently per use. So the ZT0 load or the FFR restore
could take an unexpected SME trap (EC 0x1D) in the kernel, depending
on the implementation.

FWIW the other call sites look consistent with only this one needing
it: in do_sme_acc() nothing ZT0 or FFR gated executes before the ERET,
which is the context synchronization event, and ZCR_EL1 only has LEN
allocated.

Cheers,
/fuad