Re: [PATCH v4 2/2] ACPI: CPPC: Add ospm_nominal_perf support

On 01/06/26 23:07, Rafael J. Wysocki wrote:
> External email: Use caution opening links or attachments
>
>
> On Wed, May 27, 2026 at 9:47 PM Sumit Gupta <[email protected]> wrote:
>> Expose the OSPM Nominal Performance register (ACPI 6.6, Section
>> 8.4.6.1.2.6), which conveys the desired nominal performance level
>> at which the platform may run. Unlike the existing read-only
>> Nominal Performance register, it is writable and lets OSPM
>> request a lower nominal level than the platform-reported nominal.
>> The platform classifies performance above this level as boosted
>> and below as throttled for its power/thermal decisions.
>>
>> It is exposed as a per-policy cpufreq sysfs attribute in kHz, to
>> match the cpufreq sysfs unit convention:
>>
>>    /sys/devices/system/cpu/cpufreq/policyN/ospm_nominal_freq
>>
>> The attribute is documented in
>> Documentation/ABI/testing/sysfs-devices-system-cpu.
>>
>> Writes are converted to perf via cppc_khz_to_perf(), validated
>> against [Lowest Performance, Nominal Performance], and applied to
>> every CPU in policy->cpus.
>>
>> The register is write-only; the kernel caches the last written
>> value in struct cppc_cpudata for sysfs readback (returns 0 until
>> userspace writes a value).
>>
>> Signed-off-by: Sumit Gupta <[email protected]>
> There is some sashiko.dev feedback on this one that is valid AFAICS:
>
> https://sashiko.dev/#/patchset/20260527194626.185286-1-sumitg%40nvidia.com


Thanks for sharing. Will address all four in v5.


 > Could this validation reject valid frequencies on systems with CPPC
 > favored cores?

1.
In v5, moved the range check into store_ospm_nominal_freq() in
cppc_cpufreq.c, where it uses the same cpu_data->perf_caps as the
kHz->perf conversion. OSPM Nominal is treated as a single per-policy
value. It's validated once against the policy caps and applied to all
CPUs in policy->cpus. This avoids validating against a sibling's
separately-read caps.


 > When the platform's ACPI _CPC table does not support this register,
 > will this unconditionally return '0' instead of an error like
 > -EOPNOTSUPP?

2.
In v5, added cppc_get_ospm_nominal_perf() and dropped the cached
value. show() now reads the register and returns "<unsupported>"
on -EOPNOTSUPP.


 > Are newly onlined CPUs missing this cached ospm_nominal_perf value?

3.
show() now reads the register directly and not cached value, so it
always reflects true per-CPU state.


 > Does skipping this rollback loop when prev_set is false leave the CPUs
 > in a desynchronized state?

In v5, the cache and ospm_nominal_perf_set bool are removed and
show() reads the register, so a partial write is no longer hidden.
sysfs shows the real per-CPU value, not '0').
store() reads the current value before writing and restores it on
already updated CPUs if a sibling write fails, so there is no
first write skip.

Thank you,
Sumit Gupta
....