[PATCH] arm64: signal: Update sigcontext reservations table

[PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

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@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.
  *
  * New records that can exceed this space need to be opt-in for userspace, so
  * that an expanded signal frame is not generated unexpectedly.  The mechanism

base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
-- 
2.34.1

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

On Mon, Jul 29, 2024 at 03:41:49PM +0100, Dave Martin wrote:

> If Mark in particular can double-check the SME impact, that would be
> appreciated.

...

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

There's no mutual exclusion here, but since we only generate the data
payloads if userspace explicitly chooses to enter streaming mode or
enable ZA/ZT0 I would tend to class any VL dependent size there as opt
in and only include the base structs.  I'm not clear what your thinking
is with specifying them for some vector lengths.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

On Mon, Jul 29, 2024 at 03:53:12PM +0100, Mark Brown wrote:
> On Mon, Jul 29, 2024 at 03:41:49PM +0100, Dave Martin wrote:
> 
> > If Mark in particular can double-check the SME impact, that would be
> > appreciated.
> 
> ...
> 
> > 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.
> 
> There's no mutual exclusion here, but since we only generate the data
> payloads if userspace explicitly chooses to enter streaming mode or
> enable ZA/ZT0 I would tend to class any VL dependent size there as opt
> in and only include the base structs.  I'm not clear what your thinking
> is with specifying them for some vector lengths.

OK

The basic test would be: can non large-sigframe-aware program blow up
if ld.so links it against a library that uses SME internally?

There is no absolute guarantee here, but firstly well-behaved libraries
either shouldn't mess with the vector length or should block signals
around critical sections (or create worker threads that block all
signals), and secondly a paranoid program could preempt the prctl()
function to prevent the vector length being changed.

There is no way to prevent SM/ZA twiddling though, IIUC.  Within the
AArch64 application programmer's model and PCS rules, user code should
be able to do whatever it likes without worrying about breaking other
code.

So a program must be prepared to accept the largest possible SME
sigframe, given the current streaming SME vector length.

Ditto all other features that can be used without an explicit call to
enable (or fatten) them.

Cheers
---Dave

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

On Mon, Jul 29, 2024 at 04:51:27PM +0100, Dave Martin wrote:
> On Mon, Jul 29, 2024 at 03:53:12PM +0100, Mark Brown wrote:

> > There's no mutual exclusion here, but since we only generate the data
> > payloads if userspace explicitly chooses to enter streaming mode or
> > enable ZA/ZT0 I would tend to class any VL dependent size there as opt
> > in and only include the base structs.  I'm not clear what your thinking
> > is with specifying them for some vector lengths.

> The basic test would be: can non large-sigframe-aware program blow up
> if ld.so links it against a library that uses SME internally?

> There is no absolute guarantee here, but firstly well-behaved libraries
> either shouldn't mess with the vector length or should block signals
> around critical sections (or create worker threads that block all
> signals), and secondly a paranoid program could preempt the prctl()
> function to prevent the vector length being changed.

I think anything that goes and fiddles with the vector length
dynamically from a library is sufficiently adventurous that it's kind of
out of scope here.

> There is no way to prevent SM/ZA twiddling though, IIUC.  Within the
> AArch64 application programmer's model and PCS rules, user code should
> be able to do whatever it likes without worrying about breaking other
> code.

> So a program must be prepared to accept the largest possible SME
> sigframe, given the current streaming SME vector length.

> Ditto all other features that can be used without an explicit call to
> enable (or fatten) them.

Hrm, indeed.  I think while you're at clarifying this it'd be good to
clarify what we're thinking of as opting in - is it userspace as a whole
we're thinking of or is it a specific dynamically linked binary?
There's also things like the dynamic linker and code generation options
in the compiler to worry about here...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

Hi,

On Mon, Jul 29, 2024 at 06:01:02PM +0100, Mark Brown wrote:
> On Mon, Jul 29, 2024 at 04:51:27PM +0100, Dave Martin wrote:
> > On Mon, Jul 29, 2024 at 03:53:12PM +0100, Mark Brown wrote:
> 
> > > There's no mutual exclusion here, but since we only generate the data
> > > payloads if userspace explicitly chooses to enter streaming mode or
> > > enable ZA/ZT0 I would tend to class any VL dependent size there as opt
> > > in and only include the base structs.  I'm not clear what your thinking
> > > is with specifying them for some vector lengths.
> 
> > The basic test would be: can non large-sigframe-aware program blow up
> > if ld.so links it against a library that uses SME internally?
> 
> > There is no absolute guarantee here, but firstly well-behaved libraries
> > either shouldn't mess with the vector length or should block signals
> > around critical sections (or create worker threads that block all
> > signals), and secondly a paranoid program could preempt the prctl()
> > function to prevent the vector length being changed.
> 
> I think anything that goes and fiddles with the vector length
> dynamically from a library is sufficiently adventurous that it's kind of
> out of scope here.

Yes.  There are ways to do it (mostly) safely, but it's fiddly, and it
seemed too hard to make promises that would be useful to general
purpose userspace code.

> > There is no way to prevent SM/ZA twiddling though, IIUC.  Within the
> > AArch64 application programmer's model and PCS rules, user code should
> > be able to do whatever it likes without worrying about breaking other
> > code.
> 
> > So a program must be prepared to accept the largest possible SME
> > sigframe, given the current streaming SME vector length.
> 
> > Ditto all other features that can be used without an explicit call to
> > enable (or fatten) them.
> 
> Hrm, indeed.  I think while you're at clarifying this it'd be good to
> clarify what we're thinking of as opting in - is it userspace as a whole
> we're thinking of or is it a specific dynamically linked binary?
> There's also things like the dynamic linker and code generation options
> in the compiler to worry about here...

I think that the opt-in has to be per running process.

How userspace decides whether to opt in is outside the scope of the
kernel, so the original SVE design at least tried to make things safe
by default, using the sve_default_vector_length clamp.

Since making the opt-in decision correctly requires some effort, I
expected that most programs just won't bother and won't opt in unless
they actually need a given feature in order to work at all.


In the future, distros might consider the codebase fully "large
sigframe aware", and increase sve_default_vector_length so that all
processes are opted in without having to do anything; that amounts to
a system-level opt-in.

Similarly for SME.


Ideally, the toolchain would mark binaries with the features they are
compatible with, and try to load only compatible objects into the same
process.  The ELF properties (as used for BTI etc.) provide a generic
mechanism for this, but maybe we need to start pushing for labelling
for other properties too.  The "can it trigger an oversized sigframe"
property of an arch feature won't be obvious to the toolchain folks.

If some feature needs a prctl() to enable it when first upstreamed,
then it's probably best to leave it up to ld.so and the libc startup
code to decide whether and when to call it.  Even if the kernel some
day can do this automatically based on ELF properties or something,
we'd still need to be backwards compatible.  If the "turn on" call
requires parameters, then something in userspace would need to use
it explicitly anyway.

Not sure whether any of this simplifies the discussion...

Cheers
---Dave

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

On Tue, Jul 30, 2024 at 01:51:22PM +0100, Dave Martin wrote:
> On Mon, Jul 29, 2024 at 06:01:02PM +0100, Mark Brown wrote:

> > Hrm, indeed.  I think while you're at clarifying this it'd be good to
> > clarify what we're thinking of as opting in - is it userspace as a whole
> > we're thinking of or is it a specific dynamically linked binary?
> > There's also things like the dynamic linker and code generation options
> > in the compiler to worry about here...

> I think that the opt-in has to be per running process.

Yes, I tend to agree.

> Since making the opt-in decision correctly requires some effort, I
> expected that most programs just won't bother and won't opt in unless
> they actually need a given feature in order to work at all.

Well, it only requires thought if you do something that pays attention
to the signal frame layout - an awful lot of programs simply don't look
at the frame and so don't care.  There are things like userspace threads
which are particularly likely to be impacted but there's also a lot of
code that just handles a signal and returns without ever looking at the
frame.

> Ideally, the toolchain would mark binaries with the features they are
> compatible with, and try to load only compatible objects into the same
> process.  The ELF properties (as used for BTI etc.) provide a generic
> mechanism for this, but maybe we need to start pushing for labelling
> for other properties too.  The "can it trigger an oversized sigframe"
> property of an arch feature won't be obvious to the toolchain folks.

Hrm.  I can see this being fun with working out how the various
extensions compose with each other and how to turn things that the
toolchain usually wouldn't be aware of on.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

On Tue, Jul 30, 2024 at 02:22:47PM +0100, Mark Brown wrote:
> On Tue, Jul 30, 2024 at 01:51:22PM +0100, Dave Martin wrote:
> > On Mon, Jul 29, 2024 at 06:01:02PM +0100, Mark Brown wrote:
> 
> > > Hrm, indeed.  I think while you're at clarifying this it'd be good to
> > > clarify what we're thinking of as opting in - is it userspace as a whole
> > > we're thinking of or is it a specific dynamically linked binary?
> > > There's also things like the dynamic linker and code generation options
> > > in the compiler to worry about here...
> 
> > I think that the opt-in has to be per running process.
> 
> Yes, I tend to agree.
> 
> > Since making the opt-in decision correctly requires some effort, I
> > expected that most programs just won't bother and won't opt in unless
> > they actually need a given feature in order to work at all.
> 
> Well, it only requires thought if you do something that pays attention
> to the signal frame layout - an awful lot of programs simply don't look
> at the frame and so don't care.  There are things like userspace threads
> which are particularly likely to be impacted but there's also a lot of
> code that just handles a signal and returns without ever looking at the
> frame.

A program can't not pay attention to the sigframe _size_, i.e., even if
you ignore the sigcontext, you still have to have allocated your stack
big enough for it.

That's the fundamental issue here.

> > Ideally, the toolchain would mark binaries with the features they are
> > compatible with, and try to load only compatible objects into the same
> > process.  The ELF properties (as used for BTI etc.) provide a generic
> > mechanism for this, but maybe we need to start pushing for labelling
> > for other properties too.  The "can it trigger an oversized sigframe"
> > property of an arch feature won't be obvious to the toolchain folks.
> 
> Hrm.  I can see this being fun with working out how the various
> extensions compose with each other and how to turn things that the
> toolchain usually wouldn't be aware of on.

That's why I went for a simplified model:

If a program exercises no opt-ins at all, then the sigframe must fit in
MINSIGSTKSZ bytes.

If the program exercises any opt-in at all, the sigframe is not
guaranteed to fit in MINSIGSTKSZ bytes.  It's then the program's
responsibility to pay attention to the real worst-case size advertised
in AT_MINSIGSTKSZ in the auxv.

As noted in the references, programs built against glibc-2.34 or later
with -D_GNU_SOURCE (or -D_DYNAMIC_STACK_SIZE_SOURCE) will actually be
using values based on the AT_MINSIGSTKSZ parameter rather than the old
constant; uses of MINSIGSTKSZ and SIGSTKSZ that require it to be
compile-time constant won't compile.


The idea of the table in sigcontext.h was to help us track where opt-
ins are needed, and what opt-in conditions exist.  This maybe wasn't as
clear as it could have been.

Cheers
---Dave

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2872 bytes --]

