From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave.Martin@arm.com (Dave Martin) Date: Thu, 24 May 2018 12:22:42 +0100 Subject: [PATCH v10 13/18] arm64/sve: Move sve_pffr() to fpsimd.h and make inline In-Reply-To: <8736yhtkvo.fsf@linaro.org> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-14-git-send-email-Dave.Martin@arm.com> <8736yhtkvo.fsf@linaro.org> Message-ID: <20180524112242.GU13470@e103592.cambridge.arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, May 24, 2018 at 11:20:59AM +0100, Alex Benn?e wrote: > > Dave Martin writes: > > > In order to make sve_save_state()/sve_load_state() more easily > > reusable and to get rid of a potential branch on context switch > > critical paths, this patch makes sve_pffr() inline and moves it to > > fpsimd.h. > > > > must be included in fpsimd.h in order to make > > this work, and this creates an #include cycle that is tricky to > > avoid without modifying core code, due to the way the PR_SVE_*() > > prctl helpers are included in the core prctl implementation. > > > > Instead of breaking the cycle, this patch defers inclusion of > > in until the point where it is > > actually needed: i.e., immediately before the prctl definitions. > > > > No functional change. > > > > Signed-off-by: Dave Martin > > Acked-by: Catalin Marinas > > Acked-by: Marc Zyngier > > --- > > arch/arm64/include/asm/fpsimd.h | 13 +++++++++++++ > > arch/arm64/include/asm/processor.h | 3 ++- > > arch/arm64/kernel/fpsimd.c | 12 ------------ > > 3 files changed, 15 insertions(+), 13 deletions(-) > > > > diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h > > index fb60b22..fa92747 100644 > > --- a/arch/arm64/include/asm/fpsimd.h > > +++ b/arch/arm64/include/asm/fpsimd.h > > @@ -18,6 +18,8 @@ > > > > #include > > #include > > +#include > > +#include > > > > #ifndef __ASSEMBLY__ > > > > @@ -61,6 +63,17 @@ extern void sve_flush_cpu_state(void); > > /* Maximum VL that SVE VL-agnostic software can transparently support */ > > #define SVE_VL_ARCH_MAX 0x100 > > > > +/* Offset of FFR in the SVE register dump */ > > +static inline size_t sve_ffr_offset(int vl) > > +{ > > + return SVE_SIG_FFR_OFFSET(sve_vq_from_vl(vl)) - SVE_SIG_REGS_OFFSET; > > +} > > + > > +static inline void *sve_pffr(struct thread_struct *thread) > > +{ > > + return (char *)thread->sve_state + sve_ffr_offset(thread->sve_vl); > > +} > > + > > extern void sve_save_state(void *state, u32 *pfpsr); > > extern void sve_load_state(void const *state, u32 const *pfpsr, > > unsigned long vq_minus_1); > > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h > > index f902b6d..ebaadb1 100644 > > --- a/arch/arm64/include/asm/processor.h > > +++ b/arch/arm64/include/asm/processor.h > > @@ -40,7 +40,6 @@ > > > > #include > > #include > > -#include > > #include > > #include > > #include > > @@ -245,6 +244,8 @@ void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused); > > void cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused); > > void cpu_clear_disr(const struct arm64_cpu_capabilities *__unused); > > > > +#include > > + > > You really need a one-liner comment to note why the include is in a > funny place to save someone just moving it back and then getting really > confused. Maybe: > > /* included just in time to avoid circular inclusion issues */ > #include > > It still seems weird to me though :-/ How about /* * Not at the top of the file due to a direct #include cycle between * and . Deferring this #include * ensures that contents of processor.h are visible to fpsimd.h even if * processor.h is included first. * * These prctl helpers are the only things in this file that require * fpsimd.h. The core code expects them to be in this header. */ ? Cheers ---Dave > > Otherwise: > > Reviewed-by: Alex Benn?e