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

[Rafael J. Wysocki (Intel)]

On Thu, Sep 17, 2026 at 2:59 PM Christian Loehle
<christian.loehle@xxxxxxx> wrote:
>
> 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.

Sure, thanks!

Sumit, any concerns?