Re: [PATCH v6 02/15] ACPI: CPPC: Validate _CPC entry and control semantics

On Thu, Sep 3, 2026 at 9:27 PM Rafael J. Wysocki (Intel)
<[email protected]> wrote:
>
> On Sun, Aug 30, 2026 at 3:32 PM Christian Loehle
> <[email protected]> wrote:
> >
> > On 8/30/26 12:56, Christian Loehle wrote:
> > > Writable _CPC controls are Register descriptors encoded as Buffer objects.
> > > Integer entries represent fixed values or unsupported optional registers;
> > > Autonomous Selection Integer 1 is the special immutable form which enables
> > > operation without Desired Performance.
> > >
> > > The parser accepts arbitrary object types and cpc_write() assumes that its
> > > argument contains a GAS. Malformed firmware can therefore make it interpret
> > > an Integer union member as a register.
> > >
> > > Validate the portion of each encoding consumed by the driver: bound Integer
> > > DWORD forms to 32 bits, and require Buffer entries to start with a complete
> > > Generic Register descriptor with the expected header. Continue tolerating
> > > Integer 0 for an absent optional register and retain type checks in
> > > cpc_write() as defense in depth. Reject an attempt to disable immutable
> > > Autonomous Selection instead of silently applying only the EPP part of the
> > > request.
> > >
> > > Capability registers are read into u64 temporaries but exposed through u32
> > > fields. Reject values above U32_MAX instead of allowing them to be
> > > truncated. In particular, a truncated Highest Performance value can become
> > > a zero divisor in the performance-to-frequency conversion. Enforce the
> > > required ordering from Highest through Nominal, Lowest Nonlinear, and
> > > Lowest Performance, and constrain a present Guaranteed Performance to the
> > > inclusive Lowest-to-Nominal range. Also reject reversed frequency anchors
> > > and unequal frequency anchors with identical performance anchors. Those
> > > invalid tuples otherwise make affine-conversion differences wrap or divide
> > > by zero.
> > >
> > > Check mandatory object presence separately from the Integer-zero convention
> > > for absent optional fields. ACPI does not reserve zero in the abstract
> > > Lowest Performance scale, so accept a present Lowest Performance DWORD of
> > > zero when distinct frequency anchors provide a usable nonzero physical
> > > minimum. Retain the old rejection when that mapping is unavailable and the
> > > fallback conversion would expose a 0 kHz cpufreq endpoint.
> > >
> > > Minimum Performance also defines zero as a real no-limit value, but the
> > > exported cppc_set_perf() interface historically used zero to omit a bound.
> > > Add explicit validity flags so callers can request zero without changing
> > > that legacy convention. Populate the flags when reading the controls and
> > > mark the bounds supplied by amd-pstate explicitly.
> > >
> > > Performance Limited is listed as a required Buffer, but the interface does
> > > not depend on it to control performance and the specification permits a
> > > platform with no limiting indication to always report zero. Preserve
> > > the compatibility with firmware that represents that case using a NULL
> > > register descriptor instead of disabling CPPC entirely.
> > >
> > > Emit an error when a present _CPC package fails parsing or initialization
> > > so such firmware and resource failures no longer silently suppress cpufreq.
> > > Initialize malformed-package failures to -EINVAL and preserve specific
> > > allocation, mapping, and unsupported-access errors in that diagnostic.
> > >
> > > Fixes: 337aadff8e45 ("ACPI: Introduce CPU performance controls using CPPC")
> > > Reported-by: Sashiko <[email protected]>
> > > Link: https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
> > > Signed-off-by: Christian Loehle <[email protected]>
> > > ---
> > >  drivers/acpi/cppc_acpi.c     | 172 ++++++++++++++++++++++++++++++-----
> > >  drivers/cpufreq/amd-pstate.c |  12 ++-
> > >  include/acpi/cppc_acpi.h     |   2 +
> > >  3 files changed, 159 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> > > index 3b8cdf88e31d..6f3ffa4a1845 100644
> > > --- a/drivers/acpi/cppc_acpi.c
> > > +++ b/drivers/acpi/cppc_acpi.c
> > > @@ -129,6 +129,21 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
> > >                               !!(cpc)->cpc_entry.int_value :          \
> > >                               !IS_NULL_REG(&(cpc)->cpc_entry.reg))
> > >
> > > +static bool cpc_is_writable(const struct cpc_register_resource *cpc)
> > > +{
> > > +     return cpc->type == ACPI_TYPE_BUFFER &&
> > > +            !IS_NULL_REG(&cpc->cpc_entry.reg);
> > > +}
> > > +
> > > +static bool cpc_entry_present(const struct cpc_register_resource *cpc)
> > > +{
> > > +     if (cpc->type == ACPI_TYPE_INTEGER)
> > > +             return true;
> > > +
> > > +     return cpc->type == ACPI_TYPE_BUFFER &&
> > > +            !IS_NULL_REG(&cpc->cpc_entry.reg);
> > > +}
> > > +
> > >  /*
> > >   * Each bit indicates the optionality of the register in per-cpu
> > >   * cpc_regs[] with the corresponding index. 0 means mandatory and 1
> > > @@ -142,6 +157,29 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
> > >   */
> > >  #define IS_OPTIONAL_CPC_REG(reg_idx) (REG_OPTIONAL & (1U << (reg_idx)))
> > >
> > > +static bool cpc_integer_entry_valid(unsigned int reg_idx, u64 value)
> > > +{
> > > +     switch (reg_idx) {
> > > +     case HIGHEST_PERF:
> > > +     case NOMINAL_PERF:
> > > +     case LOW_NON_LINEAR_PERF:
> > > +     case LOWEST_PERF:
> > > +     case CTR_WRAP_TIME:
> > > +     case REFERENCE_PERF:
> > > +     case LOWEST_FREQ:
> > > +     case NOMINAL_FREQ:
> > > +             return value <= U32_MAX;
> >
> >
> > Sashiko:
> > "Does this incorrectly restrict the counter wraparound time to 32 bits?
> > The ACPI specification allows firmware to provide a 64-bit QWord integer
> > for the Counter Wraparound Time. The cppc_perf_fb_ctrs structure already
> > models this as a 64-bit value internally
> > If firmware provides a valid 64-bit integer exceeding U32_MAX for this
> > register, cpc_integer_entry_valid() will return false and completely abort
> > CPPC initialization for the CPU. Can we remove this restriction for
> > CTR_WRAP_TIME?"
>
> > This is true. ACPI spec 6.6 and 6.5 (Table 8.23) describe it as
> > Integer (DWORD) or Buffer
> > The 64-bit internal representation is only for the case of firmware
> > providing it as Buffer.

Sashiko is right, the table in the spec is wrong.  If it is Integer,
it is 64-bit.

The size of an Integer in ASL cannot be restricted.

> >
> > > +     case AUTO_SEL_ENABLE:
> > > +             return value <= 1;
> > > +     case DESIRED_PERF:
> > > +             /* Validated against Autonomous Selection after parsing. */
> > > +             return value == 0;
> > > +     default:
> > > +             /* Tolerate the customary Integer 0 for an absent option. */
> > > +             return value == 0 && IS_OPTIONAL_CPC_REG(reg_idx);
> > > +     }
> > > +}
> > > +
> > Sashiko:
> > "Does this incorrectly restrict the counter wraparound time to 32 bits?
> > The ACPI specification allows firmware to provide a 64-bit QWord integer
> > for the Counter Wraparound Time. The cppc_perf_fb_ctrs structure already
> > models this as a 64-bit value internally.
> > If firmware provides a valid 64-bit integer exceeding U32_MAX for this
> > register, cpc_integer_entry_valid() will return false and completely abort
> > CPPC initialization for the CPU. Can we remove this restriction for
> > CTR_WRAP_TIME?"
>
> It looks like you pasted the same comment twice.  Or did Sashiko hallucinate?

