Re: [PATCH 1/8] ACPI: CPPC: Prepare cpc_register_resource for Package-type entries

[Rafael J. Wysocki (Intel)]

On Fri, Jul 17, 2026 at 4:46 AM Lifeng Zheng <zhenglifeng1@xxxxxxxxxx> wrote:
>
> CPPC v4 (ACPI 6.6, Section 8.4.6.1.2.7) introduces the Resource
> Priority entry, which is a Package of sub-packages rather than a
> plain Integer or Buffer register.  The existing cpc_register_resource
> union only accommodates Integer and Buffer (register descriptor)
> fields.
>
> Add a Package variant to the cpc_entry union so that nested
> structures such as RESOURCE_PRIORITY can store their element count
> and a dynamically allocated array of child cpc_register_resource
> descriptors.
>
> Update the CPC_SUPPORTED() macro to recognise Package-type entries
> with a non-zero element count, and switch the unsupported-Package
> fallback from a zero Integer to an empty Package (count = 0,
> elements = NULL) so that CPC_SUPPORTED() correctly reports such
> entries as unsupported.
>
> No functional change intended; the new Package fields are not yet
> consumed by any caller.
>
> Signed-off-by: Lifeng Zheng <zhenglifeng1@xxxxxxxxxx>
> ---
>  drivers/acpi/cppc_acpi.c | 19 +++++++++++++------
>  include/acpi/cppc_acpi.h |  8 ++++++++
>  2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 9f572f481241..d7e654f2a66c 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -124,10 +124,16 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
>                                 (reg)->bit_offset == 0 &&               \
>                                 (reg)->access_width == 0)
>
> -/* Evaluates to True if an optional cpc field is supported */
> -#define CPC_SUPPORTED(cpc) ((cpc)->type == ACPI_TYPE_INTEGER ?         \
> -                               !!(cpc)->cpc_entry.int_value :          \
> -                               !IS_NULL_REG(&(cpc)->cpc_entry.reg))
> +/*
> + * Evaluates to True if an optional cpc field is supported.
> + * Integer: non-zero value; Buffer: non-NULL register; Package: non-empty.
> + */
> +#define CPC_SUPPORTED(cpc) (((cpc)->type == ACPI_TYPE_INTEGER &&       \
> +                            !!(cpc)->cpc_entry.int_value) ||           \
> +                           ((cpc)->type == ACPI_TYPE_BUFFER &&         \
> +                            !IS_NULL_REG(&(cpc)->cpc_entry.reg)) ||    \
> +                           ((cpc)->type == ACPI_TYPE_PACKAGE &&        \
> +                            (cpc)->cpc_entry.package.count != 0))

My immediate concern regarding this change in particular is that it
potentially causes invalid data structures to be used as "supported".

Suppose that some defective platform firmware supplies a package in a
field that by the spec can only be a buffer and the length of that
package is greater than zero.  The macro will produce 'true' then
which is invalid.

It looks like there should be three different macros like the above
for the three different types of _CPC fields: the buffer/integer ones,
the buffer-only ones and the single package field.

And I'm not sure how much of a value there is in complicating the
overall parsing of all fields just in order to be able to handle the
last special one in a "generic" fashion.

As this stands now, it is a "no" from me, sorry.

>
>  /*
>   * Each bit indicates the optionality of the register in per-cpu
> @@ -859,8 +865,9 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
>                          * Mark the register as unsupported for now.
>                          */
>                         pr_debug("CPU:%d Resource Priority not supported\n", pr->id);
> -                       cpc_ptr->cpc_regs[i-2].type = ACPI_TYPE_INTEGER;
> -                       cpc_ptr->cpc_regs[i-2].cpc_entry.int_value = 0;
> +                       cpc_ptr->cpc_regs[i-2].type = ACPI_TYPE_PACKAGE;
> +                       cpc_ptr->cpc_regs[i-2].cpc_entry.package.count = 0;
> +                       cpc_ptr->cpc_regs[i-2].cpc_entry.package.elements = NULL;
>                 } else {
>                         pr_debug("Invalid entry type (%d) in _CPC for CPU:%d\n",
>                                  i, pr->id);
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 8693890a7275..1839582b80be 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -71,6 +71,14 @@ struct cpc_register_resource {
>         union {
>                 struct cpc_reg reg;
>                 u64 int_value;
> +               /*
> +                * CPPC v4: nested Package (e.g. RESOURCE_PRIORITY),
> +                * elements dynamically allocated
> +                */
> +               struct {
> +                       u32 count;
> +                       struct cpc_register_resource *elements;
> +               } package;
>         } cpc_entry;
>  };
>
> --
> 2.33.0
>