Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752518AbdIVVt6 (ORCPT ); Fri, 22 Sep 2017 17:49:58 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:34064 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752166AbdIVVt5 (ORCPT ); Fri, 22 Sep 2017 17:49:57 -0400 X-Google-Smtp-Source: AOwi7QDXmufPj/hAGw5CetwIIABFUU2cOOxI5SHvRrHHa0Y42ENCn7+27F9y8iFfIZ0WPd315OuZBk76NSQCr7t1ZlI= MIME-Version: 1.0 In-Reply-To: References: <20170920151802.7609-1-romain.izard.pro@gmail.com> From: Arnd Bergmann Date: Fri, 22 Sep 2017 23:49:56 +0200 X-Google-Sender-Auth: c3qr2DR8Wx9UxRrh8of5KnRgEg8 Message-ID: Subject: Re: [PATCH] ARM: unaligned.h: Use an arch-specific version To: Ard Biesheuvel Cc: Romain Izard , Russell King , Al Viro , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2023 Lines: 45 On Fri, Sep 22, 2017 at 11:36 PM, Ard Biesheuvel wrote: > On 20 September 2017 at 13:35, Arnd Bergmann wrote: >> The architectures that do use include/asm-generic/unaligned.h and >> also set HAVE_EFFICIENT_UNALIGNED_ACCESS in some configurations >> are arm, arm64, metag, s390 and arc. >> >> This is a rather short list, and three of them (arm64, metag and arc) only >> support very recent compilers, so we can probably just ask the respective >> arch maintainers to ack the patch that changes the asm-generic file >> for everyone. >> > > If we can limit the fallout like that, I agree that we should simply > make the struct flavor the default. It elegantly informs the compiler > about the size of the access and the potential misalignment, so it > should allow compilers for any architecture to select the most > appropriate instruction. > > But doesn't that mean that any code that currently relies on > HAVE_EFFICIENT_UNALIGNED_ACCESS should be using get_unaligned instead? > I haven't reviewed the actual use cases (other than the ones I added > myself to the crypto subsystem), but it seems to me that it is > generally unsafe to do any unaligned accesses directly on ARM, given > that the compiler may merge adjacent LDRs into LDMs or LDRDs (and > likewise for stores) It's not clear that all that code should be using get_unaligned(), but I agree that any code relying on HAVE_EFFICIENT_UNALIGNED_ACCESS is potentially inefficient on ARM when it causes a trap. I think it would be a useful goal to avoid running into the ARM alignment trap handler entirely for kernel code, but that sounds like a lot of work. If we want to do that, we'd need at least these steps: - review each reference to HAVE_EFFICIENT_UNALIGNED_ACCESS and modify it so that gcc will never use the trapping instructions - add a WARN_ON_ONCE() in the trap handler - fix any drivers we run into that should be using get_unaligned() but blindly rely on the trap handler instead. Arnd