From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752737AbdJTUWN (ORCPT ); Fri, 20 Oct 2017 16:22:13 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:53632 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751498AbdJTUWM (ORCPT ); Fri, 20 Oct 2017 16:22:12 -0400 X-Google-Smtp-Source: ABhQp+RQIZagXYHQFjfppUhLRwgziWL8gXv2A1NTfbkJ4oz4bQeSmbl8CGeJIkpsXARLlBnoAjkCkNOwEx69+5qEsug= MIME-Version: 1.0 In-Reply-To: <20171020200231.1355569-1-arnd@arndb.de> References: <20171020200231.1355569-1-arnd@arndb.de> From: Ard Biesheuvel Date: Fri, 20 Oct 2017 21:22:11 +0100 Message-ID: Subject: Re: [PATCH] ARM: add a private asm/unaligned.h To: Arnd Bergmann Cc: Russell King , Romain Izard , Sven Schmidt <4sschmid@informatik.uni-hamburg.de>, LKML , "linux-arm-kernel@lists.infradead.org" , Gregory CLEMENT , Petr Cvek , Aaro Koskinen , Andrea Adami , Robert Jarzmik Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 20 October 2017 at 21:01, Arnd Bergmann wrote: > The asm-generic/unaligned.h header provides two different implementations > for accessing unaligned variables: the access_ok.h version used when > CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set pretends that all pointers > are in fact aligned, while the le_struct.h version convinces gcc that the > alignment of a pointer is '1', to make it issue the correct load/store > instructions depending on the architecture flags. > > On ARMv5 and older, we always use the second version, to let the compiler > use byte accesses. On ARMv6 and newer, we currently use the access_ok.h > version, so the compiler can use any instruction including stm/ldm and > ldrd/strd that will cause an alignment trap. This trap can significantly > impact performance when we have to do a lot of fixups and, worse, has > led to crashes in the LZ4 decompressor code that does not have a trap > handler. > > This adds an ARM specific version of asm/unaligned.h that uses the > le_struct.h/be_struct.h implementation unconditionally. This should lead > to essentially the same code on ARMv6+ as before, with the exception of > using regular load/store instructions instead of the trapping instructions > multi-register variants. > > The crash in the LZ4 decompressor code was probably introduced by the > patch replacing the LZ4 implementation, commit 4e1a33b105dd ("lib: update > LZ4 compressor module"), so linux-4.11 and higher would be affected most. > However, we probably want to have this backported to all older stable > kernels as well, to help with the performance issues. > > There are two follow-ups that I think we should also work on, but not > backport to stable kernels, first to change the asm-generic version of > the header to remove the ARM special case, and second to review all > other uses of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS to see if they > might be affected by the same problem on ARM. > > Cc: stable@vger.kernel.org > Signed-off-by: Arnd Bergmann Acked-by: Ard Biesheuvel > --- > Untested so far, please verify that this fixes all the known problems > with the alignment traps. > --- > arch/arm/include/asm/Kbuild | 1 - > arch/arm/include/asm/unaligned.h | 27 +++++++++++++++++++++++++++ > 2 files changed, 27 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/include/asm/unaligned.h > > diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild > index 721ab5ecfb9b..0f2c8a2a8131 100644 > --- a/arch/arm/include/asm/Kbuild > +++ b/arch/arm/include/asm/Kbuild > @@ -20,7 +20,6 @@ generic-y += simd.h > generic-y += sizes.h > generic-y += timex.h > generic-y += trace_clock.h > -generic-y += unaligned.h > > generated-y += mach-types.h > generated-y += unistd-nr.h > diff --git a/arch/arm/include/asm/unaligned.h b/arch/arm/include/asm/unaligned.h > new file mode 100644 > index 000000000000..ab905ffcf193 > --- /dev/null > +++ b/arch/arm/include/asm/unaligned.h > @@ -0,0 +1,27 @@ > +#ifndef __ASM_ARM_UNALIGNED_H > +#define __ASM_ARM_UNALIGNED_H > + > +/* > + * We generally want to set CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS on ARMv6+, > + * but we don't want to use linux/unaligned/access_ok.h since that can lead > + * to traps on unaligned stm/ldm or strd/ldrd. > + */ > +#include > + > +#if defined(__LITTLE_ENDIAN) > +# include > +# include > +# include > +# define get_unaligned __get_unaligned_le > +# define put_unaligned __put_unaligned_le > +#elif defined(__BIG_ENDIAN) > +# include > +# include > +# include > +# define get_unaligned __get_unaligned_be > +# define put_unaligned __put_unaligned_be > +#else > +# error need to define endianess > +#endif > + > +#endif /* __ASM_ARM_UNALIGNED_H */ > -- > 2.9.0 >