Re: [GICv3 ITS]S2IDLE framework does not invoke syscore_ops in GICv3 ITS driver

[Youngmin Nam <youngmin.nam@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 3127 bytes --]

On Thu, Mar 27, 2025 at 08:25:14AM +0000, Marc Zyngier wrote:
> On Thu, 27 Mar 2025 03:22:19 +0000,
> Youngmin Nam <youngmin.nam@samsung.com> wrote:
> > 
> > [1  <text/plain; utf-8 (8bit)>]
> > On Wed, Mar 26, 2025 at 08:59:02AM +0000, Marc Zyngier wrote:
> > > On Wed, 26 Mar 2025 03:09:37 +0000,
> > > Youngmin Nam <youngmin.nam@samsung.com> wrote:
> > > > 
> > > > Hi.
> > > > 
> > > > On our SoC, we are using S2IDLE instead of S2R as a system suspend mode.
> > > > However, when I try to enable ARM GICv3 ITS driver (drivers/irqchip/irq-gic-v3-its.c),
> > > > I noticed that there is no proper way to invoke suspend/resume callback,
> > > > because it only uses syscore_ops, which is not called in an s2idle 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 state
> > > where the GIC (and the ITS) is going to lose its state (because power
> > > 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’d like to distinguish between the GICv3 driver (irq-gic-v3.c)
> > and the ITS driver (irq-gic-v3-its.c).
> > 
> > I now understand why the GICv3 driver doesn’t implement suspend 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 scenario,
> > 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.
> 
> 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.
> 
> 	M.

Thank you for the clear explanation. I completely understand now.

> 
> -- 
> Without deviation from the norm, progress is not possible.
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]