* Re: [PATCH v7 15/20] ACPI: CPPC: Keep Performance Limited clearable on NVIDIA T41 [not found] ` <CAJZ5v0g-ipGKWyEUKrBGTo+B6RYqWxba4KhK9RaCJmh2MRLtLA@mail.gmail.com> @ 2026-09-17 18:20 ` Sumit Gupta 0 siblings, 0 replies; 3+ messages in thread From: Sumit Gupta @ 2026-09-17 18:20 UTC (permalink / raw) To: Rafael J. Wysocki (Intel), Christian Loehle Cc: Viresh Kumar, linux-pm, linux-acpi, linux-kernel, Jie Zhan, Lifeng Zheng, Pierre Gondois, Sudeep Holla, Ionela Voinescu, zhongqiu.han, linux-tegra@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v7 0/20] ACPI: CPPC: Fix register access and lifetime bugs [not found] <20260916162805.1039247-1-christian.loehle@arm.com> [not found] ` <20260916162805.1039247-16-christian.loehle@arm.com> @ 2026-09-17 18:30 ` Sumit Gupta 2026-09-18 15:30 ` Rafael J. Wysocki (Intel) 1 sibling, 1 reply; 3+ messages in thread From: Sumit Gupta @ 2026-09-17 18:30 UTC (permalink / raw) To: Christian Loehle, Rafael J . Wysocki, Viresh Kumar Cc: linux-pm, linux-acpi, linux-kernel, Len Brown, Jie Zhan, Lifeng Zheng, Pierre Gondois, Sudeep Holla, Ionela Voinescu, zhongqiu.han, linux-tegra@vger.kernel.org Hi Christian, On 16/09/26 21:57, Christian Loehle wrote: > External email: Use caution opening links or attachments > > > This series fixes malformed _CPC handling, control-write error propagation, > PCC ownership and cleanup, CPC sysfs lifetime, register-field access, > cross-processor aliases, and Performance Limited clearing. > > Series structure > ================ > > Patches 1-7 cover parsing and control semantics, error propagation, > PCC update serialization, and descriptor/PCC cleanup. Patches 8-15 > validate the register layouts and aliases which the existing accessors and > locking can safely support, and correct Performance Limited clearing. > Patch 16 bounds x86 and arm64 FFH fields before accessing hardware, checks > x86 MSR numbers, and dispatches FFH before generic Access Size decoding. > Patch 17 propagates failed cross-CPU FFH calls on arm64 and RISC-V before > using callback output, including counter reads for an offline CPU. > Patch 18 accepts a request to retain immutable Autonomous Selection as a > no-op, rejects unsupported amd-pstate transitions to passive mode before > changing state or removing the driver, and restores cppc-cpufreq's previous > mode if a later bounds update fails. The immutable-capability query is > serialized with descriptor publication and removal. > Patch 19 selects the frequency-invariance callback per CPU, avoiding > uninitialized deferred work in policies mixing PCC and direct counters. > Patch 20 creates the FIE worker when policy initialization encounters PCC > counters, before publishing callbacks. This covers offline and later-added > PCC members without making direct-counter FIE depend on worker setup. > > The layout fixes are more substantial: a physical access unit can contain > several fields or be shared across processors and _PSD domains. Probe-time > interval registries enforce the assumptions of the existing > descriptor-local locking model. They add no lookup to the scheduler path; > standalone full-width SystemMemory writes remain lockless. Disjoint fields > within one descriptor retain their shared lock when their access units > overlap, including a full-width byte beside a wider partial-field access. > > Behavior description > ================== > > The parser validates the package before indexing its entries and the > Generic Register descriptor consumed by the driver. It tolerates trailing > package/ResourceTemplate data and legacy Integer-zero absent controls. > Buffer-backed capabilities and bound readbacks are checked before narrowing > to u32. Minimum and Maximum Performance must form a usable pair, and > an explicit minimum-valid flag preserves a zero Minimum Performance > readback across cpufreq initialization and exit. Maximum Performance and > amd-pstate's unsaved firmware-minimum restore requests retain their existing > zero-means-omitted convention. > > Lowest Performance must remain nonzero, as before the series. Accepting zero > needs separate changes to consumers which assume a zero frequency origin > or use a zero capability as an absence sentinel. This does not change the > explicit zero Minimum Performance update used to remove a lower bound. > Invalid firmware-provided Lowest/Nominal Frequency values remain an error. > > PCC ownership is acquired before staging payload updates. Direct-only, > batched PCC, and mixed-address-space performance updates retain their > separate ordering requirements. PCC lifecycle locking and payload-copy > serialization protect different state. > > Frequency-invariance policy teardown remembers which PCC work was > initialized, so it can drain that work after processor removal unpublishes > the CPC descriptor. Callback selection uses that same per-CPU initialization > state rather than looking up the potentially removed descriptor again. > Worker creation is serialized and occurs only when initializing PCC > counters, before that policy publishes callbacks. Setup failure leaves > that policy without FIE, but does not disable direct-counter or already > active policies. Direct-only systems create no worker or deadline > reservation. Direct-counter CPUs continue updating in the tick. > This does not solve the separate runtime-accessor lifetime races with > processor removal (which I will post in a later series). > > SystemMemory permits read-only overlaps, supported exact writable aliases, > and safe descriptor-local partial writes. A full-width writer may share a > larger read access unit across descriptors when their logical fields are > disjoint. Only controls written by the CPPC library count as competing > writers (i.e. OSPM Nominal Performance is not written). Its write-only > readback protections still apply. The validation rejects cross-descriptor > partial writers which its locks cannot serialize and accesses which replay > a write-only neighbour's undefined readback. SystemIO permits read-only > aliases with write-only controls, but not with readable controls whose > readback would select the wrong register. Unsupported optional controls are > disabled only where doing so cannot silently change the operating mode. > Present but inaccessible Enable or Autonomous Selection controls fail > probe, including sub-byte PCC forms which the existing writer cannot > program. This may reject firmware-enabled configurations that previously > worked until the driver attempted an unsupported control access. > Optional-writer fallback only changes unpublished descriptors. A conflict > with an already-published writer can still fail probe; making recovery > independent of CPU discovery order requires runtime quiescence support. > > Performance Limited is sticky, but ACPI does not define the effect of > writing one. A selective clear can either replay stale status or set a bit > on a plain read/write implementation. Write 0x3 to the perf_limited > attribute to clear both bits with one literal-zero register write; > selective clears return -EOPNOTSUPP. Partial SystemMemory/SystemIO status > fields remain readable but cannot be cleared generically. This is a status > acknowledgement, not a lossless event log. > > Inaccessible status ranges remain visible to overlap validation without > being accessed. Sumit Gupta's NVIDIA T41 quirk widens only the verified > two-bit, offset-zero DWord descriptor whose remaining bits are unimplemented, > allowing that platform to use the safe clear-all operation. > > The general SystemMemory/SystemIO alignment checks retain the x86 exception > for unaligned accesses. Performance Limited currently inherits that policy. > This is not a guarantee of an indivisible device transaction at an unaligned > address; the stricter status-atomicity question remains separate work. No > torn access or affected platform has been demonstrated by the reviews. > > Testing > ======= > > An earlier iteration was built and booted on Orion O6-01, with CPPC sysfs, > frequency changes, advancing counters and module reloads checked. The > subsequent AMD transition guard and FIE fixes have passed source/model > checks and checkpatch, but this tip has not yet been built or booted. > Those checks are not AMD hardware, mixed-PCC hotplug or concurrency tests. > > Changes since v6 > ================ > > Patch 1: Validate the _CPC package header > Unchanged from v6. > > Patch 2: Validate _CPC entry and control semantics > Allow the full 64-bit Integer Counter Wraparound Time and emit FW_BUG > once for tolerated legacy Integer-zero Buffer placeholders. (Rafael) > Drop v6's zero Lowest Performance support: nonzero frequency anchors do > not make it safe for all consumers. Retain the pre-series nonzero check. > Reject Minimum/Maximum readbacks above U32_MAX before narrowing them. > Keep only min_perf_valid: an explicit zero minimum removes a lower bound, > whereas zero Maximum Performance retains its legacy omitted-update > meaning. Document the flag and leave it clear for amd-pstate's unsaved > firmware-minimum restore requests. > > Patch 3: Propagate performance-control write errors > Adjust for patch 2's minimum-only validity flag; a zero maximum remains > an omitted update. > > Patch 4: Serialize PCC single-register payload updates (v6 patch 5) > Reject unsupported PCC widths before taking the exclusive lock, so a > malformed request cannot abort an older valid performance batch. > Match both subspace and generation when completing failed PCC writes; > equal generation numbers in another subspace must not receive the error. > > Patch 5: Serialize PCC EPP payload updates (v6 patch 6) > Preflight PCC widths and SystemIO writer geometry before changing any > direct control. This keeps the patch independently safe before the later > probe-time layout validation. Pass the subspace to error completion. > Patches 10 and 11 remove these preflights once probe guarantees them. > > Patch 6: Release CPC descriptors through kobject (v6 patch 7) > Track initialized PCC frequency-invariance work independently of the CPC > descriptor, so policy exit still drains it after descriptor unpublication. > Make the counter-transport query tolerate a missing descriptor. Broader > runtime-accessor lifetime protection remains deferred. > > Patch 7: Release PCC data after probe failures (v6 patch 8) > No functional changes from v6. Only clarify the channel-reuse comment. > > Patch 8: Reject unsafe cross-CPU SystemMemory RMW (v6 patch 9) > Mark successful relaxed MMIO writes pending for lock-handoff ordering. > Use the interval walk to lock both same-descriptor overlapping accesses, > including mixed-width full/partial fields, without another overlap pass. > Permit a full-width writer beside a disjoint logical read-only field, > including across descriptors when the reader uses a larger access unit. > Do not count OSPM Nominal Performance as a competing writer: Linux does > not write it. Its write-only readback protections remain in patch 9. > Exact-alias coalescing and representative promotion were already in v6 > and remain present; they are not additions in this version. > > Patch 9: Reject direct reads of write-only controls (v6 patch 10) > Also reject readable SystemMemory fields whose logical bits overlap a > write-only control, including retained inaccessible fields. Treat a > retained zero-width field conservatively before the no-writer shortcut. > Reuse the logical-field overlap check instead of a separate full-width > ownership helper. Remove the duplicate Desired Performance getter check; > the common getter already enforces write-only semantics. > > Patch 10: Validate and access PCC register layouts (v6 patch 11) > Fail probe for a present inaccessible Autonomous Selection control, > instead of hiding it without establishing the hardware's operating mode. > Separate the final bound-pair check from mandatory-control validation. > Remove setter PCC-width preflights once probe validates every published > writer, and remove unreachable PCC branches in the memory accessors. > Keep actual access-error handling and ownership serialization. > > Patch 11: Validate SystemIO register layouts (v6 patch 12) > Allow a read-only port alias with a write-only control, but reject aliases > with readable controls whose readback would select the wrong register. > Remove the EPP setter's duplicate SystemIO geometry preflight once probe > enforces it. Read-only partial-field support and the x86 exception to > natural alignment were already in v6 and remain unchanged. > > Patch 12: Validate PCC overlaps across processors (v6 patch 13) > Replace the separate same-descriptor overlap pass with registry checks. > Drop v6's expansion to additional byte-multiple PCC writer widths; retain > the existing 8/16/32/64-bit widths, subject to control-specific limits. > Keep one interval record per descriptor entry; PCC alias coalescing is > deferred. Correct Fixes to 80b8286aeec0 (CPPC request batching). > > Patch 13: Validate SystemIO overlaps across processors (v6 patch 14) > Apply patch 11's direction-aware alias policy across processors and > remove the now-redundant same-descriptor overlap pass. Retain per-entry > interval records; SystemIO alias coalescing remains deferred. > > Patch 14: Clear Performance Limited without a stale read (v6 patch 15) > Stop assuming write-zero-to-clear semantics for written ones. Reject > selective clears and implement clear-all (sysfs 0x3) with one literal-zero > register write, avoiding both stale readback and fabricated status. (Sumit) > Extend the readable-but-not-clearable fallback to partial SystemIO fields. > Extend inaccessible-range retention to PCC and SystemIO, including status > crossing the PCC payload end or port 0xffff. PCC claims retain their own > subspace without selecting or retaining an unnecessary access channel. > Permit disabling only unpublished, disjoint optional SystemMemory writers > which share a status access unit; never hide a required or Autonomous > Selection control. Published writers are not changed during another probe. > Compare bit geometry as well as byte ranges for exact PCC/SystemIO aliases, > and count only accessible controls as writers in those registries. > Make status fallback warnings once-only and document the clear-all and > unsupported-access behavior in the sysfs ABI. > > Patch 15: Keep Performance Limited clearable on NVIDIA T41 (new) > Add Sumit's quirk for the verified two-bit, offset-zero DWord layout. > Use a cached DSDT match and quirk flags, applying the fixup to the private > GAS copy before validation and mapping. Distinguish lookup failures from > non-matches and leave failures retryable. Keep all quirk plumbing here, > so patch 14 remains independently buildable without unused declarations. > > Patch 16: Validate FFH register fields before hardware access (new) > Reject invalid x86 field geometry and GAS addresses that would truncate > to another 32-bit MSR number. Bound arm64 AMU fields in both single and > paired readers. Dispatch FFH before generic Access Size decoding can > shift by an invalid amount using an architecture-specific field. > > Patch 17: Propagate errors from cross-CPU FFH calls (new) > Return failed SMP-call errors on arm64 and RISC-V before consuming > uninitialized callback output. Also check RISC-V callback errors before > copying a read value. Keep this as a separate backportable error-path fix. > > Patch 18: Accept requests to retain immutable autonomous selection (new) > Treat setting immutable Autonomous Selection Integer 1 to one as a > successful no-op; disabling it still fails. Distinguish immutable-one > descriptors from writable controls currently reading one. Reject an > unsupported amd-pstate transition to passive before changing the mode or > unregistering the working driver, checking known offline CPUs too. > Serialize the immutable query with descriptor publication/removal, > including probe failure, since CPU-hotplug locking does not pin it. > On a later bounds-update failure, restore cppc-cpufreq's previous mode > rather than always disabling selection. > > Patch 19: Select the frequency-invariance callback per CPU (new) > Register the PCC callback only on CPUs with PCC counters. A shared policy > can also contain direct-counter CPUs, whose irq_work is not initialized. > Select from the recorded work-initialization state, so descriptor removal > between initialization and publication cannot change the callback choice. > Keep callback registration after the complete counter-initialization > pass, so an online CPU's failed initial read publishes no CPPC callbacks. > > Patch 20: Create the FIE worker before enabling PCC callbacks (new) > Create the worker when initializing PCC counters, rather than relying > on an online-only startup scan. Serialize creation and reuse it until > driver teardown. Offline members and later PCC policies are covered. > On setup failure, skip FIE for that policy without changing the global > setting or affecting existing policies. Direct-only systems do not > allocate an unused worker or consume deadline admission bandwidth. > > Dropped v6 patch 4: Use 64-bit masks for register fields > All supported CPPC configurations are already 64-bit, so this is only > a cleanup I'll submit later on. > > Separate lifetime work > ====================== > > The PCC mailbox teardown fixes are posted separately: [1] is their cover, > [2] frees the channel before unmapping its shared memory, and [3] serializes > channel updates with shared-memory teardown. They are not included here; > this series alone does not close the mailbox IRQ-teardown races. > > [1] https://lore.kernel.org/all/20260903111328.805352-1-christian.loehle@arm.com/ > [2] https://lore.kernel.org/all/20260903111328.805352-2-christian.loehle@arm.com/ > [3] https://lore.kernel.org/all/20260903111328.805352-3-christian.loehle@arm.com/ > > Christian Loehle (19): > ACPI: CPPC: Validate the _CPC package header > ACPI: CPPC: Validate _CPC entry and control semantics > ACPI: CPPC: Propagate performance-control write errors > ACPI: CPPC: Serialize PCC single-register payload updates > ACPI: CPPC: Serialize PCC EPP payload updates > ACPI: CPPC: Release CPC descriptors through kobject > ACPI: CPPC: Release PCC data after probe failures > ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW > ACPI: CPPC: Reject direct reads of write-only controls > ACPI: CPPC: Validate and access PCC register layouts > ACPI: CPPC: Validate SystemIO register layouts > ACPI: CPPC: Validate PCC overlaps across processors > ACPI: CPPC: Validate SystemIO overlaps across processors > ACPI: CPPC: Clear Performance Limited without a stale read > ACPI: CPPC: Validate FFH register fields before hardware access > ACPI: CPPC: Propagate errors from cross-CPU FFH calls > ACPI: CPPC: Accept requests to retain immutable autonomous selection > cpufreq: CPPC: Select the frequency-invariance callback per CPU > cpufreq: CPPC: Create the FIE worker before enabling PCC callbacks > > Sumit Gupta (1): > ACPI: CPPC: Keep Performance Limited clearable on NVIDIA T41 > > Documentation/ABI/testing/sysfs-devices-system-cpu | 16 +- > arch/arm64/kernel/topology.c | 14 +- > arch/x86/kernel/acpi/cppc.c | 14 + > drivers/acpi/cppc_acpi.c | 2209 +++++++++++++++++--- > drivers/acpi/riscv/cppc.c | 26 +- > drivers/acpi/utils.c | 13 +- > drivers/cpufreq/amd-pstate.c | 31 +- > drivers/cpufreq/cppc_cpufreq.c | 73 +- > include/acpi/cppc_acpi.h | 18 +- > 9 files changed, 2024 insertions(+), 390 deletions(-) > > base-commit: fd73f4a6659897191fa0d40695fe370925dd3780 > -- > 2.34.1 Tested the v7 on v7.3-rc3, with both _CPC rev 3 and 4. For the entire series: Tested-by: Sumit Gupta <sumitg@nvidia.com> Patch 20/20 had a conflict in drivers/cpufreq/cppc_cpufreq.c with linux-next 20260916. The series applied cleanly to the base commit specified in the cover letter (fd73f4a66598, tag v7.3-rc3), so patch 20/20 may need to be rebased for the current linux-next. Thanks, Sumit ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v7 0/20] ACPI: CPPC: Fix register access and lifetime bugs 2026-09-17 18:30 ` [PATCH v7 0/20] ACPI: CPPC: Fix register access and lifetime bugs Sumit Gupta @ 2026-09-18 15:30 ` Rafael J. Wysocki (Intel) 0 siblings, 0 replies; 3+ messages in thread From: Rafael J. Wysocki (Intel) @ 2026-09-18 15:30 UTC (permalink / raw) To: Sumit Gupta, Christian Loehle Cc: Viresh Kumar, linux-pm, linux-acpi, linux-kernel, Jie Zhan, Lifeng Zheng, Pierre Gondois, Sudeep Holla, Ionela Voinescu, zhongqiu.han, linux-tegra@vger.kernel.org On Thu, Sep 17, 2026 at 8:31 PM Sumit Gupta <sumitg@nvidia.com> wrote: > > Hi Christian, > > > On 16/09/26 21:57, Christian Loehle wrote: > > External email: Use caution opening links or attachments > > > > > > This series fixes malformed _CPC handling, control-write error propagation, > > PCC ownership and cleanup, CPC sysfs lifetime, register-field access, > > cross-processor aliases, and Performance Limited clearing. > > > > Series structure > > ================ > > > > Patches 1-7 cover parsing and control semantics, error propagation, > > PCC update serialization, and descriptor/PCC cleanup. Patches 8-15 > > validate the register layouts and aliases which the existing accessors and > > locking can safely support, and correct Performance Limited clearing. > > Patch 16 bounds x86 and arm64 FFH fields before accessing hardware, checks > > x86 MSR numbers, and dispatches FFH before generic Access Size decoding. > > Patch 17 propagates failed cross-CPU FFH calls on arm64 and RISC-V before > > using callback output, including counter reads for an offline CPU. > > Patch 18 accepts a request to retain immutable Autonomous Selection as a > > no-op, rejects unsupported amd-pstate transitions to passive mode before > > changing state or removing the driver, and restores cppc-cpufreq's previous > > mode if a later bounds update fails. The immutable-capability query is > > serialized with descriptor publication and removal. > > Patch 19 selects the frequency-invariance callback per CPU, avoiding > > uninitialized deferred work in policies mixing PCC and direct counters. > > Patch 20 creates the FIE worker when policy initialization encounters PCC > > counters, before publishing callbacks. This covers offline and later-added > > PCC members without making direct-counter FIE depend on worker setup. > > > > The layout fixes are more substantial: a physical access unit can contain > > several fields or be shared across processors and _PSD domains. Probe-time > > interval registries enforce the assumptions of the existing > > descriptor-local locking model. They add no lookup to the scheduler path; > > standalone full-width SystemMemory writes remain lockless. Disjoint fields > > within one descriptor retain their shared lock when their access units > > overlap, including a full-width byte beside a wider partial-field access. > > > > Behavior description > > ================== > > > > The parser validates the package before indexing its entries and the > > Generic Register descriptor consumed by the driver. It tolerates trailing > > package/ResourceTemplate data and legacy Integer-zero absent controls. > > Buffer-backed capabilities and bound readbacks are checked before narrowing > > to u32. Minimum and Maximum Performance must form a usable pair, and > > an explicit minimum-valid flag preserves a zero Minimum Performance > > readback across cpufreq initialization and exit. Maximum Performance and > > amd-pstate's unsaved firmware-minimum restore requests retain their existing > > zero-means-omitted convention. > > > > Lowest Performance must remain nonzero, as before the series. Accepting zero > > needs separate changes to consumers which assume a zero frequency origin > > or use a zero capability as an absence sentinel. This does not change the > > explicit zero Minimum Performance update used to remove a lower bound. > > Invalid firmware-provided Lowest/Nominal Frequency values remain an error. > > > > PCC ownership is acquired before staging payload updates. Direct-only, > > batched PCC, and mixed-address-space performance updates retain their > > separate ordering requirements. PCC lifecycle locking and payload-copy > > serialization protect different state. > > > > Frequency-invariance policy teardown remembers which PCC work was > > initialized, so it can drain that work after processor removal unpublishes > > the CPC descriptor. Callback selection uses that same per-CPU initialization > > state rather than looking up the potentially removed descriptor again. > > Worker creation is serialized and occurs only when initializing PCC > > counters, before that policy publishes callbacks. Setup failure leaves > > that policy without FIE, but does not disable direct-counter or already > > active policies. Direct-only systems create no worker or deadline > > reservation. Direct-counter CPUs continue updating in the tick. > > This does not solve the separate runtime-accessor lifetime races with > > processor removal (which I will post in a later series). > > > > SystemMemory permits read-only overlaps, supported exact writable aliases, > > and safe descriptor-local partial writes. A full-width writer may share a > > larger read access unit across descriptors when their logical fields are > > disjoint. Only controls written by the CPPC library count as competing > > writers (i.e. OSPM Nominal Performance is not written). Its write-only > > readback protections still apply. The validation rejects cross-descriptor > > partial writers which its locks cannot serialize and accesses which replay > > a write-only neighbour's undefined readback. SystemIO permits read-only > > aliases with write-only controls, but not with readable controls whose > > readback would select the wrong register. Unsupported optional controls are > > disabled only where doing so cannot silently change the operating mode. > > Present but inaccessible Enable or Autonomous Selection controls fail > > probe, including sub-byte PCC forms which the existing writer cannot > > program. This may reject firmware-enabled configurations that previously > > worked until the driver attempted an unsupported control access. > > Optional-writer fallback only changes unpublished descriptors. A conflict > > with an already-published writer can still fail probe; making recovery > > independent of CPU discovery order requires runtime quiescence support. > > > > Performance Limited is sticky, but ACPI does not define the effect of > > writing one. A selective clear can either replay stale status or set a bit > > on a plain read/write implementation. Write 0x3 to the perf_limited > > attribute to clear both bits with one literal-zero register write; > > selective clears return -EOPNOTSUPP. Partial SystemMemory/SystemIO status > > fields remain readable but cannot be cleared generically. This is a status > > acknowledgement, not a lossless event log. > > > > Inaccessible status ranges remain visible to overlap validation without > > being accessed. Sumit Gupta's NVIDIA T41 quirk widens only the verified > > two-bit, offset-zero DWord descriptor whose remaining bits are unimplemented, > > allowing that platform to use the safe clear-all operation. > > > > The general SystemMemory/SystemIO alignment checks retain the x86 exception > > for unaligned accesses. Performance Limited currently inherits that policy. > > This is not a guarantee of an indivisible device transaction at an unaligned > > address; the stricter status-atomicity question remains separate work. No > > torn access or affected platform has been demonstrated by the reviews. > > > > Testing > > ======= > > > > An earlier iteration was built and booted on Orion O6-01, with CPPC sysfs, > > frequency changes, advancing counters and module reloads checked. The > > subsequent AMD transition guard and FIE fixes have passed source/model > > checks and checkpatch, but this tip has not yet been built or booted. > > Those checks are not AMD hardware, mixed-PCC hotplug or concurrency tests. > > > > Changes since v6 > > ================ > > > > Patch 1: Validate the _CPC package header > > Unchanged from v6. > > > > Patch 2: Validate _CPC entry and control semantics > > Allow the full 64-bit Integer Counter Wraparound Time and emit FW_BUG > > once for tolerated legacy Integer-zero Buffer placeholders. (Rafael) > > Drop v6's zero Lowest Performance support: nonzero frequency anchors do > > not make it safe for all consumers. Retain the pre-series nonzero check. > > Reject Minimum/Maximum readbacks above U32_MAX before narrowing them. > > Keep only min_perf_valid: an explicit zero minimum removes a lower bound, > > whereas zero Maximum Performance retains its legacy omitted-update > > meaning. Document the flag and leave it clear for amd-pstate's unsaved > > firmware-minimum restore requests. > > > > Patch 3: Propagate performance-control write errors > > Adjust for patch 2's minimum-only validity flag; a zero maximum remains > > an omitted update. > > > > Patch 4: Serialize PCC single-register payload updates (v6 patch 5) > > Reject unsupported PCC widths before taking the exclusive lock, so a > > malformed request cannot abort an older valid performance batch. > > Match both subspace and generation when completing failed PCC writes; > > equal generation numbers in another subspace must not receive the error. > > > > Patch 5: Serialize PCC EPP payload updates (v6 patch 6) > > Preflight PCC widths and SystemIO writer geometry before changing any > > direct control. This keeps the patch independently safe before the later > > probe-time layout validation. Pass the subspace to error completion. > > Patches 10 and 11 remove these preflights once probe guarantees them. > > > > Patch 6: Release CPC descriptors through kobject (v6 patch 7) > > Track initialized PCC frequency-invariance work independently of the CPC > > descriptor, so policy exit still drains it after descriptor unpublication. > > Make the counter-transport query tolerate a missing descriptor. Broader > > runtime-accessor lifetime protection remains deferred. > > > > Patch 7: Release PCC data after probe failures (v6 patch 8) > > No functional changes from v6. Only clarify the channel-reuse comment. > > > > Patch 8: Reject unsafe cross-CPU SystemMemory RMW (v6 patch 9) > > Mark successful relaxed MMIO writes pending for lock-handoff ordering. > > Use the interval walk to lock both same-descriptor overlapping accesses, > > including mixed-width full/partial fields, without another overlap pass. > > Permit a full-width writer beside a disjoint logical read-only field, > > including across descriptors when the reader uses a larger access unit. > > Do not count OSPM Nominal Performance as a competing writer: Linux does > > not write it. Its write-only readback protections remain in patch 9. > > Exact-alias coalescing and representative promotion were already in v6 > > and remain present; they are not additions in this version. > > > > Patch 9: Reject direct reads of write-only controls (v6 patch 10) > > Also reject readable SystemMemory fields whose logical bits overlap a > > write-only control, including retained inaccessible fields. Treat a > > retained zero-width field conservatively before the no-writer shortcut. > > Reuse the logical-field overlap check instead of a separate full-width > > ownership helper. Remove the duplicate Desired Performance getter check; > > the common getter already enforces write-only semantics. > > > > Patch 10: Validate and access PCC register layouts (v6 patch 11) > > Fail probe for a present inaccessible Autonomous Selection control, > > instead of hiding it without establishing the hardware's operating mode. > > Separate the final bound-pair check from mandatory-control validation. > > Remove setter PCC-width preflights once probe validates every published > > writer, and remove unreachable PCC branches in the memory accessors. > > Keep actual access-error handling and ownership serialization. > > > > Patch 11: Validate SystemIO register layouts (v6 patch 12) > > Allow a read-only port alias with a write-only control, but reject aliases > > with readable controls whose readback would select the wrong register. > > Remove the EPP setter's duplicate SystemIO geometry preflight once probe > > enforces it. Read-only partial-field support and the x86 exception to > > natural alignment were already in v6 and remain unchanged. > > > > Patch 12: Validate PCC overlaps across processors (v6 patch 13) > > Replace the separate same-descriptor overlap pass with registry checks. > > Drop v6's expansion to additional byte-multiple PCC writer widths; retain > > the existing 8/16/32/64-bit widths, subject to control-specific limits. > > Keep one interval record per descriptor entry; PCC alias coalescing is > > deferred. Correct Fixes to 80b8286aeec0 (CPPC request batching). > > > > Patch 13: Validate SystemIO overlaps across processors (v6 patch 14) > > Apply patch 11's direction-aware alias policy across processors and > > remove the now-redundant same-descriptor overlap pass. Retain per-entry > > interval records; SystemIO alias coalescing remains deferred. > > > > Patch 14: Clear Performance Limited without a stale read (v6 patch 15) > > Stop assuming write-zero-to-clear semantics for written ones. Reject > > selective clears and implement clear-all (sysfs 0x3) with one literal-zero > > register write, avoiding both stale readback and fabricated status. (Sumit) > > Extend the readable-but-not-clearable fallback to partial SystemIO fields. > > Extend inaccessible-range retention to PCC and SystemIO, including status > > crossing the PCC payload end or port 0xffff. PCC claims retain their own > > subspace without selecting or retaining an unnecessary access channel. > > Permit disabling only unpublished, disjoint optional SystemMemory writers > > which share a status access unit; never hide a required or Autonomous > > Selection control. Published writers are not changed during another probe. > > Compare bit geometry as well as byte ranges for exact PCC/SystemIO aliases, > > and count only accessible controls as writers in those registries. > > Make status fallback warnings once-only and document the clear-all and > > unsupported-access behavior in the sysfs ABI. > > > > Patch 15: Keep Performance Limited clearable on NVIDIA T41 (new) > > Add Sumit's quirk for the verified two-bit, offset-zero DWord layout. > > Use a cached DSDT match and quirk flags, applying the fixup to the private > > GAS copy before validation and mapping. Distinguish lookup failures from > > non-matches and leave failures retryable. Keep all quirk plumbing here, > > so patch 14 remains independently buildable without unused declarations. > > > > Patch 16: Validate FFH register fields before hardware access (new) > > Reject invalid x86 field geometry and GAS addresses that would truncate > > to another 32-bit MSR number. Bound arm64 AMU fields in both single and > > paired readers. Dispatch FFH before generic Access Size decoding can > > shift by an invalid amount using an architecture-specific field. > > > > Patch 17: Propagate errors from cross-CPU FFH calls (new) > > Return failed SMP-call errors on arm64 and RISC-V before consuming > > uninitialized callback output. Also check RISC-V callback errors before > > copying a read value. Keep this as a separate backportable error-path fix. > > > > Patch 18: Accept requests to retain immutable autonomous selection (new) > > Treat setting immutable Autonomous Selection Integer 1 to one as a > > successful no-op; disabling it still fails. Distinguish immutable-one > > descriptors from writable controls currently reading one. Reject an > > unsupported amd-pstate transition to passive before changing the mode or > > unregistering the working driver, checking known offline CPUs too. > > Serialize the immutable query with descriptor publication/removal, > > including probe failure, since CPU-hotplug locking does not pin it. > > On a later bounds-update failure, restore cppc-cpufreq's previous mode > > rather than always disabling selection. > > > > Patch 19: Select the frequency-invariance callback per CPU (new) > > Register the PCC callback only on CPUs with PCC counters. A shared policy > > can also contain direct-counter CPUs, whose irq_work is not initialized. > > Select from the recorded work-initialization state, so descriptor removal > > between initialization and publication cannot change the callback choice. > > Keep callback registration after the complete counter-initialization > > pass, so an online CPU's failed initial read publishes no CPPC callbacks. > > > > Patch 20: Create the FIE worker before enabling PCC callbacks (new) > > Create the worker when initializing PCC counters, rather than relying > > on an online-only startup scan. Serialize creation and reuse it until > > driver teardown. Offline members and later PCC policies are covered. > > On setup failure, skip FIE for that policy without changing the global > > setting or affecting existing policies. Direct-only systems do not > > allocate an unused worker or consume deadline admission bandwidth. > > > > Dropped v6 patch 4: Use 64-bit masks for register fields > > All supported CPPC configurations are already 64-bit, so this is only > > a cleanup I'll submit later on. > > > > Separate lifetime work > > ====================== > > > > The PCC mailbox teardown fixes are posted separately: [1] is their cover, > > [2] frees the channel before unmapping its shared memory, and [3] serializes > > channel updates with shared-memory teardown. They are not included here; > > this series alone does not close the mailbox IRQ-teardown races. > > > > [1] https://lore.kernel.org/all/20260903111328.805352-1-christian.loehle@arm.com/ > > [2] https://lore.kernel.org/all/20260903111328.805352-2-christian.loehle@arm.com/ > > [3] https://lore.kernel.org/all/20260903111328.805352-3-christian.loehle@arm.com/ > > > > Christian Loehle (19): > > ACPI: CPPC: Validate the _CPC package header > > ACPI: CPPC: Validate _CPC entry and control semantics > > ACPI: CPPC: Propagate performance-control write errors > > ACPI: CPPC: Serialize PCC single-register payload updates > > ACPI: CPPC: Serialize PCC EPP payload updates > > ACPI: CPPC: Release CPC descriptors through kobject > > ACPI: CPPC: Release PCC data after probe failures > > ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW > > ACPI: CPPC: Reject direct reads of write-only controls > > ACPI: CPPC: Validate and access PCC register layouts > > ACPI: CPPC: Validate SystemIO register layouts > > ACPI: CPPC: Validate PCC overlaps across processors > > ACPI: CPPC: Validate SystemIO overlaps across processors > > ACPI: CPPC: Clear Performance Limited without a stale read > > ACPI: CPPC: Validate FFH register fields before hardware access > > ACPI: CPPC: Propagate errors from cross-CPU FFH calls > > ACPI: CPPC: Accept requests to retain immutable autonomous selection > > cpufreq: CPPC: Select the frequency-invariance callback per CPU > > cpufreq: CPPC: Create the FIE worker before enabling PCC callbacks > > > > Sumit Gupta (1): > > ACPI: CPPC: Keep Performance Limited clearable on NVIDIA T41 > > > > Documentation/ABI/testing/sysfs-devices-system-cpu | 16 +- > > arch/arm64/kernel/topology.c | 14 +- > > arch/x86/kernel/acpi/cppc.c | 14 + > > drivers/acpi/cppc_acpi.c | 2209 +++++++++++++++++--- > > drivers/acpi/riscv/cppc.c | 26 +- > > drivers/acpi/utils.c | 13 +- > > drivers/cpufreq/amd-pstate.c | 31 +- > > drivers/cpufreq/cppc_cpufreq.c | 73 +- > > include/acpi/cppc_acpi.h | 18 +- > > 9 files changed, 2024 insertions(+), 390 deletions(-) > > > > base-commit: fd73f4a6659897191fa0d40695fe370925dd3780 > > -- > > 2.34.1 > > > Tested the v7 on v7.3-rc3, with both _CPC rev 3 and 4. > > For the entire series: > Tested-by: Sumit Gupta <sumitg@nvidia.com> > > > Patch 20/20 had a conflict in drivers/cpufreq/cppc_cpufreq.c with > linux-next 20260916. > > The series applied cleanly to the base commit specified in the cover > letter (fd73f4a66598, tag v7.3-rc3), so patch 20/20 may need to be > rebased for the current linux-next. Thank you both! Series applied as 7.4 material, thanks! ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-18 15:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260916162805.1039247-1-christian.loehle@arm.com>
[not found] ` <20260916162805.1039247-16-christian.loehle@arm.com>
[not found] ` <9ad67536-7b6e-4633-8cc7-52b0d68196fc@arm.com>
[not found] ` <CAJZ5v0g-ipGKWyEUKrBGTo+B6RYqWxba4KhK9RaCJmh2MRLtLA@mail.gmail.com>
2026-09-17 18:20 ` [PATCH v7 15/20] ACPI: CPPC: Keep Performance Limited clearable on NVIDIA T41 Sumit Gupta
2026-09-17 18:30 ` [PATCH v7 0/20] ACPI: CPPC: Fix register access and lifetime bugs Sumit Gupta
2026-09-18 15:30 ` Rafael J. Wysocki (Intel)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox