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 58AA5C36010 for ; Tue, 1 Apr 2025 13:13:18 +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:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Subject:Cc:To:From: Message-ID:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=CyWpiRvN0POi/1TulCaB9srdQFEHIu/XMwIF9eHHINM=; b=syZ5dD00JZh1WdZmsUSrI7rVB4 UDTEOSbuWpGAYe6IBgZhumVvBTaeXmc/02UDHCeExIxqGHg6S6Bluhu+n3dwGAyiNsqIvG32rbj73 xCXttuU+/VfA302lbDPj6lPdh3Qg5EF4No1aGyTnPClIkTN4HssURNqTqV8nww3gDjj3L9pr/qkfS 3UPxx0WC1I9rk6o5GfD5vRLG5UUE0i5isX+xtDqUslqsnl6QXPTbgxnaZUz8joCOOtv/XwVCHzVlh 0PZlVGA7/6BYJ+VInjd+vOvpXrxAW/Hc/ylzbEBMoHDctgPkXoSGpBqtwbzodqZmSAuB0ZIcKfMp4 b2fCN3Kg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.1 #2 (Red Hat Linux)) id 1tzbQO-00000003PN9-0Ip5; Tue, 01 Apr 2025 13:13:04 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.1 #2 (Red Hat Linux)) id 1tzbOb-00000003PAF-1two for linux-arm-kernel@lists.infradead.org; Tue, 01 Apr 2025 13:11:13 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 2ADE461132; Tue, 1 Apr 2025 13:11:06 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 86B57C4CEE4; Tue, 1 Apr 2025 13:11:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1743513072; bh=eFj1UtKjWSXUv/IED2cgaAtIRcrA4Zbbbpt7I7dajA0=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=R8s4/DS4YxcOEWSzlM3EE8vMNdgInvDagqB0HhLPdwt0aoPX7rxBf0I6Kpc64GT8C UUTWbxd+xYJhVopEXrXBgG8HTMB7uZTES/sW5bKL9BwW2rs4GSHajMsPPlDBZF4Aqf qNuhLIasqKX5evNv6IFIdMNHVe92B290frTpNumgISwIcHvDISWflIPzzrpKZO1sAk t1W4COqnyH9xXfZj7MTfNJSckuianb+sMptS2E0oXYGr2pS8XIl+LmhVKTsmJHrC9I yOGKbUNPH6el2Kq6/CMN5PJnPNjtH1TF/00cUlSrQpUtGrj1/PxDUm+vj0v7jIiFTK XXan03RgCy69A== Received: from sofa.misterjones.org ([185.219.108.64] helo=lobster-girl.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 1tzbOY-001L1O-Bf; Tue, 01 Apr 2025 14:11:10 +0100 Date: Tue, 01 Apr 2025 14:11:11 +0100 Message-ID: <87o6xgyqkw.wl-maz@kernel.org> From: Marc Zyngier To: Ulf Hansson Cc: Youngmin Nam , Thomas Gleixner , Saravana Kannan , Vincent Guittot , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel-team@android.com, hajun.sung@samsung.com, d7271.choe@samsung.com, joonki.min@samsung.com Subject: Re: [GICv3 ITS]S2IDLE framework does not invoke syscore_ops in GICv3 ITS driver In-Reply-To: References: <8634f0mall.wl-maz@kernel.org> <86v7rulw2d.wl-maz@kernel.org> 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/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: ulf.hansson@linaro.org, youngmin.nam@samsung.com, tglx@linutronix.de, saravanak@google.com, vincent.guittot@linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel-team@android.com, hajun.sung@samsung.com, d7271.choe@samsung.com, joonki.min@samsung.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false 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 Tue, 01 Apr 2025 13:45:43 +0100, Ulf Hansson wrote: >=20 > On Thu, 27 Mar 2025 at 09:25, Marc Zyngier wrote: > > > > On Thu, 27 Mar 2025 03:22:19 +0000, > > Youngmin Nam wrote: > > > > > > [1 ] > > > On Wed, Mar 26, 2025 at 08:59:02AM +0000, Marc Zyngier wrote: > > > > On Wed, 26 Mar 2025 03:09:37 +0000, > > > > Youngmin Nam wrote: > > > > > > > > > > Hi. > > > > > > > > > > On our SoC, we are using S2IDLE instead of S2R as a system suspen= d mode. > > > > > However, when I try to enable ARM GICv3 ITS driver (drivers/irqch= ip/irq-gic-v3-its.c), > > > > > I noticed that there is no proper way to invoke suspend/resume ca= llback, > > > > > because it only uses syscore_ops, which is not called in an s2idl= e scenario. > > > > > > > > This is *by design*. > > > > [...] > > > > > > > How should we handle this situation ? > > > > > > > > By implementing anything related to GIC power-management in your EL3 > > > > firmware. Only your firmware knows whether you are going into a sta= te > > > > where the GIC (and the ITS) is going to lose its state (because pow= er > > > > is going to be removed) or if the sleep period is short enough that > > > > you can come back from idle without loss of context. > > > > > > > > Furthermore, there is a lot of things that non-secure cannot do when > > > > it comes to GIC power management (most the controls are secure only= ), > > > > so it is pretty clear that the kernel is the wrong place for this. > > > > > > > > I'd suggest you look at what TF-A provides, because this is not > > > > exactly a new problem (it has been solved several years ago). > > > > > > > > M. > > > > > > > > -- > > > > Without deviation from the norm, progress is not possible. > > > > > > > > > > Hi Marc, > > > > > > First of all, I=E2=80=99d like to distinguish between the GICv3 drive= r (irq-gic-v3.c) > > > and the ITS driver (irq-gic-v3-its.c). > > > > > > I now understand why the GICv3 driver doesn=E2=80=99t implement suspe= nd and resume functions. > > > However, unlike the GICv3 driver, the ITS driver currently provides > > > suspend and resume functions via syscore_ops in the kernel. > > > > For *suspend*. The real suspend. Not a glorified WFI. And that's only > > for situations where we know for sure that we are going to suspend. > > > > > And AFAIK, LPIs are always treated as non-secure. (Please correct me = If I'm wrong). > > > > > > The problem is that syscore_ops is not invoked during the S2IDLE scen= ario, > > > so we cannot rely on it in that context. > > > We would like to use these suspend/resume functions during S2IDLE as = well. > > > > Again, this is *by design*. There is no semantic difference between > > s2idle and normal idle. They are the same thing. Do you really want to > > save/restore the whole ITS state on each and every call into idle? > > Absolutely not. >=20 > I agree that we don't want to save/restore for every call to idle, > that would simply be unnecessary and add latencies. >=20 > Instead, I think the save/restore could depend on what idlestate we > enter and whether it's a system-wide state (s2idle/s2ram) or just > regular cpuidle-state. >=20 > Today, we are pointing the callbacks for cpuidle and s2idle to the > same functions (at least for PSCI PC mode), but it's easy to change > that *if* we need some differentiation between s2idle and cpuidle. >=20 > > > > Only your firmware knows how deep you will be suspended, and how long > > you will be suspended for, and this is the right place for to perform > > save/restore of the ITS state. Not in generic code that runs on every > > arm64 platform on the planet. >=20 > Assuming we can make the code for saving/restoring generic (not in FW) > and that we are able to make sure the code is only executed for those > platforms and states that really need it. Do you think there would > there be any other drawback for doing this? Yes. We'd end-up having to implement all sort of split PM schemes depending on the GIC implementation, what the firmware does, the various braindead assumptions that the integration makes, and other parameters I don't even want to consider. The GIC power management is, for better or worse, *outside* of the scope of the architecture. Most of it is implementation defined, because each and every implementer/vendor sees it as added value to invent their own particular flavour of crap. For example, there is no provision for wake-up interrupts, because nobody can agree on how that's supposed to work. Do we want to deal with this in the various GIC drivers? No. It is the job of firmware to manage this mess, because this clearly delineates where the responsibilities lie. M. --=20 Jazz isn't dead. It just smells funny.