Re: [PATCH v9 1/8] ACPI: CPPC: Order performance-control updates

[Sumit Gupta]

Hi Rafael,


On 07/11/26 01:02, Sumit Gupta wrote:
> From: Christian Loehle <christian.loehle@xxxxxxx>
>
> The ACPI specification requires Minimum Performance to remain below
> Maximum Performance. When autonomous selection is disabled, Desired
> Performance must also remain within those limits.
>
> cppc_set_perf() writes the three controls in a fixed order. This can expose
> an invalid tuple after firmware resets the controls or while policy bounds
> move across the current desired value.
>
> Read the live limits. Apply non-atomic updates in three phases: widen the
> interval, update Desired Performance, then narrow it. Submit PCC controls
> in each phase together. Keep the existing single-command path when the
> complete tuple is in PCC, reject invalid requests, and stop after the first
> failed access.
>
> Make cppc_cpufreq fast switches include the limits only when they differ
> from the last successful request. Invalidate the cache across resume and
> after a failed request, since firmware may have reset the controls or an
> earlier phase of a failed update may have reached the platform. Publish
> slow-path limits only after the complete request succeeds.
>
> At policy teardown, lower Minimum Performance together with Desired
> Performance so the final request remains valid. On initialization, clamp
> Desired Performance to the live interval left by firmware or an earlier
> driver instance.
>
> [sumitg:
>   - In cppc_read_perf_limits(), skip an unimplemented MIN or MAX, and
>     do not fail on a live MIN above MAX.
>   - Clamp Desired into the new MIN and MAX instead of rejecting it, as
>     amd-pstate resume can round it above MAX.
>   - In init(), request the full range if the live MAX is zero or below MIN.
>   - Clamp Desired in set_target(), fast_switch() and store_auto_select(),
>     to keep the cached value in range.
>   - In store_auto_select(), publish the request only after success.]
>
> Fixes: 76531df5e13b ("ACPI: CPPC: Add min and max perf register writing support")
> Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
> Link: https://lore.kernel.org/all/01d7f07e-839c-428b-833d-713ed2128f4d@xxxxxxxxxx/
> Signed-off-by: Sumit Gupta <sumitg@xxxxxxxxxx>
> ---


Sashiko flagged three issues on this patch [1].
I think none of them needs a change in this patch:

1) SHARED_TYPE_ANY, when policy->cpu changes: the driver uses one set
   of controls for all CPUs in the policy, and always writes them
   through policy->cpu. So the MIN and MAX written earlier still apply.

2) MIN and MAX not written when the requested frequency is the current
   one: not new with this patch. Since commit 9801be8bef65 ("cpufreq:
   Avoid redundant target() calls for unchanged limits"), the core
   skips ->target() in that case unless the limits changed, also for
   the first request after the governor starts. I will send a core fix
   for this separately.

3) store_auto_select() racing with the governor: this is not new
   either. Before this patch, both edited the shared
   cpu_data->perf_ctrls. Now each builds its own request.
   store_auto_select() holds policy->rwsem, so the policy limits cannot
   change while it runs. store_auto_select() and the governor both take
   MIN and MAX from these limits, and keep Desired between them. So
   MIN <= Desired <= MAX holds, whichever write lands first. At worst,
   Desired keeps the older value until the next governor update.

[1] 
https://sashiko.dev/#/patchset/20261106193304.3347374-1-sumitg@xxxxxxxxxx

Thanks,
Sumit

....