linux-kernel - Re: [PATCH v5] soc: samsung: exynos-pmu: Enable CPU Idle for gs101

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250710-translucent-mastiff-of-performance-b5f1ba@sudeepholla>
Date: Thu, 10 Jul 2025 17:02:03 +0100
From: Sudeep Holla <sudeep.holla@....com>
To: Peter Griffin <peter.griffin@...aro.org>
Cc: André Draszik <andre.draszik@...aro.org>,
	Sudeep Holla <sudeep.holla@....com>,
	Tudor Ambarus <tudor.ambarus@...aro.org>,
	Alim Akhtar <alim.akhtar@...sung.com>,
	Krzysztof Kozlowski <krzk@...nel.org>,
	William Mcvicker <willmcvicker@...gle.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	kernel-team@...roid.com
Subject: Re: [PATCH v5] soc: samsung: exynos-pmu: Enable CPU Idle for gs101

On Wed, Jul 09, 2025 at 10:36:59PM +0100, Peter Griffin wrote:
> Hi Sudeep,
> 
> Thanks for your review feedback!
> 
> On Wed, 9 Jul 2025 at 17:10, Sudeep Holla <sudeep.holla@....com> wrote:
> >
> > On Wed, Jul 09, 2025 at 02:26:27PM +0100, Peter Griffin wrote:
> > > Register cpu pm notifiers for gs101 which call the
> > > gs101_cpu_pmu_online/offline callbacks which in turn program the ACPM
> > > C2 hint. This hint is required to actually enter the C2 idle state.
> > >
> > > A couple of corner cases are handled, namely when the system is rebooting
> > > or suspending we ignore the request. Additionally the request is ignored if
> > > the CPU is in CPU hot plug. Some common code is refactored so that it can
> > > be called from both the CPU hot plug callbacks and CPU PM notifier taking
> > > into account that CPU PM notifiers are called with IRQs disabled whereas
> > > CPU hotplug callbacks are not.
> > >
> > > Additionally due to CPU PM notifiers using raw_spinlock the locking is
> > > updated to use raw_spinlock variants, this includes updating the pmu_regs
> > > regmap to use .use_raw_spinlock = true and additionally creating and
> > > registering a custom  pmu-intr-gen regmap instead of using the regmap
> > > provided by syscon.
> > >
> > > Note: this patch has a runtime dependency on adding 'local-timer-stop' dt
> > > property to the CPU nodes. This informs the time framework to switch to a
> > > broadcast timer as the local timer will be shutdown. Without that DT
> > > property specified the system hangs in early boot with this patch applied.
> > >
> >
> > Assuming this is arm64 platform and using PSCI for all the power management,
> > can you please briefly explain why all these dance is absolutely necessary
> > when PSCI calls can be the clue for the EL3 firmware. I am basing my question
> > on this information in the file:
> 
> Yes, you're correct it is an arm64 platform using PSCI. Unfortunately
> I don't have access to the el3mon firmware code to speak super
> authoritatively about it, but you're correct that it is essentially
> working around a firmware limitation.
> 
> What I initially observed whilst working on suspend to RAM, when
> hotplugging CPU's with just the PSCI calls the system hangs. Debugging
> this and tracing versus the downstream production drivers the missing
> piece was programming the "ACPM hint" to the CPU_INFORM registers.
> Further debugging and power measurements also showed that the ACPM
> hint is also required in addition to PSCI calls for the cpuidle states
> to function correctly.
> 

It is definitely worth adding all the above info and shaming the firmware
for not taking care of this. I still don't like this as PSCI is there
for nearly a decade now and still we see such limitations that needs to
be fixed in the firmware and now workaround in the kernel.

> > /*
> >  * CPU_INFORM register hint values which are used by
> >  * EL3 firmware (el3mon).
> >  */
> >
> > This clearly sounds like workaround for the firmware limitations. That
> > needs to be clearly documented IMO.
> 
> Sure I can add a more verbose comment, that this is required to work
> around firmware limitations in the PSCI implementation.
> 

Thanks that's what we need to fully understand the need of otherwise
useless dance around hotplug and idle state machinery in the kernel.

-- 
Regards,
Sudeep