On Tue, Jul 30, 2024 at 04:07:22PM +0100, Dave Martin wrote:
> On Tue, Jul 30, 2024 at 02:22:47PM +0100, Mark Brown wrote:

> > Well, it only requires thought if you do something that pays attention
> > to the signal frame layout - an awful lot of programs simply don't look
> > at the frame and so don't care.  There are things like userspace threads
> > which are particularly likely to be impacted but there's also a lot of
> > code that just handles a signal and returns without ever looking at the
> > frame.

> A program can't not pay attention to the sigframe _size_, i.e., even if
> you ignore the sigcontext, you still have to have allocated your stack
> big enough for it.

> That's the fundamental issue here.

A good percentage of programs manage to just use a default rather then
ever explicitly specifying or configuring anything themselves - C
programs will default to RLIMIT_STACK IIRC which is system configured
and generally set rather high.  It's true that anything that is
explicitly configuring stack sizes needs to worry about having enough
stack space for a signal frame on top of whatever else it's doing
(including anything limiting things system wide) but I'd be a bit
surprised if it were the common case that things were actually paying
attention.

> > > Ideally, the toolchain would mark binaries with the features they are
> > > compatible with, and try to load only compatible objects into the same
> > > process.  The ELF properties (as used for BTI etc.) provide a generic
> > > mechanism for this, but maybe we need to start pushing for labelling
> > > for other properties too.  The "can it trigger an oversized sigframe"
> > > property of an arch feature won't be obvious to the toolchain folks.

> > Hrm.  I can see this being fun with working out how the various
> > extensions compose with each other and how to turn things that the
> > toolchain usually wouldn't be aware of on.

> That's why I went for a simplified model:

> If a program exercises no opt-ins at all, then the sigframe must fit in
> MINSIGSTKSZ bytes.

> If the program exercises any opt-in at all, the sigframe is not
> guaranteed to fit in MINSIGSTKSZ bytes.  It's then the program's
> responsibility to pay attention to the real worst-case size advertised
> in AT_MINSIGSTKSZ in the auxv.

> As noted in the references, programs built against glibc-2.34 or later
> with -D_GNU_SOURCE (or -D_DYNAMIC_STACK_SIZE_SOURCE) will actually be
> using values based on the AT_MINSIGSTKSZ parameter rather than the old
> constant; uses of MINSIGSTKSZ and SIGSTKSZ that require it to be
> compile-time constant won't compile.

> The idea of the table in sigcontext.h was to help us track where opt-
> ins are needed, and what opt-in conditions exist.  This maybe wasn't as
> clear as it could have been.

I think some of it is the strength of the opt ins being considered.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

On Tue, Jul 30, 2024 at 05:00:17PM +0100, Mark Brown wrote:
> On Tue, Jul 30, 2024 at 04:07:22PM +0100, Dave Martin wrote:
> > On Tue, Jul 30, 2024 at 02:22:47PM +0100, Mark Brown wrote:
> 
> > > Well, it only requires thought if you do something that pays attention
> > > to the signal frame layout - an awful lot of programs simply don't look
> > > at the frame and so don't care.  There are things like userspace threads
> > > which are particularly likely to be impacted but there's also a lot of
> > > code that just handles a signal and returns without ever looking at the
> > > frame.
> 
> > A program can't not pay attention to the sigframe _size_, i.e., even if
> > you ignore the sigcontext, you still have to have allocated your stack
> > big enough for it.
> 
> > That's the fundamental issue here.
> 
> A good percentage of programs manage to just use a default rather then
> ever explicitly specifying or configuring anything themselves - C
> programs will default to RLIMIT_STACK IIRC which is system configured
> and generally set rather high.  It's true that anything that is
> explicitly configuring stack sizes needs to worry about having enough
> stack space for a signal frame on top of whatever else it's doing
> (including anything limiting things system wide) but I'd be a bit
> surprised if it were the common case that things were actually paying
> attention.

