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 X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 65A79C433C1 for ; Tue, 30 Mar 2021 08:52:50 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EC6946195D for ; Tue, 30 Mar 2021 08:52:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EC6946195D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cerno.tech Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender: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-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=2gENEFyscGzvvK/UfOp2KM3DRys88Rgjz7ncA5O77w0=; b=Ar2xsaeqvxTMBFrHedIVtPVs9 AjivrrsOhmHlURq2bByDOnfImS/Bg8cSmnGUyZL1IH4FwNhu8nLNAIVZ2i73NyLGCTLEQLNBEOJbn PjFddv9G8CZiIxcWS7WrOsgx7rS9XvxEKQxTXyIscH/vubks4Xi+ZH4JQz9TqsMQN3p/Z1/tFDhqk FYiD9nCGsyC0EXMyLQ5G68drK4/KEm7VIMcGOqPqtnyRI++qlvGQbfEI7h6ONrS/kHq8+Zw1vWruq hQWi41X1D6gvMO/DPbf50lKOxz4wgbaWIpjtDWn9ibnB0cYfHH4g2OfZ1Ixi2CJCdgaWzeYzy1gT3 skmKVHcvQ==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lRA5g-0035q3-Ae; Tue, 30 Mar 2021 08:51:12 +0000 Received: from out2-smtp.messagingengine.com ([66.111.4.26]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lRA5Z-0035p2-IT for linux-arm-kernel@lists.infradead.org; Tue, 30 Mar 2021 08:51:09 +0000 Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id 488B85C01DD; Tue, 30 Mar 2021 04:50:58 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Tue, 30 Mar 2021 04:50:58 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm2; bh=DYuF8ynynyUmg6f7TaD/4Cm9Ex5 LmsY3DCYS1inVb+I=; b=RjkMMZ7Ogm7XVBdnoG+lxUzzkh2Votx/DWQ8H5r073x shyp3beqFmAg3LZrmP74zPJqge02q7IxfqoZVxClXkv+lYtOCsCuNTYuQomhIWsi 8CMqjMnC+mrU0V4T/ajmGnSf4raTGWxzb/8LZka+C2paburpNHLhYNoEJQjH0k3b 5ZfXaDnNsfimby32c1BHSQxs0wTEhKguVpwX4zrzp2bbyMJQgY1r3ESzodnbgPs4 Tf7ev3vC59jAwrckodGZiLQx1LeQvc0C1H4LLKYL+/smbYMI/RmZmdvu4pU5rW9C aBlE/4jelctzuv1oUwWEDkQ/5O8ra4C6K5Xn8srHn6w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=DYuF8y nynyUmg6f7TaD/4Cm9Ex5LmsY3DCYS1inVb+I=; b=crrIKOXWIkCv14bpXIgj8k ig9wzz/YtRttVsnNfF2dlCBDsYM3IVF+ITz6WxYxOqMxNnMo5VqL/KL+tM5TpNfA CD/jiKQtRcR528hipEuHFZVF8EAsldzjull9tb1gqOSUPkNoOZDA4uj7PocO5qqT 77CGFpeYy6+Bgo/SLtEJYCoAp4BL5qoZGpIegiLVOIzfFP/xRBO/c8jf8aa3OnRs H3p1eRbqHpPRUR8vtrGfbfuN6aaq1GnPHuJRsYZl2ptrlK7Tksv0jz6QiZMYhfUg Ysu+xq7l2WV8zlspu6iwI/Ss8jFcAMHlcY4lywvlSRUx4vJ2xD8YVTOH2MRx2oAg == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrudeitddgtdekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnhepleekgeehhfdutdeljefgleejffehfffgieejhffgueefhfdtveetgeehieeh gedunecukfhppeeltddrkeelrdeikedrjeeinecuvehluhhsthgvrhfuihiivgeptdenuc frrghrrghmpehmrghilhhfrhhomhepmhgrgihimhgvsegtvghrnhhordhtvggthh X-ME-Proxy: Received: from localhost (lfbn-tou-1-1502-76.w90-89.abo.wanadoo.fr [90.89.68.76]) by mail.messagingengine.com (Postfix) with ESMTPA id 224C224005C; Tue, 30 Mar 2021 04:50:57 -0400 (EDT) Date: Tue, 30 Mar 2021 10:50:54 +0200 From: Maxime Ripard To: Samuel Holland Cc: Andre Przywara , Chen-Yu Tsai , Jernej Skrabec , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-sunxi@googlegroups.com Subject: Re: [RFC PATCH] arm64: dts: allwinner: a64/h5: Add CPU idle states Message-ID: <20210330085054.6ijkwc2jww6ovkvl@gilmour> References: <20210322062514.40747-1-samuel@sholland.org> <20210323015627.08f9afd6@slackpad.fritz.box> MIME-Version: 1.0 In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210330_095105_936363_82BC6E7A X-CRM114-Status: GOOD ( 46.57 ) 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: multipart/mixed; boundary="===============6251222033969221842==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============6251222033969221842== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fqu5tsevk4cqb364" Content-Disposition: inline --fqu5tsevk4cqb364 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Samuel, On Tue, Mar 23, 2021 at 11:44:50PM -0500, Samuel Holland wrote: > On 3/22/21 8:56 PM, Andre Przywara wrote: > >> I'm sending this patch as an RFC because it raises questions about how > >> we handle firmware versioning. How far back does (or should) our suppo= rt > >> for old TF-A and Crust versions go? > >> > >> cpuidle has a problem that without working firmware support, CPUs will > >> enter idle states and be unable to wake up. As a result, the system wi= ll > >> hang at some point during boot, usually before getting to userspace. > >> > >> For over a year[0], TF-A has exposed the PSCI CPU_SUSPEND function when > >> a SCPI implementation is present[1]. Implementing CPU_SUSPEND is > >> required for implementing SYSTEM_SUSPEND[2], even if CPU_SUSPEND is not > >> itself used for anything.=20 > >> > >> However, there was no code to actually wake up a CPU once it called the > >> CPU_SUSPEND function, because I could not find the register providing > >> the necessary information. The fact that CPU_SUSPEND was broken affect= ed > >> nobody, because nothing ever called it -- there were no idle states in > >> the DTS. In hindsight, what I should have done was always return failu= re > >> from sunxi_validate_power_state(), but that ship has long sailed. > >> > >> I finally found the elusive register and implemented the wakeup code > >> earlier this month[3]. So now, CPU_SUSPEND actually works, if all of > >> your firmware is up to date, and cpuidle works if you add the states in > >> your device tree. > >> > >> Unfortunately, there is currently nothing verifying that compatibility. > >> So you can get into four possible scenarios: > >> 1) No idle states in DTS, any firmware =3D> Linux works, with baseli= ne > >> power consumption. > >> 2) Idle states added to DTS, no Crust/SCPI =3D> Linux works, but eve= ry > >> attempt to enter an idle state is rejected because CPU_SUSPEND is > >> not hooked up. So power consumption increases by a sizable amount. > >> 3) Idle states added to DTS, "old" Crust/SCPI (before [3]) =3D> Linux > >> fails to boot, because CPUs never return from idle states. > >> 4) Idle states added to DTS, "new" Crust/SCPI (after [3]) =3D> Linux > >> works, with improved power consumption compared to the baseline. > >> > >> Obviously, we want to prevent scenario 3 if possible. > >=20 > > So I think the core of the problem is that the DT describes some > > firmware feature, but we have the DT bundled with the kernel, not the > > firmware. >=20 > I would say the core problem is that the firmware lies about supporting > PSCI CPU_SUSPEND. Linux shouldn't be calling CPU_SUSPEND if the firmware > declares it as unavailable, regardless of what is in the DTS. I would say we have two core problems :) > (Technically, per the PSCI standard, CPU_SUSPEND is a mandatory > function, but a quick survey of the TF-A platforms shows it is far from > universally implemented.) U-boot also implements CPU_SUSPEND but will return -1 if you don't have an implementation. I guess that at the moment we shouldn't be reporting it as supported if we don't But I also agree with Andre here, we shouldn't list cpuidles capabilities in the DTS if we don't always have them either. > > So is there any way we can detect an older crust version in U-Boot, > > then remove any potential idle states from the DT? >=20 > Let's assume that we are using a functioning SoC (H3) or the secure fuse > is blown (A64) and therefore U-Boot cannot access SRAM A2. I can think > of three ways it can learn about crust: >=20 > a) PSCI_FEATURES (e.g. is CPU_SUSPEND supported) > b) Metadata in the FIT image > c) Custom SMCs >=20 > TF-A has some additional methods available: >=20 > d) The SCPI-reported firmware version > e) The magic number at the beginning of the firmware binary >=20 > > Granted, this requires recent U-Boot as well, but at least we could try > > to mitigate the worst case a bit? >=20 > If we're okay with modifying firmware to solve this problem, then I > propose the following solution: >=20 > 1) Version bump crust or change its magic number. > 2) Modify TF-A to only report CPU_SUSPEND as available if it detects the > new crust version. This would involve conditionally setting > sunxi_scpi_psci_ops.validate_power_state, and updating psci_setup.c > to also check for .validate_power_state when setting psci_caps. > 3) Modify the Linux PSCI client to respect PSCI_FEATURES when setting > psci_ops.cpu_suspend. cpuidle-psci checks for this function before > setting up idle states. > 4) Finally, after some time, add the idle states to the DTS. >=20 > In fact, this solution solves both scenarios 2 and 3, because it also > takes care of the native PM implementation, which doesn't implement > CPU_SUSPEND at all. >=20 > Does that sound workable? If we can add the DT node at boot in crust (or in TF-A), then we don't really need PSCI_FEATURES, and it would magically work once the firmware is updated. It looks like a solid plan otherwise Maxime --fqu5tsevk4cqb364 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYGLmbgAKCRDj7w1vZxhR xR6GAP9Xd46Sh1ZKV6TSYgW3Ro2ikOoq7BAh96jMt2rg5YtKswEArrxcKgP0z8lf RvhIhSmKDy+iQ+pMz2uu+FJp4/x9+gk= =05SX -----END PGP SIGNATURE----- --fqu5tsevk4cqb364-- --===============6251222033969221842== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============6251222033969221842==--