[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4227f19d-f065-4563-b48b-538aafe52ed9@amd.com>
Date: Thu, 5 Dec 2024 15:49:37 -0500
From: Jason Andryuk <jason.andryuk@....com>
To: Penny Zheng <Penny.Zheng@....com>, Juergen Gross <jgross@...e.com>,
Stefano Stabellini <sstabellini@...nel.org>, Oleksandr Tyshchenko
<oleksandr_tyshchenko@...m.com>, "Rafael J. Wysocki" <rafael@...nel.org>,
"Len Brown" <lenb@...nel.org>
CC: Ray Huang <Ray.Huang@....com>, Xenia Ragiadakou
<Xenia.Ragiadakou@....com>, <xen-devel@...ts.xenproject.org>,
<linux-kernel@...r.kernel.org>, <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v2 2/4] acpi/cppc: extract _cpc entry parsing logic
Adding Rafael and Len, since the patch touches drivers/acpi/.
On 2024-12-05 00:42, Penny Zheng wrote:
> When running as Xen dom0 PVH guest, MADT table is customized
> and may have the "wrong" UID processor number, which is
> inconsistent with the UID in Processor entry in native DSDT.
>
> As a result, during ACPI boot-up for dom0, linux fails to set
> up proper processor logical id <-> physical id map(acpi_map_cpuid).
> Furthermore, It leads to that some ACPI processor feature data,
> like per-cpu cpc_desc structure, failed to be correctly stored.
>
> In order to re-parse _CPC entry later for delivering correct data
> in performance hypercall, firstly, we extract parsing logic from
> acpi_cppc_processor_probe() and export it as a new function
> acpi_cppc_processor_parse().
>
> Also, replace logical processor id with ACPI ID, to show correct print
> info in Xen dom0 PVH guest.
>
> Signed-off-by: Penny Zheng <Penny.Zheng@....com>
Reviewed-by: Jason Andryuk <jason.andryuk@....com>
No further comments by me - leaving untrimmed for Rafael and Len.
Regards,
Jason
> ---
> drivers/acpi/cppc_acpi.c | 93 +++++++++++++++++++++++-----------------
> 1 file changed, 53 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index c3fc2c05d868..13d6ff84a1e9 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -675,19 +675,11 @@ static int pcc_data_alloc(int pcc_ss_id)
> static inline void arch_init_invariance_cppc(void) { }
> #endif
>
> -/**
> - * acpi_cppc_processor_probe - Search for per CPU _CPC objects.
> - * @pr: Ptr to acpi_processor containing this CPU's logical ID.
> - *
> - * Return: 0 for success or negative value for err.
> - */
> -int acpi_cppc_processor_probe(struct acpi_processor *pr)
> +static int acpi_cppc_processor_parse(struct acpi_processor *pr, struct cpc_desc *cpc_ptr)
> {
> struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
> union acpi_object *out_obj, *cpc_obj;
> - struct cpc_desc *cpc_ptr;
> struct cpc_reg *gas_t;
> - struct device *cpu_dev;
> acpi_handle handle = pr->handle;
> unsigned int num_ent, i, cpc_rev;
> int pcc_subspace_id = -1;
> @@ -706,31 +698,24 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> status = acpi_evaluate_object_typed(handle, "_CPC", NULL, &output,
> ACPI_TYPE_PACKAGE);
> if (ACPI_FAILURE(status)) {
> - ret = -ENODEV;
> - goto out_buf_free;
> + return -ENODEV;
> }
>
> out_obj = (union acpi_object *) output.pointer;
>
> - cpc_ptr = kzalloc(sizeof(struct cpc_desc), GFP_KERNEL);
> - if (!cpc_ptr) {
> - ret = -ENOMEM;
> - goto out_buf_free;
> - }
> -
> /* First entry is NumEntries. */
> cpc_obj = &out_obj->package.elements[0];
> if (cpc_obj->type == ACPI_TYPE_INTEGER) {
> num_ent = cpc_obj->integer.value;
> if (num_ent <= 1) {
> pr_debug("Unexpected _CPC NumEntries value (%d) for CPU:%d\n",
> - num_ent, pr->id);
> - goto out_free;
> + num_ent, pr->acpi_id);
> + goto out_pointer;
> }
> } else {
> pr_debug("Unexpected _CPC NumEntries entry type (%d) for CPU:%d\n",
> - cpc_obj->type, pr->id);
> - goto out_free;
> + cpc_obj->type, pr->acpi_id);
> + goto out_pointer;
> }
>
> /* Second entry should be revision. */
> @@ -739,14 +724,14 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> cpc_rev = cpc_obj->integer.value;
> } else {
> pr_debug("Unexpected _CPC Revision entry type (%d) for CPU:%d\n",
> - cpc_obj->type, pr->id);
> - goto out_free;
> + cpc_obj->type, pr->acpi_id);
> + goto out_pointer;
> }
>
> if (cpc_rev < CPPC_V2_REV) {
> pr_debug("Unsupported _CPC Revision (%d) for CPU:%d\n", cpc_rev,
> - pr->id);
> - goto out_free;
> + pr->acpi_id);
> + goto out_pointer;
> }
>
> /*
> @@ -758,8 +743,8 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> (cpc_rev == CPPC_V3_REV && num_ent != CPPC_V3_NUM_ENT) ||
> (cpc_rev > CPPC_V3_REV && num_ent <= CPPC_V3_NUM_ENT)) {
> pr_debug("Unexpected number of _CPC return package entries (%d) for CPU:%d\n",
> - num_ent, pr->id);
> - goto out_free;
> + num_ent, pr->acpi_id);
> + goto out_pointer;
> }
> if (cpc_rev > CPPC_V3_REV) {
> num_ent = CPPC_V3_NUM_ENT;
> @@ -793,7 +778,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> 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);
> + pr->acpi_id);
> goto out_free;
> }
> } else if (gas_t->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> @@ -848,7 +833,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> memcpy(&cpc_ptr->cpc_regs[i-2].cpc_entry.reg, gas_t, sizeof(*gas_t));
> } else {
> pr_debug("Invalid entry type (%d) in _CPC for CPU:%d\n",
> - i, pr->id);
> + i, pr->acpi_id);
> goto out_free;
> }
> }
> @@ -864,6 +849,45 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
> cpc_ptr->cpc_regs[i].cpc_entry.int_value = 0;
> }
>
> + pr_debug("Parsed _CPC entry for CPU: %d\n", pr->acpi_id);
> + kfree(output.pointer);
> + return 0;
> +
> + out_free:
> + /* Free all the mapped sys mem areas for this CPU */
> + for (i = 2; i < cpc_ptr->num_entries; i++) {
> + void __iomem *addr = cpc_ptr->cpc_regs[i-2].sys_mem_vaddr;
> +
> + if (addr)
> + iounmap(addr);
> + }
> + out_pointer:
> + kfree(output.pointer);
> + return ret;
> +}
> +
> +/**
> + * acpi_cppc_processor_probe - Search for per CPU _CPC objects.
> + * @pr: Ptr to acpi_processor containing this CPU's logical ID.
> + *
> + * Return: 0 for success or negative value for err.
> + */
> +int acpi_cppc_processor_probe(struct acpi_processor *pr)
> +{
> + acpi_handle handle = pr->handle;
> + struct cpc_desc *cpc_ptr;
> + struct device *cpu_dev;
> + int pcc_subspace_id = -1;
> + int ret = -ENODATA;
> +
> + cpc_ptr = kzalloc(sizeof(struct cpc_desc), GFP_KERNEL);
> + if (!cpc_ptr)
> + return -ENOMEM;
> +
> + ret = acpi_cppc_processor_parse(pr, cpc_ptr);
> + if (ret)
> + goto out_free;
> + pcc_subspace_id = per_cpu(cpu_pcc_subspace_idx, pr->id);
>
> /* Store CPU Logical ID */
> cpc_ptr->cpu_id = pr->id;
> @@ -907,21 +931,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
>
> arch_init_invariance_cppc();
>
> - kfree(output.pointer);
> return 0;
>
> out_free:
> - /* Free all the mapped sys mem areas for this CPU */
> - for (i = 2; i < cpc_ptr->num_entries; i++) {
> - void __iomem *addr = cpc_ptr->cpc_regs[i-2].sys_mem_vaddr;
> -
> - if (addr)
> - iounmap(addr);
> - }
> kfree(cpc_ptr);
> -
> -out_buf_free:
> - kfree(output.pointer);
> return ret;
> }
> EXPORT_SYMBOL_GPL(acpi_cppc_processor_probe);
Powered by blists - more mailing lists