This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 11/27] arm64/sve: Core task context handling
- From: Dave Martin <Dave dot Martin at arm dot com>
- To: Alex Bennée <alex dot bennee at linaro dot org>
- Cc: linux-arch at vger dot kernel dot org, libc-alpha at sourceware dot org, Ard Biesheuvel <ard dot biesheuvel at linaro dot org>, Szabolcs Nagy <szabolcs dot nagy at arm dot com>, Catalin Marinas <catalin dot marinas at arm dot com>, Will Deacon <will dot deacon at arm dot com>, Richard Sandiford <richard dot sandiford at arm dot com>, kvmarm at lists dot cs dot columbia dot edu, linux-arm-kernel at lists dot infradead dot org
- Date: Tue, 22 Aug 2017 18:19:59 +0100
- Subject: Re: [PATCH 11/27] arm64/sve: Core task context handling
- Authentication-results: sourceware.org; auth=none
- References: <1502280338-23002-1-git-send-email-Dave.Martin@arm.com> <1502280338-23002-12-git-send-email-Dave.Martin@arm.com> <87378jbmkg.fsf@linaro.org>
On Tue, Aug 22, 2017 at 05:21:19PM +0100, Alex Bennée wrote:
>
> Dave Martin <Dave.Martin@arm.com> writes:
[...]
> > --- 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 ;)
> > 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