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

[Donghyeok Choe]

On Thu, Apr 03, 2025 at 10:18:54AM +0100, Sudeep Holla wrote:
> /me more confused.
> 
> Are you saying you have some cpuidle platform specific logic inside
> trace_android_vh_cpuidle_psci_enter(). I would assume it was just to
> trace the entry into the state and nothing more.

Hi, I am a Samsung developer working with Youngmin Nam.

Yes, you are correct. The platform-specific logic runs, and only
the booting core performs the GIC ITS save/restore. I understand that
using trace in this way might have surprised you.
However, this kind of trace usage is actually quite common
within the Android kernel.

The Android kernel aims to ensure that all SoC vendors use
the same kernel code, aiming for the benefits of the GKI project.
Since vendors often want to control kernel behavior, they utilize
traces like this to achieve that flexibility.

> In fact, it was recently added upstream as well.
> Commit 7b7644831e72 ("cpuidle: psci: Add trace for PSCI domain idle")

Then it seems that this will be used instead of the Android kernel specific
vendor hook(trace_android_vh_cpuidle_psci_enter).
We can eliminate the diff between the mainline code and the ACK.

> Further you didn't explicitly answer my question. IIUC are you calling
> GIC ITS suspend function unconditionally if its boot cpu ? Or is it
> done only for s2idle ? If done only for s2idle, how does it work for
> normal cpuidle entry to deepest idle state that matches the one entered
> during s2idle.

In the s2idle situation, only the booting core performs
the GIC ITS suspend/resume. The difference from normal cpuidle
is that in the case of s2idle, the entire system enters suspend
at a level equivalent to kernel suspend.

Through that trace(trace_android_vh_cpuidle_psci_enter),
we don't directly call syscore suspend, but rather mimic
the actions that syscore suspend would perform.

You might be concerned about the following:
While kernel suspend hotplugs out the remaining cores except for
the booting core and performs syscore suspend, in the s2idle scenario,
cores enter cpuidle in parallel when going into suspend.
I understand that you may worry about the synchronization issues
that could arise when cores enter cpuidle in parallel during
the s2idle situation.
There were many exceptional cases, and we resolved them all together
with my colleagues. We paid the price for not using the mainline kernel
code as intended.

If you have any further questions, feel free to reach out.

Best regards,  
Donghyeok Choe