Re: [PATCH 09/27] arm64/sve: Signal frame and context structure definition

["Alex Bennée" <alex.bennee@linaro.org>]

Dave Martin <Dave.Martin@arm.com> writes:

> On Tue, Aug 22, 2017 at 11:22:44AM +0100, Alex Bennée wrote:
>>
>> Dave Martin <Dave.Martin@arm.com> writes:
>>
>> > This patch defines the representation that will be used for the SVE
>> > register state in the signal frame, and implements support for
>> > saving and restoring the SVE registers around signals.
>> >
>> > The same layout will also be used for the in-kernel task state.
>> >
>> > Due to the variability of the SVE vector length, it is not possible
>> > to define a fixed C struct to describe all the registers.  Instead,
>> > Macros are defined in sigcontext.h to facilitate access to the
>> > parts of the structure.
>> >
>> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>> > ---
>> >  arch/arm64/include/uapi/asm/sigcontext.h | 113 ++++++++++++++++++++++++++++++-
>> >  1 file changed, 112 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
>> > index f0a76b9..0533bdf 100644
>> > --- a/arch/arm64/include/uapi/asm/sigcontext.h
>> > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
>> > @@ -16,6 +16,8 @@
>> >  #ifndef _UAPI__ASM_SIGCONTEXT_H
>> >  #define _UAPI__ASM_SIGCONTEXT_H
>> >
>> > +#ifndef __ASSEMBLY__
>> > +
>> >  #include <linux/types.h>
>> >
>> >  /*
>> > @@ -41,10 +43,11 @@ struct sigcontext {
>> >   *
>> >   *	0x210		fpsimd_context
>> >   *	 0x10		esr_context
>> > + *	0x8a0		sve_context (vl <= 64) (optional)
>> >   *	 0x20		extra_context (optional)
>> >   *	 0x10		terminator (null _aarch64_ctx)
>> >   *
>> > - *	0xdb0		(reserved for future allocation)
>> > + *	0x510		(reserved for future allocation)
>> >   *
>> >   * 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
>> > @@ -116,4 +119,112 @@ struct extra_context {
>> >  	__u32 __reserved[3];
>> >  };
>> >
>> > +#define SVE_MAGIC	0x53564501
>> > +
>> > +struct sve_context {
>> > +	struct _aarch64_ctx head;
>> > +	__u16 vl;
>> > +	__u16 __reserved[3];
>> > +};
>> > +
>> > +#endif /* !__ASSEMBLY__ */
>> > +
>> > +/*
>> > + * The SVE architecture leaves space for future expansion of the
>> > + * vector length beyond its initial architectural limit of 2048 bits
>> > + * (16 quadwords).
>> > + */
>> > +#define SVE_VQ_MIN		1
>> > +#define SVE_VQ_MAX		0x200
>> > +
>> > +#define SVE_VL_MIN		(SVE_VQ_MIN * 0x10)
>> > +#define SVE_VL_MAX		(SVE_VQ_MAX * 0x10)
>> > +
>> > +#define SVE_NUM_ZREGS		32
>> > +#define SVE_NUM_PREGS		16
>> > +
>> > +#define sve_vl_valid(vl) \
>> > +	((vl) % 0x10 == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX)
>> > +#define sve_vq_from_vl(vl)	((vl) / 0x10)
>> > +#define sve_vl_from_vq(vq)	((vq) * 0x10)
>>
>> I got a little confused first time through over what VQ and VL where.
>> Maybe it would make sense to expand a little more from first principles?
>>
>>   /*
>>    * The SVE architecture defines vector registers as a multiple of 128
>>    * bit quadwords. The current architectural limit is 2048 bits (16
>>    * quadwords) but there is room for future expansion beyond that.
>>     */
>
> This comes up in several places and so I didn't want to comment it
> repeatedly everywhere.
>
> Instead, I wrote up something in section 2 (Vector length terminology)
> of Documentation/arm64/sve.txt -- see patch 25.  Can you take a look and
> see whether that's adequate?

Ahh, I hadn't got to that yet. I'm unsure to the order the kernel likes
to put things but I like to put design documents at the front of the
patch queue as they are useful primers and saves you having to patch a:

modified   arch/arm64/include/uapi/asm/sigcontext.h
@@ -132,19 +132,24 @@ struct sve_context {
 /*
  * The SVE architecture leaves space for future expansion of the
  * vector length beyond its initial architectural limit of 2048 bits
- * (16 quadwords).
+ * (16 quadwords). See Documentation/arm64/sve.txt for a summary of
+ * the terminology of Vector Quads (VQ) and Vector Lengths (VL).
  */
+
+#define SVE_VQ_BITS             128      /* 128 bits in one quadword */
+#define SVE_VQ_BYTES            (SVE_VQ_BITS / 8)
+
 #define SVE_VQ_MIN		1
 #define SVE_VQ_MAX		0x200

-#define SVE_VL_MIN		(SVE_VQ_MIN * 0x10)
-#define SVE_VL_MAX		(SVE_VQ_MAX * 0x10)
+#define SVE_VL_MIN		(SVE_VQ_MIN * SVE_VQ_BYTES)
+#define SVE_VL_MAX		(SVE_VQ_MAX * SVE_VQ_BYTES)

 #define SVE_NUM_ZREGS		32
 #define SVE_NUM_PREGS		16

 #define sve_vl_valid(vl) \
-	((vl) % 0x10 == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX)
+	((vl) % SVE_VQ_BYTES == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX)
 #define sve_vq_from_vl(vl)	((vl) / 0x10)
 #define sve_vl_from_vq(vq)	((vq) * 0x10)


>
> [...]
>
> Cheers
> ---Dave


--
Alex Bennée