That's all true, but even programs that don't explicity work out stack
sizes may be using implicit knowledge, beacuse the developers may have
simple bumped up stack sizes.

Note, RLIMIT_STACK only applies the initial stack of the main thread.
Processes with threads might have many stacks, as might processes with
fibers/coroutines (allocated any old how, and often with no reference
to RLIMIT_STACK).

The aim here is to minimise surprises for code that made reasonable
assumptions at the time it was written, rather than to ensure that
every ancient binary that ever worked by accident still works, no
matter what crazy nonportable shenanigans it gets up to.

> 
> > > > Ideally, the toolchain would mark binaries with the features they are
> > > > compatible with, and try to load only compatible objects into the same
> > > > process.  The ELF properties (as used for BTI etc.) provide a generic
> > > > mechanism for this, but maybe we need to start pushing for labelling
> > > > for other properties too.  The "can it trigger an oversized sigframe"
> > > > property of an arch feature won't be obvious to the toolchain folks.
> 
> > > Hrm.  I can see this being fun with working out how the various
> > > extensions compose with each other and how to turn things that the
> > > toolchain usually wouldn't be aware of on.
> 
> > That's why I went for a simplified model:
> 
> > If a program exercises no opt-ins at all, then the sigframe must fit in
> > MINSIGSTKSZ bytes.
> 
> > If the program exercises any opt-in at all, the sigframe is not
> > guaranteed to fit in MINSIGSTKSZ bytes.  It's then the program's
> > responsibility to pay attention to the real worst-case size advertised
> > in AT_MINSIGSTKSZ in the auxv.
> 
> > As noted in the references, programs built against glibc-2.34 or later
> > with -D_GNU_SOURCE (or -D_DYNAMIC_STACK_SIZE_SOURCE) will actually be
> > using values based on the AT_MINSIGSTKSZ parameter rather than the old
> > constant; uses of MINSIGSTKSZ and SIGSTKSZ that require it to be
> > compile-time constant won't compile.
> 
> > The idea of the table in sigcontext.h was to help us track where opt-
> > ins are needed, and what opt-in conditions exist.  This maybe wasn't as
> > clear as it could have been.
> 
> I think some of it is the strength of the opt ins being considered.

Sorry, what do you mean here?

Cheers
---Dave

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2302 bytes --]

On Wed, Jul 31, 2024 at 11:38:55AM +0100, Dave Martin wrote:
> On Tue, Jul 30, 2024 at 05:00:17PM +0100, Mark Brown wrote:

> > A good percentage of programs manage to just use a default rather then
> > ever explicitly specifying or configuring anything themselves - C
> > programs will default to RLIMIT_STACK IIRC which is system configured
> > and generally set rather high.  It's true that anything that is

> That's all true, but even programs that don't explicity work out stack
> sizes may be using implicit knowledge, beacuse the developers may have
> simple bumped up stack sizes.

Sure, there will be problems in some places but poking around I'm seeing
defaults that tend more to the generous sizes which is consistent with
the x86 experience here.

> Note, RLIMIT_STACK only applies the initial stack of the main thread.
> Processes with threads might have many stacks, as might processes with
> fibers/coroutines (allocated any old how, and often with no reference
> to RLIMIT_STACK).

It's the default for pthread_create() too - dunno about any common
libraries for light threads, they're definitely the sort of thing where
I'd more expect to find code that actually does anything explicit about
the stack size.

> The aim here is to minimise surprises for code that made reasonable
> assumptions at the time it was written, rather than to ensure that
> every ancient binary that ever worked by accident still works, no
> matter what crazy nonportable shenanigans it gets up to.

Indeed, my suspicion is that the problem is more limited than it might
be - things that either look at the signal frame or not only explicitly
configure stack sizes, but also explicitly configure small stack sizes.

> > > The idea of the table in sigcontext.h was to help us track where opt-
> > > ins are needed, and what opt-in conditions exist.  This maybe wasn't as
> > > clear as it could have been.

> > I think some of it is the strength of the opt ins being considered.

> Sorry, what do you mean here?

