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 7AF7AC04A6A for ; Tue, 15 Aug 2023 11:13:38 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5F63186725; Tue, 15 Aug 2023 13:13:36 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.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=kernel.org header.i=@kernel.org header.b="onQzV5Jj"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 04D9D86763; Tue, 15 Aug 2023 13:13:35 +0200 (CEST) Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 0A44B833BF for ; Tue, 15 Aug 2023 13:13:32 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=maz@kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 520A960FE6; Tue, 15 Aug 2023 11:13:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B51FBC433C8; Tue, 15 Aug 2023 11:13:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1692098009; bh=93NVAEz+ieVEokzSkAWAyk9BHz8E7LhO3aiZ8GwMN3o=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=onQzV5JjLGGvy7zCVFw5IW7GxYOdfS9Abc83jOSXnM4sbaGKd+EFrknunS3ltJwQE kLd8JYpLB877qf3g4t+CclGJh7b0mB7AyzIUhEZKaPZ/3go94cznzhpJtQvByIwRqR /seeeoEutU5hDTSf/pY0tLf7OdWinnn+/p/Uc+5D5x8l+jElIw3RCwh2FdhoonaTEB /qHdNzZyDd0I/2udEms4pqyAeludYKvd3UYYi7l+YGWmnXsrxq53ZoVWgvY4/JhCLJ R4tcyQv5c8Us5rf7jyoivFF+yOw5hAir6jfUDVMbodRf3eBQzs725IV6F8Z5ihMXzo AuwS8INVvRhTw== Received: from ip-185-104-136-31.ptr.icomera.net ([185.104.136.31] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1qVrzL-004zEr-Eu; Tue, 15 Aug 2023 12:13:27 +0100 Date: Tue, 15 Aug 2023 12:13:26 +0100 Message-ID: <87edk4pnhl.wl-maz@kernel.org> From: Marc Zyngier To: Andre Przywara Cc: Sam Edwards , u-boot@lists.denx.de, Jagan Teki , Chen-Yu Tsai Subject: Re: [PATCH] sunxi: psci: remove redundant initialization from psci_arch_init In-Reply-To: <20230814213854.408962d5@slackpad.lan> References: <20230531201520.15479-1-CFSworks@gmail.com> <20230814213854.408962d5@slackpad.lan> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.104.136.31 X-SA-Exim-Rcpt-To: andre.przywara@arm.com, cfsworks@gmail.com, u-boot@lists.denx.de, jagan@amarulasolutions.com, wens@csie.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false 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 On Mon, 14 Aug 2023 21:39:10 +0100, Andre Przywara wrote: > > On Wed, 31 May 2023 14:15:20 -0600 > Sam Edwards wrote: > > Hi, > > (CC:ing Marc and Chen-Yu as the original authors) > > sorry for the delay, found that mouldering in my Drafts folder. > > > The nonsec code overrides/handles these: > > - Setting PMR to 0xFF happens earlier, in _nonsec_init, so this is > > redundant. > > - The NS bit is not yet set: it gets set later in _secure_monitor. > > Trying to clear it here is pointless and misleading. > > So superficially this looks alright, but I am still somewhat reluctant > to apply this, see below ... > > > Signed-off-by: Sam Edwards > > --- > > arch/arm/cpu/armv7/sunxi/psci.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c > > index e1d3638b5c..f866025c37 100644 > > --- a/arch/arm/cpu/armv7/sunxi/psci.c > > +++ b/arch/arm/cpu/armv7/sunxi/psci.c > > @@ -300,14 +300,10 @@ void __secure psci_arch_init(void) > > /* Set SGI15 priority to 0 */ > > writeb(0, GICD_BASE + GICD_IPRIORITYRn + 15); > > > > - /* Be cool with non-secure */ > > - writel(0xff, GICC_BASE + GICC_PMR); > > - > > /* Switch FIQEn on */ > > setbits_le32(GICC_BASE + GICC_CTLR, BIT(3)); > > > > reg = cp15_read_scr(); > > reg |= BIT(2); /* Enable FIQ in monitor mode */ > > - reg &= ~BIT(0); /* Secure mode */ > > I am scratching my head on this one: I see it deliberately being done in > the original patch by Marc[1], and wonder why. If I read the code and > the architecture correctly, this routine is called only(?) from the SMC > handler, which means we are in monitor mode, that only exists in secure > state. So the NS bit is irrelevant until we switch to another mode. And > we indeed set the bit later, before dropping back to non-secure. So I > agree that clearing the bit is pointless. Was it put in to be more > robust, for other potential callers of this function, for instance in > the boot path? > > Does anyone remember? I don't remember much about this, but setting PMR to include the NS priorities (PMR[7]) is definitely significant when SCR.NS==0. Otherwise, NS cannot write to PMR, which basically kills NS any use. Now, and without the full context, it is hard to figure out what is going on. But a lot of that crap was written with mandate to be able to support the stupid old AW BSP which ran in secure mode, so there were a number of convoluted paths to get there. If this nonsense can now be dropped, I'm sure the whole thing can be simplified. But someone who still care about 32bit platforms (i.e. not me) should have a look. Thanks, M. -- Without deviation from the norm, progress is not possible.