| 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
| ||
|
Date: Wed, 12 Jan 2022 20:25:59 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Linux PM <linux-pm@...r.kernel.org>,
"the arch/x86 maintainers" <x86@...nel.org>,
"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
Len Brown <len.brown@...el.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Aubrey Li <aubrey.li@...ux.intel.com>,
Amit Kucheria <amitk@...nel.org>,
Andi Kleen <ak@...ux.intel.com>,
Tim Chen <tim.c.chen@...ux.intel.com>,
Lukasz Luba <lukasz.luba@....com>,
"Ravi V. Shankar" <ravi.v.shankar@...el.com>,
Ricardo Neri <ricardo.neri@...el.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 6/7] thermal: netlink: Add a new event to notify CPU
capabilities change
On Thu, Jan 6, 2022 at 3:49 AM Ricardo Neri
<ricardo.neri-calderon@...ux.intel.com> wrote:
>
> From: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
>
> Add a new netlink event to notify change in CPU capabilities in terms of
> performance and efficiency.
>
> Firmware may change CPU capabilities as a result of thermal events in the
> system or to account for changes in the TDP (thermal design power) level.
>
> This notification type will allow user space to avoid running workloads
> on certain CPUs or proactively adjust power limits to avoid future events.
>
> The netlink message consists of a nested attribute
> (THERMAL_GENL_ATTR_CPU_CAPABILITY) with three attributes:
>
> * THERMAL_GENL_ATTR_CPU_CAPABILITY_ID (type u32):
> -- logical CPU number
> * THERMAL_GENL_ATTR_CPU_CAPABILITY_PERFORMANCE (type u32):
> -- Scaled performance from 0-1023
> * THERMAL_GENL_ATTR_CPU_CAPABILITY_EFFICIENCY (type u32):
> -- Scaled efficiency from 0-1023
>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Aubrey Li <aubrey.li@...ux.intel.com>
> Cc: Lukasz Luba <lukasz.luba@....com>
> Cc: Tim Chen <tim.c.chen@...ux.intel.com>
> Cc: "Ravi V. Shankar" <ravi.v.shankar@...el.com>
> Reviewed-by: Len Brown <len.brown@...el.com>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Of course, I need to know if Daniel and Lukasz agree with this patch.
> ---
> Changes since v2:
> * None
>
> Changes since v1:
> * Reworded commit message.
> * Reworded the members of struct cpu_capacity for clarity. (Lukasz)
> * Defined the CPU capability attributes to be scaled in the [0, 1023]
> interval. (Lukasz)
> ---
> drivers/thermal/thermal_netlink.c | 55 +++++++++++++++++++++++++++++++
> drivers/thermal/thermal_netlink.h | 13 ++++++++
> include/uapi/linux/thermal.h | 6 +++-
> 3 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
> index a16dd4d5d710..38e6c20f460c 100644
> --- a/drivers/thermal/thermal_netlink.c
> +++ b/drivers/thermal/thermal_netlink.c
> @@ -43,6 +43,11 @@ static const struct nla_policy thermal_genl_policy[THERMAL_GENL_ATTR_MAX + 1] =
> [THERMAL_GENL_ATTR_CDEV_MAX_STATE] = { .type = NLA_U32 },
> [THERMAL_GENL_ATTR_CDEV_NAME] = { .type = NLA_STRING,
> .len = THERMAL_NAME_LENGTH },
> + /* CPU capabilities */
> + [THERMAL_GENL_ATTR_CPU_CAPABILITY] = { .type = NLA_NESTED },
> + [THERMAL_GENL_ATTR_CPU_CAPABILITY_ID] = { .type = NLA_U32 },
> + [THERMAL_GENL_ATTR_CPU_CAPABILITY_PERFORMANCE] = { .type = NLA_U32 },
> + [THERMAL_GENL_ATTR_CPU_CAPABILITY_EFFICIENCY] = { .type = NLA_U32 },
> };
>
> struct param {
> @@ -58,6 +63,8 @@ struct param {
> int temp;
> int cdev_state;
> int cdev_max_state;
> + struct cpu_capability *cpu_capabilities;
> + int cpu_capabilities_count;
> };
>
> typedef int (*cb_t)(struct param *);
> @@ -190,6 +197,45 @@ static int thermal_genl_event_gov_change(struct param *p)
> return 0;
> }
>
> +static int thermal_genl_event_cpu_capability_change(struct param *p)
> +{
> + struct cpu_capability *cpu_cap = p->cpu_capabilities;
> + struct sk_buff *msg = p->msg;
> + struct nlattr *start_cap;
> + int i, ret;
> +
> + start_cap = nla_nest_start(msg, THERMAL_GENL_ATTR_CPU_CAPABILITY);
> + if (!start_cap)
> + return -EMSGSIZE;
> +
> + for (i = 0; i < p->cpu_capabilities_count; ++i) {
> + if (nla_put_u32(msg, THERMAL_GENL_ATTR_CPU_CAPABILITY_ID,
> + cpu_cap->cpu)) {
> + ret = -EMSGSIZE;
> + goto out_cancel_nest;
> + }
> + if (nla_put_u32(msg, THERMAL_GENL_ATTR_CPU_CAPABILITY_PERFORMANCE,
> + cpu_cap->performance)) {
> + ret = -EMSGSIZE;
> + goto out_cancel_nest;
> + }
> + if (nla_put_u32(msg, THERMAL_GENL_ATTR_CPU_CAPABILITY_EFFICIENCY,
> + cpu_cap->efficiency)) {
> + ret = -EMSGSIZE;
> + goto out_cancel_nest;
> + }
> + ++cpu_cap;
> + }
> +
> + nla_nest_end(msg, start_cap);
> +
> + return 0;
> +out_cancel_nest:
> + nla_nest_cancel(msg, start_cap);
> +
> + return ret;
It looks like ret is never different from -EMSGSIZE here, so I'd just
return that error and drop the ret variable.
> +}
> +
> int thermal_genl_event_tz_delete(struct param *p)
> __attribute__((alias("thermal_genl_event_tz")));
>
> @@ -219,6 +265,7 @@ static cb_t event_cb[] = {
> [THERMAL_GENL_EVENT_CDEV_DELETE] = thermal_genl_event_cdev_delete,
> [THERMAL_GENL_EVENT_CDEV_STATE_UPDATE] = thermal_genl_event_cdev_state_update,
> [THERMAL_GENL_EVENT_TZ_GOV_CHANGE] = thermal_genl_event_gov_change,
> + [THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE] = thermal_genl_event_cpu_capability_change,
> };
>
> /*
> @@ -356,6 +403,14 @@ int thermal_notify_tz_gov_change(int tz_id, const char *name)
> return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_GOV_CHANGE, &p);
> }
>
> +int thermal_genl_cpu_capability_event(int count, struct cpu_capability *caps)
> +{
> + struct param p = { .cpu_capabilities_count = count, .cpu_capabilities = caps };
> +
> + return thermal_genl_send_event(THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE, &p);
> +}
> +EXPORT_SYMBOL_GPL(thermal_genl_cpu_capability_event);
> +
> /*************************** Command encoding ********************************/
>
> static int __thermal_genl_cmd_tz_get_id(struct thermal_zone_device *tz,
> diff --git a/drivers/thermal/thermal_netlink.h b/drivers/thermal/thermal_netlink.h
> index e554f76291f4..44bc3dec5568 100644
> --- a/drivers/thermal/thermal_netlink.h
> +++ b/drivers/thermal/thermal_netlink.h
> @@ -4,6 +4,12 @@
> * Author: Daniel Lezcano <daniel.lezcano@...aro.org>
> */
>
> +struct cpu_capability {
I'm wondering if the struct name is not too generic as the purpose it
is used for is rather narrow and specific.
> + int cpu;
> + int performance;
> + int efficiency;
> +};
> +
> /* Netlink notification function */
> #ifdef CONFIG_THERMAL_NETLINK
> int __init thermal_netlink_init(void);
> @@ -23,6 +29,7 @@ int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state);
> int thermal_notify_cdev_delete(int cdev_id);
> int thermal_notify_tz_gov_change(int tz_id, const char *name);
> int thermal_genl_sampling_temp(int id, int temp);
> +int thermal_genl_cpu_capability_event(int count, struct cpu_capability *caps);
> #else
> static inline int thermal_netlink_init(void)
> {
> @@ -101,4 +108,10 @@ static inline int thermal_genl_sampling_temp(int id, int temp)
> {
> return 0;
> }
> +
> +static inline int thermal_genl_cpu_capability_event(int count, struct cpu_capability *caps)
> +{
> + return 0;
> +}
> +
> #endif /* CONFIG_THERMAL_NETLINK */
> diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
> index 9aa2fedfa309..fc78bf3aead7 100644
> --- a/include/uapi/linux/thermal.h
> +++ b/include/uapi/linux/thermal.h
> @@ -44,7 +44,10 @@ enum thermal_genl_attr {
> THERMAL_GENL_ATTR_CDEV_MAX_STATE,
> THERMAL_GENL_ATTR_CDEV_NAME,
> THERMAL_GENL_ATTR_GOV_NAME,
> -
> + THERMAL_GENL_ATTR_CPU_CAPABILITY,
> + THERMAL_GENL_ATTR_CPU_CAPABILITY_ID,
> + THERMAL_GENL_ATTR_CPU_CAPABILITY_PERFORMANCE,
> + THERMAL_GENL_ATTR_CPU_CAPABILITY_EFFICIENCY,
> __THERMAL_GENL_ATTR_MAX,
> };
> #define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
> @@ -71,6 +74,7 @@ enum thermal_genl_event {
> THERMAL_GENL_EVENT_CDEV_DELETE, /* Cdev unbound */
> THERMAL_GENL_EVENT_CDEV_STATE_UPDATE, /* Cdev state updated */
> THERMAL_GENL_EVENT_TZ_GOV_CHANGE, /* Governor policy changed */
> + THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE, /* CPU capability changed */
> __THERMAL_GENL_EVENT_MAX,
> };
> #define THERMAL_GENL_EVENT_MAX (__THERMAL_GENL_EVENT_MAX - 1)
> --
> 2.17.1
>
Powered by blists - more mailing lists