The comments in the header just mention that something chooses to use
the extension, they don't highlight the idea that it's considering the
worst case where some random library somewhere in an executable uses an
extension - it's a bit vauge about what specifically is expected to be
considered.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

On Wed, Jul 31, 2024 at 12:49:39PM +0100, Mark Brown wrote:
> On Wed, Jul 31, 2024 at 11:38:55AM +0100, Dave Martin wrote:
> > On Tue, Jul 30, 2024 at 05:00:17PM +0100, Mark Brown wrote:
> 
> > > A good percentage of programs manage to just use a default rather then
> > > ever explicitly specifying or configuring anything themselves - C
> > > programs will default to RLIMIT_STACK IIRC which is system configured
> > > and generally set rather high.  It's true that anything that is
> 
> > That's all true, but even programs that don't explicity work out stack
> > sizes may be using implicit knowledge, beacuse the developers may have
> > simple bumped up stack sizes.
> 
> Sure, there will be problems in some places but poking around I'm seeing
> defaults that tend more to the generous sizes which is consistent with
> the x86 experience here.
> 
> > Note, RLIMIT_STACK only applies the initial stack of the main thread.
> > Processes with threads might have many stacks, as might processes with
> > fibers/coroutines (allocated any old how, and often with no reference
> > to RLIMIT_STACK).
> 
> It's the default for pthread_create() too - dunno about any common
> libraries for light threads, they're definitely the sort of thing where
> I'd more expect to find code that actually does anything explicit about
> the stack size.
> 
> > The aim here is to minimise surprises for code that made reasonable
> > assumptions at the time it was written, rather than to ensure that
> > every ancient binary that ever worked by accident still works, no
> > matter what crazy nonportable shenanigans it gets up to.
> 
> Indeed, my suspicion is that the problem is more limited than it might
> be - things that either look at the signal frame or not only explicitly
> configure stack sizes, but also explicitly configure small stack sizes.

Sure, it's rare, and we expect it to be rare; and defaults for stack
allocation are indeed usually fairly generous.

> > > > The idea of the table in sigcontext.h was to help us track where opt-
> > > > ins are needed, and what opt-in conditions exist.  This maybe wasn't as
> > > > clear as it could have been.
> 
> > > I think some of it is the strength of the opt ins being considered.
> 
> > Sorry, what do you mean here?
> 
> The comments in the header just mention that something chooses to use
> the extension, they don't highlight the idea that it's considering the
> worst case where some random library somewhere in an executable uses an
> extension - it's a bit vauge about what specifically is expected to be
> considered.

I guess I didn't want to over-promise re. guarantees.

This table was planted here partly as a reminder that there is a
problem coming down the tracks here that requires thinking about...


Can we refocus this discussion?

For the patch itself, can you say whether the proposed update is right
or wrong, and whether you think we need to document this better and/or
change the approach?

I'm not committed to any particular way forward, but it feels like we
shouldn't just let this ABI break sail past without making a decision
about how to manage it.

Cheers
---Dave

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

On Wed, Jul 31, 2024 at 03:38:40PM +0100, Dave Martin wrote:

> For the patch itself, can you say whether the proposed update is right
> or wrong, and whether you think we need to document this better and/or
> change the approach?

I haven't double checked the sizeofs but it covers all the things in the
frame and like I said before there's no exclusion between PSTATE.{SM,ZA}.
I do think this should be clearer about what it's supposed to be
tracking.

+ * 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.

I'd just say VL is the vector length, that's the term the architecture
uses and it says it's set with PR_SVE_SET_VL to clarify.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

