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 0F393C433F5 for ; Tue, 11 Jan 2022 10:28:40 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3928481DD6; Tue, 11 Jan 2022 11:28:38 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id DC11E81E25; Tue, 11 Jan 2022 11:28:36 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id CFD088142A for ; Tue, 11 Jan 2022 11:28:32 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E924B1FB; Tue, 11 Jan 2022 02:28:31 -0800 (PST) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7728B3F766; Tue, 11 Jan 2022 02:28:30 -0800 (PST) Date: Tue, 11 Jan 2022 10:28:26 +0000 From: Andre Przywara To: Mark Kettenis Cc: heinrich.schuchardt@canonical.com, trini@konsulko.com, sjg@chromium.org, alison.wang@nxp.com, michael@walle.cc, nm@ti.com, priyanka.singh@nxp.com, Peter.Hoyes@arm.com, marek.vasut+renesas@gmail.com, u-boot@lists.denx.de Subject: Re: [PATCH 1/6] cmd: exception: arm64: fix undefined, add faults Message-ID: <20220111102826.3524c312@donnerap.cambridge.arm.com> In-Reply-To: References: <20220109173009.25522-1-andre.przywara@arm.com> <20220109173009.25522-2-andre.przywara@arm.com> <7fd24059-c47a-b7ec-3ecb-7066e33fcdc1@canonical.com> <20220109213147.479dad13@slackpad.fritz.box> <20220109223527.788c93ca@slackpad.fritz.box> <20220109231910.3dd1602c@slackpad.fritz.box> Organization: ARM X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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.2 at phobos.denx.de X-Virus-Status: Clean On Mon, 10 Jan 2022 23:37:59 +0100 (CET) Mark Kettenis wrote: > > Date: Sun, 9 Jan 2022 23:19:10 +0000 > > From: Andre Przywara > > > > On Sun, 9 Jan 2022 23:47:32 +0100 (CET) > > Mark Kettenis wrote: > > > > Hi Mark, > > > > (I knew I forgot to CC: one person ...) > > > > > > Date: Sun, 9 Jan 2022 22:35:27 +0000 > > > > From: Andre Przywara > > > > > > > > Hi Heinrich, > > > > > > > > > On 1/9/22 22:31, Andre Przywara wrote: > > > > > > On Sun, 9 Jan 2022 20:08:41 +0100 > > > > > > Heinrich Schuchardt wrote: > > > > > > > > > > > >> On 1/9/22 18:30, Andre Przywara wrote: > > > > > >>> The arm64 version of the exception command was just defining the > > > > > >>> undefined exception, but actually copied the AArch32 instruction. > > > > > >>> > > > > > >>> Replace that with an encoding that is guaranteed to be and stay > > > > > >>> undefined. Also add instructions to trigger unaligned access faults and > > > > > >>> a breakpoint. > > > > > >>> This brings ARM64 on par with ARM(32) for the exception command. > > > > > >>> > > > > > >>> Signed-off-by: Andre Przywara > > > > > >>> --- > > > > > >>> cmd/arm/exception64.c | 42 ++++++++++++++++++++++++++++++++++++++---- > > > > > >>> 1 file changed, 38 insertions(+), 4 deletions(-) > > > > > >>> > > > > > >>> diff --git a/cmd/arm/exception64.c b/cmd/arm/exception64.c > > > > > >>> index d5de50a0803..1a9730e6aec 100644 > > > > > >>> --- a/cmd/arm/exception64.c > > > > > >>> +++ b/cmd/arm/exception64.c > > > > > >>> @@ -12,14 +12,46 @@ static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc, > > > > > >>> char *const argv[]) > > > > > >>> { > > > > > >>> /* > > > > > >>> - * 0xe7f...f. is undefined in ARM mode > > > > > >>> - * 0xde.. is undefined in Thumb mode > > > > > >>> + * Instructions starting with the upper 16 bits all 0 are permanently > > > > > >>> + * undefined. The lower 16 bits can be used for some kind of immediate. > > > > > >>> + * --- ARMv8 ARM (ARM DDI 0487G.a C6.2.339: "UDF") > > > > > >>> */ > > > > > >>> - asm volatile (".word 0xe7f7defb\n"); > > > > > >>> + asm volatile (".word 0x00001234\n"); > > > > > >>> + > > > > > >>> + return CMD_RET_FAILURE; > > > > > >>> +} > > > > > >>> + > > > > > >>> +static int do_unaligned(struct cmd_tbl *cmdtp, int flag, int argc, > > > > > >>> + char *const argv[]) > > > > > >>> +{ > > > > > >>> + /* > > > > > >>> + * The load acquire instruction requires the data source to be > > > > > >>> + * naturally aligned, and will fault even if strict alignment fault > > > > > >>> + * checking is disabled. > > > > > >>> + * --- ARMv8 ARM (ARM DDI 0487G.a B2.5.2: "Alignment of data accesses") > > > > > >> > > > > > >> According to DI0487G_b_armv8_arm.pdf available at > > > > > >> https://developer.arm.com/documentation/ddi0487/latest the generation of > > > > > >> an alignment fault for ldar depends on FEAT_LSE2 (Large System > > > > > >> Extensions v2) which is mandatory for ARMv8.4. See p. B2-161. > > > > > > > > > > > > Well found, but I wonder if that matters for the SoCs running U-Boot. > > > > > > It looks like the Apple M1 is the only one so far and will probably > > > > > > stay for a while. > > > > > > > > > > Developers are using U-Boot on Apple M1 already. > > > > > > > > Yeah, sorry, I didn't realise that the M1 is fully v8.4 compliant. Most > > > > other SoCs I checked are v8.2/v8.3 + some 8.4 features, at most. The > > > > Cortex-A76/Neoverse-N1 for instance does not have LSE2. > > > > > > > > > > But I can of course check ID_AA64MMFR2_EL1.AT before executing the LDAR, > > > > > > and will ask around for a better method to provoke unaligned accesses. > > > > > > > > > > It is sufficient if you update the comment for this function. Returning > > > > > CMD_RET_FAILURE as return value if unaligned access is supported is > > > > > fine. Cf. cmd/riscv/exception.c. (On RISC-V OpenSBI emulates unaligned > > > > > access.) > > > > > > > > Well, I now have the check and a message, always returning FAILURE in > > > > this case. Let me see if people in the office have a better idea... > > > > > > > > > Maybe we should also add a comment in doc/usage/exception.rst. > > > > > > > > By the way: this was triggered by my need to check SError generation. I > > > > don't know of a nice architectural way to trigger an SError (yet), but > > > > some SoCs happily generate one by accessing unimplemented memory regions > > > > (beyond DRAM, for instance). So I could trigger it on my Juno board > > > > with a specific address, but not on an Allwinner board so far. > > > > Do you think it's worthwhile to have a platform specific address in > > > > Kconfig to implement the serror exception sub-command? > > > > > > Well, on the M1 writing to the serial port output register with the > > > wrong width (8 bits instead of 32 bits) triggered an SError while > > > still producing output. But once the OS booted, it did panic with an > > > SError. Which indeed caused some head scratching like you described. > > > > > > Do you want me to test this series on the M1? > > > > Oh yes, please, that would be much appreciated! Just booting would be a > > good test already, but bonus points for enabling CMD_EXCEPTION and > > testing all subcommands. > > Booting the M1 mini still works. With CMD_EXCEPTION enabled: > > => exception breakpoint > "Synchronous Abort" handler, esr 0xf200007b > > => exception unaligned > > > => exception undefined > "Synchronous Abort" handler, esr 0x02000000 Great, exactly as expected! I have a slight change to check for the FEAT_LSE ID bit and print a warning for the unaligned check, but it's already OK as is, I guess. Will CC: you on a v2. > So: > > Tested-by: Mark Kettenis Many thanks, much appreciated! Cheers, Andre