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 7C334C433EF for ; Mon, 17 Jan 2022 15:51:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=zw4azZBZgknsDelvrXPMtOJbT/IN++bWHgJqMAvHJqk=; b=vSqL6Oan30t384 b1B4+RdpLhVNGydyErqgc7SHXIva9jSHvz1rzowctl6vqWdzWrPI6vNAFlUvAzfw0vbpubhfFblne FRNosBfzLZwTdXuc2dpgCMMgMxCPFQeOsN90BTfkH8X7B2yP3Gm4hLzeRGm0RqiMzgXO6NiNzHutx j4oPBQN/T6J5dbnMdSQf/1KJpHZIah6JlgFhmO+bTyLdFplgoQb2bDH9lzDozs3Q1jNzUBqwXzBV0 38BDjcozDhJO3cTGeVcffege7kFShL9OluH9yNNsfpaTShLElTse+UYMslASKJV4BiOKfIOwAO4zh 9Xe3+pjacZrcTYNWP8Lg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9UH8-00FTqa-UK; Mon, 17 Jan 2022 15:50:31 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9UH4-00FTp2-SN for linux-arm-kernel@lists.infradead.org; Mon, 17 Jan 2022 15:50:28 +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 BBB2C1FB; Mon, 17 Jan 2022 07:50:25 -0800 (PST) Received: from C02TD0UTHF1T.local (unknown [10.57.38.30]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8C1583F766; Mon, 17 Jan 2022 07:50:24 -0800 (PST) Date: Mon, 17 Jan 2022 15:50:21 +0000 From: Mark Rutland To: Andre Przywara Cc: linux-arm-kernel@lists.infradead.org, Jaxson.Han@arm.com, robin.murphy@arm.com, vladimir.murzin@arm.com, Wei.Chen@arm.com Subject: Re: [bootwrapper PATCH v2 08/13] Announce boot-wrapper mode / exception level Message-ID: <20220117155021.GG87485@C02TD0UTHF1T.local> References: <20220114105653.3003399-1-mark.rutland@arm.com> <20220114105653.3003399-9-mark.rutland@arm.com> <20220117143913.520a553c@donnerap.cambridge.arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220117143913.520a553c@donnerap.cambridge.arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220117_075027_048566_67F16841 X-CRM114-Status: GOOD ( 27.42 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Jan 17, 2022 at 02:39:13PM +0000, Andre Przywara wrote: > On Fri, 14 Jan 2022 10:56:48 +0000 > Mark Rutland wrote: > > Hi Mark, > > > When something goes wrong within the boot-wrapper, it can be very > > helpful to know where we started from. Add an arch_announce() function > > to log this early in the boot process. More information can be added > > here in future. > > > > This is logged ot the serial console as: > > > > | Boot-wrapper v0.2 > > | Entered at EL3 > > I like that one, and apart from the (already existing) UART issue below, > this looks fine: > > > Signed-off-by: Mark Rutland > > Reviewed-by: Andre Przywara Thanks! > > diff --git a/common/platform.c b/common/platform.c > > index 47bf547..80d0562 100644 > > --- a/common/platform.c > > +++ b/common/platform.c > > @@ -31,21 +31,25 @@ > > #define V2M_SYS(reg) ((void *)SYSREGS_BASE + V2M_SYS_##reg) > > #endif > > > > -void print_string(const char *str) > > +void print_char(char c) > > { > > uint32_t flags; > > > > - while (*str) { > > - do > > - flags = raw_readl(PL011(UARTFR)); > > - while (flags & PL011_UARTFR_FIFO_FULL); > > + do { > > + flags = raw_readl(PL011(UARTFR)); > > + } while (flags & PL011_UARTFR_FIFO_FULL); > > + > > + raw_writel(c, PL011(UARTDR)); > > > > - raw_writel(*str++, PL011(UARTDR)); > > + do { > > + flags = raw_readl(PL011(UARTFR)); > > + } while (flags & PL011_UARTFR_BUSY); > > Apologies if that appears to be nitpicking over a totally pointless issue > (given the nature of the *emulated* PL011 in the model), but: > > I understand that this code has not changed, but this loop basically > renders the FIFOs ineffective. Is this intended? Hmm... AFAICT, we started to enable the FIFOs in commit: 26a17ad59544f026 ("bootwrapper: improve UART initialisation") ... but back then the boot-wrapper didn't print anything to the UART, so that was purely about putting the UART into a sane state for the OS. We only started to write to the UART in commit: 7ff3872adb33b068 ("Rewrite platform initialisation in C") ... where we said nothing about the FIFOs. Therefore, I think there was no intent either way to use/avoid the FIFOs within the boot-wrapper. > I mean we explicitly enable the FIFOs in init_uart(), but then poll > here until the transmit FIFO becomes empty, after *every* character > pushed. I see that we probably want the output to be synchronous, for > debug reasons, but I wonder if this should be achieved via an extra > uart_flush() routine, once per line output? So call uart_flush() just > in print_string(), but not in print_char(), for instance? Hmm... we also use print_char() directly from the aarch64 init code when logging the exception level, so we'd need an explicit sync there, if we do want that output before other things can go wrong. Otherwise, I don't have strong feelings either way, but I would like to have the output completed before print_*() return. We could add an internal print_char_nosync() that print_{char,string}() use to handle that. I'm happy to take a follow-up patch for that, but for now I'm going to leave this as-is. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel