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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1220CC3DA4A for ; Thu, 22 Aug 2024 10:03:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Z/mes+5wYYE+WO+MyuxDa82IC2u0x0goIgng8xncTSo=; b=U1oxB8PGoGWNu7xLWEdwhqJBuA XZvbUvZ1X0WG7I5NcGbGH1JN5WNtha3ld7mC365CMRnLhkKneWLmXvhPaob2oY8CJYbAmtNVyj7p3 ZFX69pWI9UnlCZo1nWLzPNFr/J9Ebsr5iZcV279F3JI2tHjw6pplBtK2TH2vfwUdaVFoIWYxln2Si 8MuKezyf5hCp2RtaBaYgJAVyNaFAiMJzs1JXIvNYIH0ISrxRitI0K7Xm8zGHK82LppSposATDN+mc k6s1isH1NVwP5tx7vhFcMlA20XIP85bJ3QhqELY1bdb9HMKp30jKBjLGG02H9YSO/c1xdTOm/b4U8 OhNpVMjA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sh4f8-0000000CMXg-1sLI; Thu, 22 Aug 2024 10:03:26 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sh4eO-0000000CMUd-2426 for linux-arm-kernel@lists.infradead.org; Thu, 22 Aug 2024 10:02:41 +0000 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 8C1E0DA7; Thu, 22 Aug 2024 03:03:05 -0700 (PDT) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8A90E3F66E; Thu, 22 Aug 2024 03:02:38 -0700 (PDT) Date: Thu, 22 Aug 2024 11:02:36 +0100 From: Mark Rutland To: Andre Przywara Cc: linux-arm-kernel@lists.infradead.org, akos.denke@arm.com, luca.fancellu@arm.com, maz@kernel.org Subject: Re: [BOOT-WRAPPER v2 10/10] Boot CPUs sequentially Message-ID: References: <20240812101555.3558589-1-mark.rutland@arm.com> <20240812101555.3558589-11-mark.rutland@arm.com> <20240821184923.2ff8d41e@donnerap.manchester.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240821184923.2ff8d41e@donnerap.manchester.arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240822_030240_654237_2709997E X-CRM114-Status: GOOD ( 21.96 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Aug 21, 2024 at 06:49:23PM +0100, Andre Przywara wrote: > On Mon, 12 Aug 2024 11:15:55 +0100 > Mark Rutland wrote: > > > Currently the boot-wrapper initializes all CPUs in parallel. This means > > that we cannot log errors as they happen, as this would mean multiple > > CPUs concurrently writing to the UART, producing garbled output. To > > produce meaningful output we have to special-case errors on the boot CPU > > and hope CPUs have been configured consistently. > > > > To make it easier to handle errors, boot CPUs sequentially so that errors > > can be logged as they happen. With this change it's pretty clear that > > the cpu_init_bootmethod() abstraction isn't helpful, and so this is > > removed with cpu_init_arch() directly initializing PSCI where necessary. > > Yeah, makes sense, though the change is a bit hard to follow in the diff > below. But I convinced myself that it's correct. Just one small comment > inside: > > > When things go well this looks like: > > > > | Boot-wrapper v0.2 > > | Entered at EL3 > > | Memory layout: > > | [0000000080000000..0000000080001f90] => boot-wrapper > > | [000000008000fff8..0000000080010000] => mbox > > | [0000000080200000..0000000082cbaa00] => kernel > > | [0000000088000000..0000000088002df1] => dtb > > | CPU0: (MPIDR 0000000000000000) initializing... > > | CPU0: Done > > | CPU1: (MPIDR 0000000000000100) initializing... > > | CPU1: Done > > | CPU2: (MPIDR 0000000000000200) initializing... > > | CPU2: Done > > | CPU3: (MPIDR 0000000000000300) initializing... > > | CPU3: Done > > | CPU4: (MPIDR 0000000000010000) initializing... > > | CPU4: Done > > | CPU5: (MPIDR 0000000000010100) initializing... > > | CPU5: Done > > | CPU6: (MPIDR 0000000000010200) initializing... > > | CPU6: Done > > | CPU7: (MPIDR 0000000000010300) initializing... > > | CPU7: Done > > | All CPUs initialized. Entering kernel... > > | > > | [ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd0f0] > > +static void cpu_init_self(unsigned int cpu) > > +{ > > + print_string("CPU"); > > + print_uint_dec(cpu); > > + print_string(": (MPIDR "); > > + print_ulong_hex(read_mpidr()); > > + print_string(") initializing...\r\n"); > > It feels like using two lines per core for something that rarely fails > is a bit wasteful. Can we get rid of the EOL here, and just put the "Done" > right behind it, in the same line? That reduces the noise on the terminal. That'll interact poorly with logging errors, as the first will get appended to the "initializing..." line. Instead, I'll get rid of the "Done" line, so that the output ends up as: | CPU0: (MPIDR 0000000000000000) initializing... | CPU1: (MPIDR 0000000000000100) initializing... | CPU2: (MPIDR 0000000000000200) initializing... | CPU3: (MPIDR 0000000000000300) initializing... | CPU4: (MPIDR 0000000000010000) initializing... | CPU5: (MPIDR 0000000000010100) initializing... | CPU6: (MPIDR 0000000000010200) initializing... | CPU7: (MPIDR 0000000000010300) initializing... | All CPUs initialized. Entering kernel... ... and if any error occurs it'll still be clear which CPU it belongs to given the next line will either be the next CPU being announced or the final "All CPUs initialized." message. Mark.