Re: [PATCH] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload

[Pierre Gondois]

Hello Rafael, Sumit,

On 6/24/26 15:20, Rafael J. Wysocki wrote:
> On Wed, Jun 24, 2026 at 2:56 PM Sumit Gupta<sumitg@xxxxxxxxxx> wrote:
> > Hi Rafael,
> >
> >
> > On 23/06/26 16:30, Rafael J. Wysocki wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Tue, Jun 23, 2026 at 11:54 AM Sumit Gupta<sumitg@xxxxxxxxxx> wrote:
> > > > Values written to OSPM-set CPPC registers (via sysfs or the autonomous
> > > > boot parameter) can be lost in two ways:
> > > >
> > > >     - Across CPU hotplug: the platform may reset a CPU's registers when it
> > > >       is offlined.
> > > >     - On driver unload: the value the driver wrote is left in the register
> > > >       instead of returning to its pre-driver state.
> > > >
> > > > Add a small table-driven mechanism that handles both:
> > > >
> > > >     - Capture each register's firmware value when a CPU is first seen and
> > > >       restore it on driver unload.
> > > >     - Record the last value the driver set and reapply it from ->init()
> > > >       when the policy is reactivated after CPU hotplug.
> > > I'm not sure if this is a good idea TBH.
> > >
> > > The overall system state when the CPU goes online may be completely
> > > different from the system state when the CPU was online last time, so
> > > there is no reason to restore its settings from before offline, at
> > > least in principle.
> > These are values userspace deliberately set, more like a frequency QoS
> > request that persists until userspace changes it than transient state.
> > It's platform dependent though, as the platform I test on preserves them
> > across hotplug, but where the platform resets the register on offline
> > the value is silently lost. Should the kernel re-apply it on online,
> > or is that better left to userspace? If so, I will drop the hotplug reapply
> > and keep only the restore on driver unload.
> >
> > intel_pstate and amd-pstate already re-sync these on CPU online, as they
> > keep the policy across hotplug via ->online()/->offline(). So for them
> > it's re-syncing the hardware, not restoring state from before offline.
> >
> > cppc_cpufreq has no ->online()/->offline() today, so it fully tears the
> > policy down and rebuilds it, which is why this reads as "restoring
> > settings from before offline".
> > Would adding ->online()/->offline() be acceptable, with policy preserved
> > and ->online() just re-syncing the registers the platform reset?
> That would be a better approach IMV.
Just an example to be sure I understand correctly.

1. on a platform, auto_sel=0 by default
2. cppc_cpufreq driver is loaded + we set auto_sel=1
3. all the CPUs of a policy are unplugged
4. cppc_cpufreq driver is unloaded

To be sure we let the platform in the default state,
we should set auto_sel=0 in step A.3, i.e. in the
.offline() callback.
Doing it at A.4 (i.e. in the .exit()) would not be possible:
the CPU's registers might not be available anymore once offline.

IoW, we should only need:
.init()
\-cppc_cpufreq_save_firmware_regs()
firmware_captured=0, save the firmware regs.

.online()
\-cppc_cpufreq_save_firmware_regs()
firmware_captured=1, restore the user (or previous)
values that were saved in the previous .offline() call
(as .online() is not called for new policies).

.offline()
\-cppc_cpufreq_restore_firmware_regs()
Save the user/current regs values
+ Restore the firmware registers.

And it should be ok to remove:
- cppc_cpufreq_reapply_requested_regs()
- the firmware_captured field.
If during .init(), we init requested_val to the firmware
values aswell, firmware_captured should be useless