From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5F672C6FD1D for ; Fri, 17 Mar 2023 14:02:13 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B4ACD83CB1; Fri, 17 Mar 2023 15:02:10 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="mr7Vbske"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 6DCBD85E36; Fri, 17 Mar 2023 15:02:07 +0100 (CET) Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id E30C480B9B for ; Fri, 17 Mar 2023 15:02:00 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-ed1-x52c.google.com with SMTP id o12so20718829edb.9 for ; Fri, 17 Mar 2023 07:02:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1679061720; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=AswLn0g/rknVeGDjZMq603smWI5CxDxD7fWoFWpj9dg=; b=mr7Vbskej8G39nki5RbUFgHoCYf2tBm8gUeVnQgnpXeR8vir2NqfsIOQSGR4CkoXyv 3whAFpFdaL98DXaGZ7hByarxenrfEoE3NABUNok9vhxXzAFnHjBgxYPl9tqkmWtt7G9Y fwZBX9sLF6iHBiA4fwRzfZfn6yKTSjraL1l1ctFZLWWX0VqCl3n2WR88A+k4KpoHY9EV QSeKGkP2stP0qrDI2gSO4s1uhBq1dg24H/6AlILWu5/I7oGuv4I1ad5JuM+PVymwleCX 3RUYIEXnaBOzHGpVtqklBfvG4wRyfFvd9vqGjRiP2NFo9KolWziE/mg9eiqdhPvZFbbD z3uw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679061720; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=AswLn0g/rknVeGDjZMq603smWI5CxDxD7fWoFWpj9dg=; b=zp6hd0P/FkuKOU74c/GaZUIuKIEdhfjHNmvHWIz1/hnvIm5Czz4PiAcIch6op26eOS yxOIGO5okwzxMUK74peWpyRRvw4laykSyXFKWu5uizuTLU14hfnWegL0N84+3P8X+ZjZ kGTQQM0PXlDHP+0nsw1f7QBvndrKySCjbxPRfF0avRnz7BvKjDdHb30VR/8xz8Ax35BB EvpVRyMhjr42rNj3ztL92psvQKsBuVW4moWhQWS+ZrpZJlFjzAYf1ro5Okywt34SGzcR BEFe09jZOo0jp4rECKE20sRE92qYpun+59JUjY7kc6ypyXhijpwusvwSggHz18Zc20kS b5Pg== X-Gm-Message-State: AO0yUKVKElgmn8P85kmSiyyiD0+9VCymiIFZe0+Z08n1wm3u7dYbpx1F GCV2UhHzkDcsHR9nzJjeJMw1Og== X-Google-Smtp-Source: AK7set/Bnu655RP5oJ8Y8R01qEIdKLTeNQLttFh7v9UGhhc0Ill41EdxSEFjPIhyY28LOIjMlYlAYQ== X-Received: by 2002:a17:906:1193:b0:92f:df03:551 with SMTP id n19-20020a170906119300b0092fdf030551mr8001024eja.15.1679061720487; Fri, 17 Mar 2023 07:02:00 -0700 (PDT) Received: from hera (ppp176092130041.access.hol.gr. [176.92.130.41]) by smtp.gmail.com with ESMTPSA id v2-20020a17090651c200b008d57e796dcbsm1033972ejk.25.2023.03.17.07.01.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Mar 2023 07:02:00 -0700 (PDT) Date: Fri, 17 Mar 2023 16:01:58 +0200 From: Ilias Apalodimas To: Heinrich Schuchardt Cc: Heinrich Schuchardt , Simon Glass , Pali =?iso-8859-1?Q?Roh=E1r?= , u-boot@lists.denx.de, Tom Rini Subject: Re: [PATCH] arm: enable unaligned accesses by default if EFI is configured Message-ID: References: <20230317134257.1057261-1-ilias.apalodimas@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean Hi Heinrich, On Fri, Mar 17, 2023 at 02:56:02PM +0100, Heinrich Schuchardt wrote: > On 3/17/23 14:42, Ilias Apalodimas wrote: > > Heinrich reports that compiling with LTO & EFI breaks armv7 and arm11* > > builds. The reason is that LTO (maybe a binutils bug?) replaces the > > asm version of allow_unaligned() with the __weak definition of the > > function. As a result EFI code ends up running with unaligned accesses > > disabled and eventually crashes. > > > > So let's enable unaligned access for those architectures during > > our start.S routines and avoid the linker issues. > > > > Reported-by: Heinrich Schuchardt > > The problem was originally reported by Tom. My contribution was to track it > down to missing enabling of unaligned access due to a linker problem. > Ah thanks, I'll fix that in v2 [...] > > bic r0, r0, #0x00002300 @ clear bits 13, 9:8 (--V- --RS) > > bic r0, r0, #0x00000087 @ clear bits 7, 2:0 (B--- -CAM) > > +#if !CONFIG_IS_ENABLED(EFI_LOADER) > > + /* allow unaligned accesses since EFI requires it */ > > This comment line is only reached without UEFI support. So here you should > explain why we forbid unaligned access without UEFI. I am not sure I am following. I don't really remember why we had UA disabled on armv7. Do you have any suggestions? regards /Ilias > > Best regards > > Heinrich > > > orr r0, r0, #0x00000002 @ set bit 1 (A) Align > > +#endif > > orr r0, r0, #0x00001000 @ set bit 12 (I) I-Cache > > mcr p15, 0, r0, c1, c0, 0 > > diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S > > index 78a9cc173a39..0c80160702ba 100644 > > --- a/arch/arm/cpu/arm1176/start.S > > +++ b/arch/arm/cpu/arm1176/start.S > > @@ -79,7 +79,10 @@ cpu_init_crit: > > mrc p15, 0, r0, c1, c0, 0 > > bic r0, r0, #0x00002300 @ clear bits 13, 9:8 (--V- --RS) > > bic r0, r0, #0x00000087 @ clear bits 7, 2:0 (B--- -CAM) > > +#if !CONFIG_IS_ENABLED(EFI_LOADER) > > + /* allow unailgned accesses since EFI requires it */ > > orr r0, r0, #0x00000002 @ set bit 1 (A) Align > > +#endif > > orr r0, r0, #0x00001000 @ set bit 12 (I) I-Cache > > /* Prepare to disable the MMU */ > > diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile > > index 653eef8ad79e..ca50f6e93e10 100644 > > --- a/arch/arm/cpu/armv7/Makefile > > +++ b/arch/arm/cpu/armv7/Makefile > > @@ -13,7 +13,6 @@ obj-y += syslib.o > > obj-$(CONFIG_SYS_ARM_MPU) += mpu_v7r.o > > ifneq ($(CONFIG_SPL_BUILD),y) > > -obj-$(CONFIG_EFI_LOADER) += sctlr.o > > obj-$(CONFIG_ARMV7_NONSEC) += exception_level.o > > endif > > diff --git a/arch/arm/cpu/armv7/sctlr.S b/arch/arm/cpu/armv7/sctlr.S > > deleted file mode 100644 > > index bd56e41afe18..000000000000 > > --- a/arch/arm/cpu/armv7/sctlr.S > > +++ /dev/null > > @@ -1,22 +0,0 @@ > > -/* SPDX-License-Identifier: GPL-2.0+ */ > > -/* > > - * Routines to access the system control register > > - * > > - * Copyright (c) 2018 Heinrich Schuchardt > > - */ > > - > > -#include > > - > > -/* > > - * void allow_unaligned(void) - allow unaligned access > > - * > > - * This routine clears the aligned flag in the system control register. > > - * After calling this routine unaligned access does no longer lead to a > > - * data abort but is handled by the CPU. > > - */ > > -ENTRY(allow_unaligned) > > - mrc p15, 0, r0, c1, c0, 0 @ load system control register > > - bic r0, r0, #2 @ clear aligned flag > > - mcr p15, 0, r0, c1, c0, 0 @ write system control register > > - bx lr @ return > > -ENDPROC(allow_unaligned) > > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S > > index 7d7aac021e22..abb89efb9ed5 100644 > > --- a/arch/arm/cpu/armv7/start.S > > +++ b/arch/arm/cpu/armv7/start.S > > @@ -197,7 +197,10 @@ ENTRY(cpu_init_cp15) > > mrc p15, 0, r0, c1, c0, 0 > > bic r0, r0, #0x00002000 @ clear bits 13 (--V-) > > bic r0, r0, #0x00000007 @ clear bits 2:0 (-CAM) > > +#if !CONFIG_IS_ENABLED(EFI_LOADER) > > + /* allow unaligned accesses since EFI requires it */ > > orr r0, r0, #0x00000002 @ set bit 1 (--A-) Align > > +#endif > > orr r0, r0, #0x00000800 @ set bit 11 (Z---) BTB > > #if CONFIG_IS_ENABLED(SYS_ICACHE_OFF) > > bic r0, r0, #0x00001000 @ clear bit 12 (I) I-cache > > diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h > > index 3d33a5a063e8..cf032ce1d4cc 100644 > > --- a/include/asm-generic/unaligned.h > > +++ b/include/asm-generic/unaligned.h > > @@ -19,8 +19,4 @@ > > #else > > #error invalid endian > > #endif > > - > > -/* Allow unaligned memory access */ > > -void allow_unaligned(void); > > - > > #endif > > diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c > > index 3b267b713e98..bbaaf626d9cb 100644 > > --- a/lib/efi_loader/efi_device_path.c > > +++ b/lib/efi_loader/efi_device_path.c > > @@ -954,13 +954,6 @@ static void path_to_uefi(void *uefi, const char *src) > > { > > u16 *pos = uefi; > > - /* > > - * efi_set_bootdev() calls this routine indirectly before the UEFI > > - * subsystem is initialized. So we cannot assume unaligned access to be > > - * enabled. > > - */ > > - allow_unaligned(); > > - > > while (*src) { > > s32 code = utf8_get(&src); > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > > index 58d4e1340233..bd17db61c697 100644 > > --- a/lib/efi_loader/efi_setup.c > > +++ b/lib/efi_loader/efi_setup.c > > @@ -17,15 +17,6 @@ > > efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED; > > -/* > > - * Allow unaligned memory access. > > - * > > - * This routine is overridden by architectures providing this feature. > > - */ > > -void __weak allow_unaligned(void) > > -{ > > -} > > - > > /** > > * efi_init_platform_lang() - define supported languages > > * > > @@ -191,9 +182,6 @@ int efi_init_early(void) > > { > > efi_status_t ret; > > - /* Allow unaligned memory access */ > > - allow_unaligned(); > > - > > /* Initialize root node */ > > ret = efi_root_node_register(); > > if (ret != EFI_SUCCESS) >