Well, its other comment is actually different from the first one.  Let
me paste it:

Will this strict rejection break CPPC initialization on compliant firmware
that provides non-zero integers for other optional capabilities?
For capability registers not explicitly listed in the switch statement
above, such as GUARANTEED_PERF or TIME_WINDOW, the ACPI 6.5 specification
explicitly allows platforms to provide fixed non-zero values encoded as
Integer objects.

When firmware provides a valid non-zero integer for these optional
registers, this default case enforces that the value must be zero. Since
acpi_cppc_processor_probe() fails and returns -EINVAL when this returns
false, it will completely disable cpufreq and CPPC support for the CPU.
Should we allow non-zero integer values for these other optional registers?

> > GUARANTEED_PERF and TIME_WINDOW: both are Buffer-only Register descriptors.
> > Nonzero Integer encodings are invalid and Integer 0 is tolerated because
> > the previous parser allowed it too. I don't know of any platform describing
> > this myself.

Strictly speaking Integer 0 is not allowed, see

https://uefi.org/specs/ACPI/6.6/08_Processor_Configuration_and_Control.html#cpc-continuous-performance-control

> > >  /*
> > >   * Arbitrary Retries in case the remote processor is slow to respond
> > >   * to PCC commands. Keeping it high enough to cover emulators where
> > > @@ -150,6 +188,8 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
> > >  #define NUM_RETRIES 500ULL
> > >
> > >  #define OVER_16BTS_MASK ~0xFFFFULL
> > > +#define CPC_GENERIC_REGISTER_DESCRIPTOR 0x82
> > > +#define CPC_GENERIC_REGISTER_LENGTH (sizeof(struct cpc_reg) - 3)
> > >
> > >  #define define_one_cppc_ro(_name)            \
> > >  static struct kobj_attribute _name =         \
> > > @@ -773,8 +813,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> > >       acpi_handle handle = pr->handle;
> > >       unsigned int num_ent, i, cpc_rev;
> > >       int pcc_subspace_id = -1;
> > > +     bool cpc_present = false;
> > >       acpi_status status;
> > >       int ret = -ENODATA;
> > > +     int err;
> > >
> > >       if (!osc_sb_cppc2_support_acked) {
> > >               pr_debug("CPPC v2 _OSC not acked\n");
> > > @@ -791,6 +833,8 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> > >               ret = -ENODEV;
> > >               goto out_buf_free;
> > >       }
> > > +     cpc_present = true;
> > > +     ret = -EINVAL;
> > >
> > >       out_obj = (union acpi_object *) output.pointer;
> > >       if (out_obj->package.count < 2) {
> > > @@ -871,11 +915,32 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> > >               cpc_obj = &out_obj->package.elements[i];
> > >
> > >               if (cpc_obj->type == ACPI_TYPE_INTEGER) {
> > > -                     cpc_ptr->cpc_regs[i-2].type = ACPI_TYPE_INTEGER;
> > > -                     cpc_ptr->cpc_regs[i-2].cpc_entry.int_value = cpc_obj->integer.value;
> > > +                     if (!cpc_integer_entry_valid(i - 2,
> > > +                                                  cpc_obj->integer.value)) {
> > > +                             pr_debug("Invalid Integer _CPC register %u for CPU:%d\n",
> > > +                                      i - 2, pr->id);
> > > +                             ret = -EINVAL;
> > > +                             goto out_free;
> > > +                     }
> > > +                     cpc_ptr->cpc_regs[i - 2].type = ACPI_TYPE_INTEGER;
> > > +                     cpc_ptr->cpc_regs[i - 2].cpc_entry.int_value = cpc_obj->integer.value;
> > >               } else if (cpc_obj->type == ACPI_TYPE_BUFFER) {
> > > +                     if (cpc_obj->buffer.length < sizeof(*gas_t)) {
> > > +                             pr_debug("Invalid register descriptor for CPU:%d\n",
> > > +                                      pr->id);
> > > +                             ret = -EINVAL;
> > > +                             goto out_free;
> > > +                     }
> > > +
> > >                       gas_t = (struct cpc_reg *)
> > >                               cpc_obj->buffer.pointer;
> > > +                     if (gas_t->descriptor != CPC_GENERIC_REGISTER_DESCRIPTOR ||
> > > +                         gas_t->length != CPC_GENERIC_REGISTER_LENGTH) {
> > > +                             pr_debug("Invalid register resource for CPU:%d\n",
> > > +                                      pr->id);
> > > +                             ret = -EINVAL;
> > > +                             goto out_free;
> > > +                     }
> > >
> > >                       /*
> > >                        * The PCC Subspace index is encoded inside
> > > @@ -886,8 +951,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> > >                       if (gas_t->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
> > >                               if (pcc_subspace_id < 0) {
> > >                                       pcc_subspace_id = gas_t->access_width;
> > > -                                     if (pcc_data_alloc(pcc_subspace_id))
> > > +                                     err = pcc_data_alloc(pcc_subspace_id);
> > > +                                     if (err) {
> > > +                                             ret = err;
> > >                                               goto out_free;
> > > +                                     }
> > >                               } else if (pcc_subspace_id != gas_t->access_width) {
> > >                                       pr_debug("Mismatched PCC ids in _CPC for CPU:%d\n",
> > >                                                pr->id);
> > > @@ -900,14 +968,18 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> > >
> > >                                       if (!osc_cpc_flexible_adr_space_confirmed) {
> > >                                               pr_debug("Flexible address space capability not supported\n");
> > > +                                             ret = -EOPNOTSUPP;
> > >                                               if (!cpc_supported_by_cpu())
> > >                                                       goto out_free;
> > > +                                             ret = -EINVAL;
> > >                                       }
> > >
> > >                                       access_width = GET_BIT_WIDTH(gas_t) / 8;
> > >                                       addr = ioremap(gas_t->address, access_width);
> > > -                                     if (!addr)
> > > +                                     if (!addr) {
> > > +                                             ret = -ENOMEM;
> > >                                               goto out_free;
> > > +                                     }
> > >                                       cpc_ptr->cpc_regs[i-2].sys_mem_vaddr = addr;
> > >                               }
> > >                       } else if (gas_t->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> > > @@ -929,14 +1001,17 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> > >                               }
> > >                               if (!osc_cpc_flexible_adr_space_confirmed) {
> > >                                       pr_debug("Flexible address space capability not supported\n");
> > > +                                     ret = -EOPNOTSUPP;
> > >                                       if (!cpc_supported_by_cpu())
> > >                                               goto out_free;
> > > +                                     ret = -EINVAL;
> > >                               }
> > >                       } else {
> > >                               if (gas_t->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE || !cpc_ffh_supported()) {
> > >                                       /* Support only PCC, SystemMemory, SystemIO, and FFH type regs. */
> > >                                       pr_debug("Unsupported register type (%d) in _CPC\n",
> > >                                                gas_t->space_id);
> > > +                                     ret = -EOPNOTSUPP;
> > >                                       goto out_free;
> > >                               }
> > >                       }
> > > @@ -961,15 +1036,35 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> > >       }
> > >       per_cpu(cpu_pcc_subspace_idx, pr->id) = pcc_subspace_id;
> > >
> > > +     /*
> > > +      * Performance Limited is required by the specification, but tolerate a
> > > +      * NULL descriptor used by firmware which cannot report limiting events.
> > > +      * CPPC control does not depend on this status.
> > > +      */
> > > +     for (i = 0; i < num_ent - 2; i++) {
> > > +             if (i != DESIRED_PERF && i != PERF_LIMITED &&
> > > +                 !IS_OPTIONAL_CPC_REG(i) &&
> > > +                 !cpc_entry_present(&cpc_ptr->cpc_regs[i])) {
> > > +                     pr_debug("CPU:%d lacks mandatory _CPC register %u\n",
> > > +                              pr->id, i);
> > > +                     ret = -EINVAL;
> > > +                     goto out_free;
> > > +             }
> > > +     }
> > > +
> > >       /*
> > >        * In CPPC v1, DESIRED_PERF is mandatory. In CPPC v2, it is optional
> > >        * only when AUTO_SEL_ENABLE is supported.
> > >        */
> > > -     if (!CPC_SUPPORTED(&cpc_ptr->cpc_regs[DESIRED_PERF]) &&
> > > +     if (!cpc_is_writable(&cpc_ptr->cpc_regs[DESIRED_PERF]) &&
> > >           (!osc_sb_cppc2_support_acked ||
> > > -          !CPC_SUPPORTED(&cpc_ptr->cpc_regs[AUTO_SEL_ENABLE])))
> > > -             pr_warn("Desired perf. register is mandatory if CPPC v2 is not supported "
> > > -                     "or autonomous selection is disabled\n");
> > > +          cpc_ptr->cpc_regs[AUTO_SEL_ENABLE].type != ACPI_TYPE_INTEGER ||
> > > +          cpc_ptr->cpc_regs[AUTO_SEL_ENABLE].cpc_entry.int_value != 1)) {
> > > +             pr_debug("CPU:%d lacks a writable Desired Performance register\n",
> > > +                      pr->id);
> > > +             ret = -EINVAL;
> > > +             goto out_free;
> > > +     }
> > >
> > >       /*
> > >        * Initialize the remaining cpc_regs as unsupported.
> > > @@ -1037,6 +1132,8 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> > >       kfree(cpc_ptr);
> > >
> > >  out_buf_free:
> > > +     if (cpc_present)
> > > +             pr_err("CPU%d: failed to initialize _CPC: %d\n", pr->id, ret);
> > >       kfree(output.pointer);
> > >       return ret;
> > >  }
> > > @@ -1217,11 +1314,18 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
> > >       u64 prev_val;
> > >       void __iomem *vaddr = NULL;
> > >       int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
> > > -     struct cpc_reg *reg = &reg_res->cpc_entry.reg;
> > > +     struct cpc_reg *reg;
> > >       struct cpc_desc *cpc_desc;
> > >       unsigned long flags;
> > >       bool locked = false;
> > >
> > > +     if (reg_res->type != ACPI_TYPE_BUFFER)
> > > +             return -EOPNOTSUPP;
> > > +
> > > +     reg = &reg_res->cpc_entry.reg;
> > > +     if (IS_NULL_REG(reg))
> > > +             return -EOPNOTSUPP;
> > > +
> > >       size = GET_BIT_WIDTH(reg);
> > >
> > >       if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
> > > @@ -1364,7 +1468,9 @@ static int cppc_get_reg_val(int cpu, enum cppc_regs reg_idx, u64 *val)
> > >
> > >       reg = &cpc_desc->cpc_regs[reg_idx];
> > >
> > > -     if ((reg->type == ACPI_TYPE_INTEGER && IS_OPTIONAL_CPC_REG(reg_idx) &&
> > > +     /* Desired may be absent for immutable autonomous selection. */
> > > +     if ((reg->type == ACPI_TYPE_INTEGER &&
> > > +          (IS_OPTIONAL_CPC_REG(reg_idx) || reg_idx == DESIRED_PERF) &&
> > >            !reg->cpc_entry.int_value) || (reg->type != ACPI_TYPE_INTEGER &&
> > >            IS_NULL_REG(&reg->cpc_entry.reg))) {
> > >               pr_debug("CPC register is not supported\n");
> > > @@ -1415,7 +1521,7 @@ static int cppc_set_reg_val(int cpu, enum cppc_regs reg_idx, u64 val)
> > >       reg = &cpc_desc->cpc_regs[reg_idx];
> > >
> > >       /* if a register is writeable, it must be a buffer and not null */
> > > -     if ((reg->type != ACPI_TYPE_BUFFER) || IS_NULL_REG(&reg->cpc_entry.reg)) {
> > > +     if (!cpc_is_writable(reg)) {
> > >               pr_debug("CPC register is not supported\n");
> > >               return -EOPNOTSUPP;
> > >       }
> > > @@ -1505,7 +1611,7 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
> > >       struct cpc_register_resource *highest_reg, *lowest_reg,
> > >               *lowest_non_linear_reg, *nominal_reg, *reference_reg,
> > >               *guaranteed_reg, *low_freq_reg = NULL, *nom_freq_reg = NULL;
> > > -     u64 high, low, guaranteed, nom, ref, min_nonlinear,
> > > +     u64 high, low, guaranteed = 0, nom, ref, min_nonlinear,
> > >           low_f = 0, nom_f = 0;
> > >       int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
> > >       struct cppc_pcc_data *pcc_ss_data = NULL;
> > > @@ -1588,7 +1694,12 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
> > >               goto out_err;
> > >       perf_caps->lowest_nonlinear_perf = min_nonlinear;
> > >
> > > -     if (!high || !low || !nom || !ref || !min_nonlinear) {
> > > +     if (!high || !nom || !ref || !min_nonlinear ||
> > > +         high > U32_MAX || low > U32_MAX || guaranteed > U32_MAX ||
> > > +         nom > U32_MAX || ref > U32_MAX || min_nonlinear > U32_MAX ||
> > > +         high < nom || nom < min_nonlinear || min_nonlinear < low ||
> > > +         (CPC_SUPPORTED(guaranteed_reg) &&
> > > +          (guaranteed < low || guaranteed > nom))) {
> > >               ret = -EFAULT;
> > >               goto out_err;
> > >       }
> > > @@ -1605,6 +1716,19 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
> > >               if (ret)
> > >                       goto out_err;
> > >       }
> > > +     /*
> > > +      * Require ordered anchors and a nonzero slope when frequencies differ.
> > > +      * A zero Lowest Performance needs that affine mapping to produce a
> > > +      * nonzero physical minimum frequency.
> > > +      */
> > > +     if (low_f > U32_MAX || nom_f > U32_MAX ||
> > > +         (!low && (!low_f || !nom_f || low_f == nom_f)) ||
> > > +         (low_f && nom_f &&
> > > +          (nom_f < low_f || nom < low ||
> > > +           (nom_f != low_f && nom == low)))) {
> > > +             ret = -EFAULT;
> > > +             goto out_err;
> > > +     }
> > >
> > >       perf_caps->lowest_freq = low_f;
> > >       perf_caps->nominal_freq = nom_f;
> > > @@ -1779,6 +1903,9 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
> > >
> > >       auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
> > >       epp_set_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
> > > +     if (!enable && auto_sel_reg->type == ACPI_TYPE_INTEGER &&
> > > +         auto_sel_reg->cpc_entry.int_value == 1)
> > > +             return -EOPNOTSUPP;
> > >
> > >       epp_ffh_sysmem = CPC_SUPPORTED(epp_set_reg) &&
> > >               (CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
> > > @@ -1791,13 +1918,13 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
> > >                       return -ENODEV;
> > >               }
> > >
> > > -             if (CPC_SUPPORTED(auto_sel_reg)) {
> > > +             if (cpc_is_writable(auto_sel_reg)) {
> > >                       ret = cpc_write(cpu, auto_sel_reg, enable);
> > >                       if (ret)
> > >                               return ret;
> > >               }
> > >
> > > -             if (CPC_SUPPORTED(epp_set_reg)) {
> > > +             if (cpc_is_writable(epp_set_reg)) {
> > >                       ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
> > >                       if (ret)
> > >                               return ret;
> > > @@ -1996,6 +2123,8 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
> > >       max_perf_reg = &cpc_desc->cpc_regs[MAX_PERF];
> > >       energy_perf_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
> > >       auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
> > > +     perf_ctrls->max_perf_valid = false;
> > > +     perf_ctrls->min_perf_valid = false;
> > >
> > >       /* Are any of the regs PCC ?*/
> > >       if (CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg) ||
> > > @@ -2020,6 +2149,7 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
> > >               ret = cpc_read(cpu, max_perf_reg, &max);
> > >               if (ret)
> > >                       goto out_err;
> > > +             perf_ctrls->max_perf_valid = true;
> > >       }
> > >       perf_ctrls->max_perf = max;
> > >
> > > @@ -2027,6 +2157,7 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
> > >               ret = cpc_read(cpu, min_perf_reg, &min);
> > >               if (ret)
> > >                       goto out_err;
> > > +             perf_ctrls->min_perf_valid = true;
> > >       }
> > >       perf_ctrls->min_perf = min;
> > >
> > > @@ -2113,14 +2244,11 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
> > >       if (CPC_SUPPORTED(desired_reg))
> > >               cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
> > >
> > > -     /*
> > > -      * Only write if min_perf and max_perf not zero. Some drivers pass zero
> > > -      * value to min and max perf, but they don't mean to set the zero value,
> > > -      * they just don't want to write to those registers.
> > > -      */
> > > -     if (perf_ctrls->min_perf && CPC_SUPPORTED(min_perf_reg))
> > > +     if (CPC_SUPPORTED(min_perf_reg) &&
> > > +         (perf_ctrls->min_perf || perf_ctrls->min_perf_valid))
> > >               cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
> > > -     if (perf_ctrls->max_perf && CPC_SUPPORTED(max_perf_reg))
> > > +     if (CPC_SUPPORTED(max_perf_reg) &&
> > > +         (perf_ctrls->max_perf || perf_ctrls->max_perf_valid))
> > >               cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
> > >
> > >       if (regs_in_pcc)
> > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > > index d4ff8b228f86..63f0ca5f19b3 100644
> > > --- a/drivers/cpufreq/amd-pstate.c
> > > +++ b/drivers/cpufreq/amd-pstate.c
> > > @@ -544,7 +544,13 @@ static int shmem_update_perf(struct cpufreq_policy *policy, u8 min_perf,
> > >                            u8 des_perf, u8 max_perf, u8 epp, bool fast_switch)
> > >  {
> > >       struct amd_cpudata *cpudata = policy->driver_data;
> > > -     struct cppc_perf_ctrls perf_ctrls;
> > > +     struct cppc_perf_ctrls perf_ctrls = {
> > > +             .max_perf = max_perf,
> > > +             .min_perf = min_perf,
> > > +             .desired_perf = des_perf,
> > > +             .max_perf_valid = true,
> > > +             .min_perf_valid = true,
> > > +     };
> > >       u64 value, prev;
> > >       int ret;
> > >
> > > @@ -577,10 +583,6 @@ static int shmem_update_perf(struct cpufreq_policy *policy, u8 min_perf,
> > >       if (value == prev)
> > >               return 0;
> > >
> > > -     perf_ctrls.max_perf = max_perf;
> > > -     perf_ctrls.min_perf = min_perf;
> > > -     perf_ctrls.desired_perf = des_perf;
> > > -
> > >       ret = cppc_set_perf(cpudata->cpu, &perf_ctrls);
> > >       if (ret)
> > >               return ret;
> > > diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> > > index 94a6277edab2..5dcbe65c5ddc 100644
> > > --- a/include/acpi/cppc_acpi.h
> > > +++ b/include/acpi/cppc_acpi.h
> > > @@ -141,6 +141,8 @@ struct cppc_perf_ctrls {
> > >       u32 desired_perf;
> > >       u32 energy_perf;
> > >       bool auto_sel;
> > > +     bool max_perf_valid;
> > > +     bool min_perf_valid;
> > >  };
> > >
> > >  struct cppc_perf_fb_ctrs {
> >