On Wed, Jul 31, 2024 at 03:58:00PM +0100, Mark Brown wrote:
> On Wed, Jul 31, 2024 at 03:38:40PM +0100, Dave Martin wrote:
> 
> > For the patch itself, can you say whether the proposed update is right
> > or wrong, and whether you think we need to document this better and/or
> > change the approach?
> 
> I haven't double checked the sizeofs but it covers all the things in the
> frame and like I said before there's no exclusion between PSTATE.{SM,ZA}.
> I do think this should be clearer about what it's supposed to be
> tracking.
> 
> + * 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.
> 
> I'd just say VL is the vector length, that's the term the architecture
> uses and it says it's set with PR_SVE_SET_VL to clarify.

It's the worst-case sigframe size that we care about here, regardless
of what code a signal is delivered in the middle of.  Surely that
depends on both vector length settings?

PSTATE.SM and .ZA can be twiddled on and off in userspace by the
compiler IIUC; other translations units aren't supposed to care (or
notice), so we can't know ahead of time which vector length setting
will be used when generting the sigframe.  User code allocating the
stack must assume the worst.

Can you think of a description that clarifies this?

Cheers
---Dave

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Wed, Jul 31, 2024 at 05:09:36PM +0100, Dave Martin wrote:
> On Wed, Jul 31, 2024 at 03:58:00PM +0100, Mark Brown wrote:

> > + * 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.

> > I'd just say VL is the vector length, that's the term the architecture
> > uses and it says it's set with PR_SVE_SET_VL to clarify.

> It's the worst-case sigframe size that we care about here, regardless
> of what code a signal is delivered in the middle of.  Surely that
> depends on both vector length settings?

Sure - what I'm saying is that you should refer to the non-streaming
vector length as just the vector length in line with the architecture
terminology.  The architecture does refer to the streaming vector length
as the streaming SVE vector length sometimes so I guess we can stick
with that there.

> Can you think of a description that clarifies this?

s/non-streaming SVE vector length/vector length/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

On Wed, Jul 31, 2024 at 05:14:46PM +0100, Mark Brown wrote:
> On Wed, Jul 31, 2024 at 05:09:36PM +0100, Dave Martin wrote:
> > On Wed, Jul 31, 2024 at 03:58:00PM +0100, Mark Brown wrote:
> 
> > > + * 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.
> 
> > > I'd just say VL is the vector length, that's the term the architecture
> > > uses and it says it's set with PR_SVE_SET_VL to clarify.
> 
> > It's the worst-case sigframe size that we care about here, regardless
> > of what code a signal is delivered in the middle of.  Surely that
> > depends on both vector length settings?
> 
> Sure - what I'm saying is that you should refer to the non-streaming
> vector length as just the vector length in line with the architecture
> terminology.  The architecture does refer to the streaming vector length
> as the streaming SVE vector length sometimes so I guess we can stick
> with that there.
> 
> > Can you think of a description that clarifies this?
> 
> s/non-streaming SVE vector length/vector length/

Oh, I thought you were commenting on the pair of statements making an
unnecessary distinction, rather than just commenting on the wording
for PR_SVE_SET_VL.
 
I thought that "non-streaming SVE vector length" might be less
ambiguous given that there was no SME when I originally worded this.
I have no problem with just saying "vector length" to refer to this
setting though.

You're right that mentioning PR_SVE_SET_VL should make it unambiguous
anyway.

Cheers
---Dave

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Will Deacon]

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@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

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

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@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].

The discussion re POR_EL0 has moved in the direction of probably
dumping this register unconditionally. [5]  Ideally we wouldn't, but
there's no way to know in advance whether POR_EL0 counts as "in use",
or to anticipate whether a signal handler might want to alter it.

Trapping POR_EL0 until something writes it or a pkey_foo() syscall is
called is an option, but this feels like overkill.

We could also come up with a way for a signal handler to hang extra
stuff off the sigframe to be restored by sigreturn even when there's no
space to allocate another record, but again, the complexity does not
feel well justified until somebody shouts about it.

I can propose something if you think it's worth discussing.

Cheers
---Dave

[4] https://lore.kernel.org/linux-arm-kernel/Zr4aJqc%2FifRXJQAd@e133380.arm.com/

