[PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

Hi,

On Fri, Aug 23, 2024 at 12:46:05PM +0100, Will Deacon wrote:
> On Tue, Aug 20, 2024 at 03:38:34PM +0100, Dave Martin wrote:
> > On Tue, Aug 20, 2024 at 02:37:40PM +0100, Will Deacon wrote:
> > > On Mon, Jul 29, 2024 at 03:41:49PM +0100, Dave Martin wrote:
> > > > The table tracking space usage in sigcontext.__reserved[] has got
> > > > a bit out of date.
> > > > 
> > > > Update it, and clarify the opt-in constraints.
> > > > 
> > > > Note, svl <= 64 would be a sufficient condition for keeping the
> > > > sve_context within range when in streaming SVE mode under SME, but
> > > > then za_context gets too big and userspace loses anyway.  To keep
> > > > the conditions simple, just write "svl <= 32" everywhere.
> > > > 
> > > > No functional change.
> > > > 
> > > > Signed-off-by: Dave Martin <Dave.Martin at arm.com>
> > > > 
> > > > ---
> > > > 
> > > > Note, this is a back-of-the-envelope calculation... but whatever way I
> > > > slice it, __reserved[] looks pretty much full (!)
> > > > 
> > > > If Mark in particular can double-check the SME impact, that would be
> > > > appreciated.
> > > > 
> > > > New arch features with a non-trivial amount of state that needs to be
> > > > saved may need to be disabled by default and require explicitly turning
> > > > on by a syscall unless we want to allow some ABI breakage (x86's
> > > > experience suggests that the world takes a long time to explode when
> > > > signal frames outgrow their official size, though).
> > > > 
> > > > Either way, do we need a new strategy to slow down the filling of the
> > > > remaining space?  There is continuing demand on it (see e.g., [1]).
> > > > 
> > > > Migration note: at least glibc since version 2.34 [2] has stopped
> > > > offering compile-time constant signal stack size #defines to programs
> > > > built with -D_GNU_SOURCE. [3]  This should mitigate ABI breaks for
> > > > programs that bother to size stacks correctly.
> > > > 
> > > > I haven't checked what other libcs and runtimes are doing.
> > > > 
> > > > Additional SME note: since userspace can freely switch in and out of
> > > > streaming SVE mode and freely enable/disable ZA and ZT0, I'm assuming
> > > > that none of sve_context, za_context and zt_context are mutually
> > > > exclusive, but please shout if I have confused myself here.
> > > > 
> > > > [1] [PATCH v4 18/29] arm64: add POE signal support
> > > > https://lore.kernel.org/linux-arm-kernel/20240503130147.1154804-19-joey.gouly@arm.com/
> > > > 
> > > > [2] [glibc] glibc-2.34
> > > > https://sourceware.org/git/?p=glibc.git;a=tag;h=9df03063320651bc629fa427eef3ac73fabb61ba
> > > > 
> > > > [3] [glibc] sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305]
> > > > https://sourceware.org/git/?p=glibc.git;a=commit;f=sysdeps/unix/sysv/linux/bits/sigstksz.h;h=6c57d320484988e87e446e2e60ce42816bf51d53
> > > > ---
> > > >  arch/arm64/include/uapi/asm/sigcontext.h | 11 +++++++++--
> > > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
> > > > index 8a45b7a411e0..2cd60fd64e9a 100644
> > > > --- a/arch/arm64/include/uapi/asm/sigcontext.h
> > > > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
> > > > @@ -44,11 +44,18 @@ struct sigcontext {
> > > >   *
> > > >   *	0x210		fpsimd_context
> > > >   *	 0x10		esr_context
> > > > - *	0x8a0		sve_context (vl <= 64) (optional)
> > > > + *	0x8a0		sve_context (vl <= 64 && svl <= 32) (optional)
> > > > + *	 0x10		tpidr2_context (optional)
> > > > + *	0x410		za_context (svl <= 32) (optional)
> > > > + *	 0x50		zt_context (optional)
> > > > + *	 0x10		fpmr_context (optional)
> > > >   *	 0x20		extra_context (optional)
> > > >   *	 0x10		terminator (null _aarch64_ctx)
> > > >   *
> > > > - *	0x510		(reserved for future allocation)
> > > > + *	 0x90		(reserved for future allocation)
> > > > + *
> > > > + * where vl is the non-streaming SVE vector length, as set with PR_SVE_SET_VL,
> > > > + * and svl is the streaming SVE vector length, as set with PR_SME_SET_VL.
> > > 
> > > These are quite fiddly to check by hand, but the ones I *did* check
> > > appear to be correct. The discussion with Mark seems tangential to the
> > > actual diff, so I'm inclined to apply this if nobody objects.
> > > 
> > > Will
> > 
> > I'm wondering whether we should just get rid of this instead, since
> > it's not that maintainable: see [4].
> 
> I suppose we could, but I must confess that I find this comment a lot
> easier to digest that the fiddly maze of inconsistent macros we have
> for the different contexts. Then again, all that really matters, I
> suppose, is that we don't accidentally over-allocate the maximum size
> of the sigcontext. That ought to be enforceable.
> 
> Will

At the moment, we just have a lot of fiddly, hand-hacked contitions in
the sigframe code saying when each record is emitted.  It's not
immediately obvious from those what ought to be accounted in this
table.

Some of it is based on assumptions about how userspace is going to use
the feature, so I'm not sure we can work this out purely mechanically.

I think my best approach for now would be to try to wire up something
that at least helps remind us that we need to review this table when
something new is added in the sigframe, unless you can think of a
better idea.

Cheers
---Dave