Re: [PATCH v7 15/20] ACPI: CPPC: Keep Performance Limited clearable on NVIDIA T41

[Sumit Gupta <sumitg@nvidia.com>]

On 17/09/26 18:53, Rafael J. Wysocki (Intel) wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Sep 17, 2026 at 2:59 PM Christian Loehle
> <christian.loehle@arm.com> wrote:
>>
>> On 9/16/26 17:28, Christian Loehle wrote:
>>> From: Sumit Gupta <sumitg@nvidia.com>
>>>
>>> NVIDIA T41 firmware describes Performance Limited as a two-bit field at
>>> offset zero in a DWord SystemMemory access unit. Generic CPPC code must
>>> keep such a field read-only because preserving the remainder when clearing
>>> it requires a read-modify-write which cannot be interlocked with platform
>>> updates.
>>>
>>> The remaining bits of this access unit are unimplemented on T41: they read
>>> as zero, writes have no side effects, and no other register uses them. The
>>> Performance Limited register therefore owns the complete access unit, but
>>> shipped firmware does not describe that property accurately.
>>>
>>> Add a CPPC platform-quirk table keyed by the DSDT header and carry quirk
>>> behavior through explicit flags. Cache a successful table lookup, copy each
>>> GAS into the driver's private descriptor, and apply fixups before layout
>>> validation, mapping and overlap registration.
>>>
>>> Distinguish a genuine non-match from a table-header lookup failure in
>>> acpi_match_platform_list(). Propagate lookup errors from CPPC probe without
>>> caching them, so a transient mapping failure cannot disable the workaround
>>> for every later processor. Existing matcher callers still treat all
>>> negative results as no match.
>>>
>>> For the known T41 layout only, widen a two-bit Performance Limited field at
>>> offset zero to its 32-bit access width. Clearing both status bits can then
>>> be issued as one DWord write of zero without a stale read. Corrected
>>> firmware which reports the full width is unchanged. The workaround
>>> therefore lapses automatically when corrected firmware ships.
>>>
>>> Link: https://lore.kernel.org/lkml/d5f1ea9b-53b7-4db2-983a-b5be8e71a371@arm.com/
>>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>>> [ Rework quirk matching and fixup placement; propagate lookup failures
>>>    without caching them. ]
>>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>>> ---
>>>   drivers/acpi/cppc_acpi.c | 72 ++++++++++++++++++++++++++++++++++++++++
>>>   drivers/acpi/utils.c     | 13 ++++++--
>>>   2 files changed, 82 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>> index 6d130381245e..0e218f2be0fe 100644
>>> --- a/drivers/acpi/cppc_acpi.c
>>> +++ b/drivers/acpi/cppc_acpi.c
>>> @@ -330,6 +330,68 @@ static unsigned int cpc_reg_access_width(const struct cpc_reg *reg)
>>>        return reg->bit_width;
>>>   }
>>>
>>> +enum cpc_platform_quirk {
>>> +     CPC_QUIRK_PERF_LIMITED_OWNS_UNIT = BIT(0),
>>> +};
>>> +
>>> +static const struct acpi_platform_list cpc_platform_quirk_list[] = {
>>> +     {
>>> +             .oem_id = "NVIDIA",
>>> +             .oem_table_id = "T41",
>>
>> Sashiko:
>> "Will this quirk successfully match a standard ACPI table header?
>> The ACPI specification requires the OEM Table ID to be exactly 8 bytes long,
>> typically padded with trailing spaces by compliant firmware (e.g.,
>> "T41     ").
>> Looking at acpi_match_platform_list(), it compares the IDs using:
>> strncmp(plat->oem_table_id, hdr.oem_table_id, ACPI_OEM_TABLE_ID_SIZE)
>> Because "T41" is a null-terminated 3-character string, strncmp() will
>> compare the 4th character ('\0' from the quirk definition vs ' ' from the
>> ACPI table header) and immediately report a mismatch, causing the quirk to
>> silently fail on compliant firmware.
>> Should this be padded with spaces (e.g., "T41     ") to ensure it matches
>> the firmware's table correctly?"
>>
>> non-padded "T41" matches exactly what Sumit proposed and was discussed in v6:
>> https://lore.kernel.org/lkml/55a5c9fa-cfd3-4000-b3cc-52c343841c9f@nvidia.com/
>> So once Sumit adds Tested-by: this should be fine.
> 
> Sure, thanks!
> 
> Sumit, any concerns?

Hi Rafael,

No concerns.
"T41" without space padding is correct because this firmware NUL pads
the eight-byte field:

  # od -An -tc -j 16 -N 8 /sys/firmware/acpi/tables/DSDT
    T   4   1  \0  \0  \0  \0  \0

The quirk matched, producing the following message during boot:

  ] ACPI CPPC: firmware quirk: Performance Limited owns its access unit, 
using Bit Width 32

Thanks,
Sumit