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

[Rafael J. Wysocki (Intel)]

On Wed, Jul 1, 2026 at 6:25 PM Pierre Gondois <pierre.gondois@xxxxxxx> wrote:
>
> 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

This sounds about right to me.