| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b9d5636c-0485-4a27-bf17-fd092b0b8ef9@amd.com>
Date: Thu, 5 Dec 2024 17:08:56 -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 4/4] xen/cppc: introduce cppc data upload sub-hypercall
Adding Rafael and Len. and again keeping the full patch.
On 2024-12-05 00:42, Penny Zheng wrote:
> As Xen is uncapable of parsing the ACPI dynamic table, this commit
s/uncapable/incapable/
> introduces a new sub-hypercall XEN_PM_CPPC to deliver CPPC perf
> caps data.
>
> Signed-off-by: Penny Zheng <Penny.Zheng@....com>
> ---
> drivers/acpi/cppc_acpi.c | 1 +
> drivers/xen/xen-acpi-processor.c | 89 +++++++++++++++++++++++++++++++-
> include/acpi/processor.h | 1 +
> include/xen/interface/platform.h | 11 ++++
> 4 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 3a436591da07..3570a52a5dbd 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -860,6 +860,7 @@ static int acpi_cppc_processor_parse(struct acpi_processor *pr, struct cpc_desc
> cpc_ptr->cpc_regs[i].cpc_entry.int_value = 0;
> }
>
> + pr->flags.has_cpc = 1;
> pr_debug("Parsed _CPC entry for CPU: %d\n", pr->acpi_id);
> kfree(output.pointer);
> return 0;
> diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c
> index e9f38f171240..8a39e46c1ebc 100644
> --- a/drivers/xen/xen-acpi-processor.c
> +++ b/drivers/xen/xen-acpi-processor.c
> @@ -25,6 +25,7 @@
> #include <xen/xen.h>
> #include <xen/interface/platform.h>
> #include <asm/xen/hypercall.h>
> +#include <acpi/cppc_acpi.h>
>
> static int no_hypercall;
> MODULE_PARM_DESC(off, "Inhibit the hypercall.");
> @@ -45,8 +46,12 @@ static unsigned long *acpi_ids_done;
> static unsigned long *acpi_id_present;
> /* And if there is an _CST definition (or a PBLK) for the ACPI IDs */
> static unsigned long *acpi_id_cst_present;
> +/* And if there is an _CPC entry for the ACPI IDs */
> +static unsigned long *acpi_id_cpc_present;
> /* Which ACPI P-State dependencies for a enumerated processor */
> static struct acpi_psd_package *acpi_psd;
> +/* ACPI CPPC structures for a enumerated processor */
> +static struct cppc_perf_caps *acpi_cppc_data;
>
> static bool pr_initialized;
>
> @@ -208,6 +213,44 @@ static int xen_copy_pct_data(struct acpi_pct_register *pct,
> dst_pct->address = pct->address;
> return 0;
> }
> +static int push_cppc_to_hypervisor(struct acpi_processor *_pr)
> +{
> + int ret = 0;
> + struct xen_platform_op op = {
> + .cmd = XENPF_set_processor_pminfo,
> + .interface_version = XENPF_INTERFACE_VERSION,
> + .u.set_pminfo.id = _pr->acpi_id,
> + .u.set_pminfo.type = XEN_PM_CPPC,
> + };
> + struct cppc_perf_caps *cppc_perf = acpi_cppc_data + _pr->acpi_id;
> +
> + op.u.set_pminfo.cppc_data.highest_perf = cppc_perf->highest_perf;
> + op.u.set_pminfo.cppc_data.lowest_perf = cppc_perf->lowest_perf;
> + op.u.set_pminfo.cppc_data.nominal_perf = cppc_perf->nominal_perf;
> + op.u.set_pminfo.cppc_data.lowest_nonlinear_perf = cppc_perf->lowest_nonlinear_perf;
> + op.u.set_pminfo.cppc_data.lowest_freq = cppc_perf->lowest_freq;
> + op.u.set_pminfo.cppc_data.nominal_freq = cppc_perf->nominal_freq;
> +
> + if (!no_hypercall)
> + ret = HYPERVISOR_platform_op(&op);
> +
> + if (!ret) {
> + pr_debug("ACPI CPU%u - CPPC uploaded.\n", _pr->acpi_id);
> + pr_debug(" highest_perf: %d\n", cppc_perf->highest_perf);
> + pr_debug(" lowest_perf: %d\n", cppc_perf->lowest_perf);
> + pr_debug(" lowest_nonlinear_perf: %d\n", cppc_perf->lowest_nonlinear_perf);
> + pr_debug(" nominal_perf: %d\n", cppc_perf->nominal_perf);
> + pr_debug(" lowest_freq: %d Mhz\n", cppc_perf->lowest_freq);
> + pr_debug(" nominal_freq: %d Mhz\n", cppc_perf->nominal_freq);
> + } else if ((ret != -EINVAL) && (ret != -ENOSYS))
> + /* EINVAL means the ACPI ID is incorrect - meaning the ACPI
> + * table is referencing a non-existing CPU - which can happen
> + * with broken ACPI tables. */
> + pr_warn("(_CPC): Hypervisor error (%d) for ACPI CPU%u\n",
> + ret, _pr->acpi_id);
> +
> + return ret;
> +}
> static int push_pxx_to_hypervisor(struct acpi_processor *_pr)
> {
> int ret = 0;
> @@ -284,6 +327,9 @@ static int upload_pm_data(struct acpi_processor *_pr)
> if (_pr->flags.power)
> err = push_cxx_to_hypervisor(_pr);
>
> + if (_pr->flags.has_cpc)
> + err |= push_cppc_to_hypervisor(_pr);
> +
Older hypervisors without the xen changes will return an error since
they don't implement XEN_PM_CPPC. We might want to ignore the return
when it's EINVAL, the default for the XEN_PM_* switch, but that may
ignore more than intended.
The two calls to upload_pm_data() ignore the return value, so maybe it
doesn't matter.
(This is the end of my comments.)
Thanks,
Jason
> if (_pr->performance && _pr->performance->states)
> err |= push_pxx_to_hypervisor(_pr);
>
> @@ -488,6 +534,7 @@ read_acpi_id(acpi_handle handle, u32 lvl, void *context, void **rv)
> union acpi_object object = { 0 };
> struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
> struct acpi_buffer cst_buf = { ACPI_ALLOCATE_BUFFER, NULL };
> + struct acpi_buffer cpc_buf = { ACPI_ALLOCATE_BUFFER, NULL };
> acpi_io_address pblk = 0;
>
> status = acpi_get_type(handle, &acpi_type);
> @@ -567,11 +614,20 @@ read_acpi_id(acpi_handle handle, u32 lvl, void *context, void **rv)
> /* .. and it has a C-state */
> __set_bit(acpi_id, acpi_id_cst_present);
>
> + status = acpi_evaluate_object(handle, "_CPC", NULL, &cpc_buf);
> + if (ACPI_FAILURE(status)) {
> + return AE_OK;
> + }
> + kfree(cpc_buf.pointer);
> +
> + /* .. and it has a _CPC entry */
> + __set_bit(acpi_id, acpi_id_cpc_present);
> +
> return AE_OK;
> }
> static int check_acpi_ids(struct acpi_processor *pr_backup)
> {
> - if (acpi_id_present && acpi_id_cst_present)
> + if (acpi_id_present && acpi_id_cst_present && acpi_id_cpc_present)
> /* OK, done this once .. skip to uploading */
> goto upload;
>
> @@ -588,11 +644,19 @@ static int check_acpi_ids(struct acpi_processor *pr_backup)
> return -ENOMEM;
> }
>
> + acpi_id_cpc_present = bitmap_zalloc(nr_acpi_bits, GFP_KERNEL);
> + if (!acpi_id_cpc_present) {
> + bitmap_free(acpi_id_present);
> + bitmap_free(acpi_id_cst_present);
> + return -ENOMEM;
> + }
> +
> acpi_psd = kcalloc(nr_acpi_bits, sizeof(struct acpi_psd_package),
> GFP_KERNEL);
> if (!acpi_psd) {
> bitmap_free(acpi_id_present);
> bitmap_free(acpi_id_cst_present);
> + bitmap_free(acpi_id_cpc_present);
> return -ENOMEM;
> }
>
> @@ -608,6 +672,12 @@ static int check_acpi_ids(struct acpi_processor *pr_backup)
> pr_backup->acpi_id = i;
> /* Mask out C-states if there are no _CST or PBLK */
> pr_backup->flags.power = test_bit(i, acpi_id_cst_present);
> + /* Mask out relevant flag if there are no _CPC */
> + pr_backup->flags.has_cpc = test_bit(i, acpi_id_cpc_present);
> + if (pr_backup->flags.has_cpc) {
> + if (xen_processor_get_perf_caps(pr_backup, acpi_cppc_data + i))
> + return -EINVAL;
> + }
> /* num_entries is non-zero if we evaluated _PSD */
> if (acpi_psd[i].num_entries) {
> memcpy(&pr_backup->performance->domain_info,
> @@ -726,6 +796,15 @@ static int __init xen_acpi_processor_init(void)
> bitmap_free(acpi_ids_done);
> return -ENOMEM;
> }
> +
> + acpi_cppc_data = kcalloc(nr_acpi_bits, sizeof(struct cppc_perf_caps),
> + GFP_KERNEL);
> + if (!acpi_cppc_data) {
> + pr_debug("Memory allocation error for acpi_cppc_data\n");
> + rc = -ENOMEM;
> + goto err1_out;
> + }
> +
> for_each_possible_cpu(i) {
> if (!zalloc_cpumask_var_node(
> &per_cpu_ptr(acpi_perf_data, i)->shared_cpu_map,
> @@ -751,6 +830,11 @@ static int __init xen_acpi_processor_init(void)
> rc = acpi_processor_get_performance_info(pr);
> if (rc)
> goto err_out;
> +
> + pr->flags.pcc_unsupported = true;
> + rc = xen_processor_get_perf_caps(pr, acpi_cppc_data + i);
> + if (rc)
> + goto err_out;
> }
>
> rc = xen_upload_processor_pm_data();
> @@ -766,6 +850,8 @@ static int __init xen_acpi_processor_init(void)
>
> err_out:
> /* Freeing a NULL pointer is OK: alloc_percpu zeroes. */
> + kfree(acpi_cppc_data);
> +err1_out:
> free_acpi_perf_data();
> bitmap_free(acpi_ids_done);
> return rc;
> @@ -779,6 +865,7 @@ static void __exit xen_acpi_processor_exit(void)
> bitmap_free(acpi_id_present);
> bitmap_free(acpi_id_cst_present);
> kfree(acpi_psd);
> + kfree(acpi_cppc_data);
> for_each_possible_cpu(i)
> acpi_processor_unregister_performance(i);
>
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 18499cc11366..66492f5d68a8 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -214,6 +214,7 @@ struct acpi_processor_flags {
> u8 bm_control:1;
> u8 bm_check:1;
> u8 has_cst:1;
> + u8 has_cpc:1;
> u8 pcc_unsupported:1;
> u8 has_lpi:1;
> u8 power_setup_done:1;
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> index 79a443c65ea9..e11bb9443dc0 100644
> --- a/include/xen/interface/platform.h
> +++ b/include/xen/interface/platform.h
> @@ -319,6 +319,7 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_getidletime_t);
> #define XEN_PM_PX 1
> #define XEN_PM_TX 2
> #define XEN_PM_PDC 3
> +#define XEN_PM_CPPC 4
> /* Px sub info type */
> #define XEN_PX_PCT 1
> #define XEN_PX_PSS 2
> @@ -384,6 +385,15 @@ struct xen_processor_px {
> };
> DEFINE_GUEST_HANDLE_STRUCT(xen_processor_px);
>
> +struct xen_processor_cppc {
> + uint32_t highest_perf;
> + uint32_t nominal_perf;
> + uint32_t lowest_perf;
> + uint32_t lowest_nonlinear_perf;
> + uint32_t lowest_freq;
> + uint32_t nominal_freq;
> +};
> +
> struct xen_psd_package {
> uint64_t num_entries;
> uint64_t revision;
> @@ -412,6 +422,7 @@ struct xenpf_set_processor_pminfo {
> struct xen_processor_power power;/* Cx: _CST/_CSD */
> struct xen_processor_performance perf; /* Px: _PPC/_PCT/_PSS/_PSD */
> GUEST_HANDLE(uint32_t) pdc;
> + struct xen_processor_cppc cppc_data; /* _CPC */
> };
> };
> DEFINE_GUEST_HANDLE_STRUCT(xenpf_set_processor_pminfo);
Powered by blists - more mailing lists