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

[Christian Loehle]

On 9/16/26 17:28, Christian Loehle wrote:
> From: Sumit Gupta <sumitg@xxxxxxxxxx>
> 
> 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@xxxxxxx/
> Signed-off-by: Sumit Gupta <sumitg@xxxxxxxxxx>
> [ Rework quirk matching and fixup placement; propagate lookup failures
>   without caching them. ]
> Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
> ---
>  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@xxxxxxxxxx/
So once Sumit adds Tested-by: this should be fine.

>[snip]