[5] https://lore.kernel.org/linux-arm-kernel/ZsSgKl2JINjdpuW1@e133380.arm.com/

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Will Deacon]

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@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

Re: [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@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

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

On Tue, Sep 03, 2024 at 03:18:25PM +0100, Dave Martin wrote:
> On Fri, Aug 23, 2024 at 12:46:05PM +0100, Will Deacon wrote:

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

...

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

It be good to add a selftest that flags this, that way people might
notice when adding things and if we miss something it'll probably turn
up in one of the CIs at some point (possibly after it's too late but at
least we'd know).  That'd give us some level of integration test with
whatever libcs and other default software are actually doing, as opposed
to what we think they'll do.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

On Tue, Sep 03, 2024 at 07:26:04PM +0100, Mark Brown wrote:
> On Tue, Sep 03, 2024 at 03:18:25PM +0100, Dave Martin wrote:
> > On Fri, Aug 23, 2024 at 12:46:05PM +0100, Will Deacon wrote:
> 
> > > 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.
> 
> ...
> 
> > 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.
> 
> It be good to add a selftest that flags this, that way people might
> notice when adding things and if we miss something it'll probably turn
> up in one of the CIs at some point (possibly after it's too late but at
> least we'd know).  That'd give us some level of integration test with
> whatever libcs and other default software are actually doing, as opposed
> to what we think they'll do.

I suppose we could write a test that sets VL=64, SVL=32 and dirties the
SVE and SME regs before triggering a signal, then checks that
extra_context is not there.  This will only work if SVE and SME are
present and big enough.  If we can run this as a routine CI test on a
model, it might be useful though.

Cheers
---Dave

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On Wed, Sep 04, 2024 at 12:24:24PM +0100, Dave Martin wrote:
> On Tue, Sep 03, 2024 at 07:26:04PM +0100, Mark Brown wrote:

> > It be good to add a selftest that flags this, that way people might
> > notice when adding things and if we miss something it'll probably turn
> > up in one of the CIs at some point (possibly after it's too late but at
> > least we'd know).  That'd give us some level of integration test with
> > whatever libcs and other default software are actually doing, as opposed
> > to what we think they'll do.

> I suppose we could write a test that sets VL=64, SVL=32 and dirties the
> SVE and SME regs before triggering a signal, then checks that
> extra_context is not there.  This will only work if SVE and SME are
> present and big enough.  If we can run this as a routine CI test on a
> model, it might be useful though.

I'd also run this test with the default settings for the system, it
might help people notice if they've configured their system in some
way that causes issues unexpectedly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: signal: Update sigcontext reservations table

[Dave Martin]

On Wed, Sep 04, 2024 at 12:49:03PM +0100, Mark Brown wrote:
> On Wed, Sep 04, 2024 at 12:24:24PM +0100, Dave Martin wrote:
> > On Tue, Sep 03, 2024 at 07:26:04PM +0100, Mark Brown wrote:
> 
> > > It be good to add a selftest that flags this, that way people might
> > > notice when adding things and if we miss something it'll probably turn
> > > up in one of the CIs at some point (possibly after it's too late but at
> > > least we'd know).  That'd give us some level of integration test with
> > > whatever libcs and other default software are actually doing, as opposed
> > > to what we think they'll do.
> 
> > I suppose we could write a test that sets VL=64, SVL=32 and dirties the
> > SVE and SME regs before triggering a signal, then checks that
> > extra_context is not there.  This will only work if SVE and SME are
> > present and big enough.  If we can run this as a routine CI test on a
> > model, it might be useful though.
> 
> I'd also run this test with the default settings for the system, it
> might help people notice if they've configured their system in some
> way that causes issues unexpectedly.

Probably; though if they've increased the sve_default_vector_length or
sme_default_vector_length, they're on their own!

Having a test that flags this up probably no bad thing though.

Cheers
---Dave