On Tue, Aug 22, 2017 at 05:21:19PM +0100, Alex Benn?e wrote:

Dave Martin [off-list ref] writes:

[...]

quoted

--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h

@@ -85,6 +85,8 @@ struct thread_struct {
 	unsigned long		tp2_value;
 #endif
 	struct fpsimd_state	fpsimd_state;
+	void			*sve_state;	/* SVE registers, if any */
+	u16			sve_vl;		/* SVE vector length */

sve_vl is implicitly cast to unsigned int bellow - it should be
consistent.

Agreed -- I think this can just be unsigned int here, which is the type
I use everywhere except when encoding the vector length in the signal
frame and ptrace data.

During development, there was an additional flags field here (now
merged into the thread_info flags).  u16 was big enough for the largest
architectural vector length, so it seemed "tidy" to make both u16s.
Elsewhere, this created a risk of overflow if you try to compute say
the size of the whole register file from a u16, so I generally used
unsigned int for local variables in functions that handle the vl.

Given the allocation functions rely on sve_vl being valid it might be
worth noting where this is set/live from?

Agreed, I need to look more closely at exactly how to justify the
soundness of this in order to thin out BUG_ONs.

If you need any pointers, please ping me.  In the meantime, I would
rather not colour your judgement by infecting you with my assumptions ;)

quoted

 	unsigned long		fault_address;	/* fault info */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93..1a4b30b 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h

<snip>

And I see there are other comments from Ard.

Yup, I've already picked those up.

I'll be posting a v2 with feedback applied once I've reviewed the
existing BUG_ON()s -- should happen sometime this week.

Cheers
---Dave