Re: [PATCH v4 15/15] ACPI: CPPC: Clear Performance Limited without a stale read

[Sumit Gupta <sumitg@nvidia.com>]

On 26/08/26 12:00, Christian Loehle wrote:
> External email: Use caution opening links or attachments
>
>
> The Performance Limited status bits are sticky and write-zero-to-clear.
> ACPI 6.6 Section 8.4.6.1.3.2 also requires both entities to use interlocked
> accesses.
>
> cppc_set_perf_limited() currently reads the register, computes a new value,
> and writes it in a separate transaction. If the platform reports another
> excursion between those transactions, the stale write can clear that new
> event.
>
> Write zero to the requested bits and one to the other defined status bits
> directly. Keep reserved bits zero as required for hardware status registers
> by ACPI 6.6 Section 4.6.1. This removes the stale read window.
>
> A partial SystemMemory field would still make the generic writer perform a
> read-modify-write to preserve the containing access unit. The
> per-descriptor spinlock cannot interlock that RMW with platform updates, so
> reject clears of such a field. Keep the descriptor mapped and readable,
> because reading the containing access unit once and extracting the field
> does not require RMW.
>
> Also reject another writable SystemMemory field sharing Performance
> Limited's access unit. Its RMW could similarly replay stale status bits,
> and an OSPM lock cannot serialize against the platform.
>
> Also reject 64-bit SystemMemory descriptions on 32-bit kernels, where
> generic readq()/writeq() may be split into two 32-bit operations and cannot
> provide the required portable interlocked access. A naturally aligned
> full-width QWord remains supported on 64-bit kernels, where the
> architecture provides a native 64-bit MMIO accessor.
>
> Performance Limited status is not required for CPPC control. If firmware
> describes it using an access which Linux cannot read safely, disable that
> status register instead of rejecting the processor's otherwise usable _CPC
> package. Report reads as unsupported rather than returning a synthetic
> zero, and emit a single warning for each nonfatal fallback.
>
> Fixes: 13c45a26635f ("ACPI: CPPC: add APIs and sysfs interface for perf_limited")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---

Performance Limited is readable but not clear-able on my test platform,
with a warning on every CPU:

   ACPI CPPC: CPU0: Performance Limited register cannot be cleared 
safely; keeping it readable

This occurs with both _CPC revision 3 and revision 4.

Could clearing remain supported?
cppc_set_perf_limited() now builds the W0C value without reading the
register first. Here the field is 2 bits at Bit Offset 0 in a 32-bit
access unit. If Section 4.6.1 allows bits 31:2 to be treated as
reserved status bits, the value could be issued as a single 32-bit write
with those bits zero. That avoids RMW.

A full-width description already writes zero to the reserved bits.
Is the same treatment valid for bits outside the GAS Bit Width when
Bit Offset is zero?

Apart from this, cppc_cpufreq probes and basic frequency scaling work
on this platform.

Thanks,
Sumit
....