[PATCH RESEND v4 07/10] PM: EM: Implement em_nl_get_pd_table_doit()

Changwoo Min posted 10 patches 4 months, 3 weeks ago
There is a newer version of this series
[PATCH RESEND v4 07/10] PM: EM: Implement em_nl_get_pd_table_doit()
Posted by Changwoo Min 4 months, 3 weeks ago
When a userspace requests EM_CMD_GET_PD_TABLE with an ID of a performancei
domain, the kernel reports back the energy model table of the specified
performance domain. The message format of the response is as follows:

EM_A_PD_TABLE_PD_ID (NLA_U32)
EM_A_PD_TABLE_PS (NLA_NESTED)*
    EM_A_PS_PERFORMANCE (NLA_U64)
    EM_A_PS_FREQUENCY (NLA_U64)
    EM_A_PS_POWER (NLA_U64)
    EM_A_PS_COST (NLA_U64)
    EM_A_PS_FLAGS (NLA_U64)

where EM_A_PD_TABLE_PS can be repeated as many times as there are
performance states (struct em_perf_state).

Signed-off-by: Changwoo Min <changwoo@igalia.com>
---
 kernel/power/em_netlink.c | 108 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/kernel/power/em_netlink.c b/kernel/power/em_netlink.c
index 31b27c6fe3c9..59953cfedf78 100644
--- a/kernel/power/em_netlink.c
+++ b/kernel/power/em_netlink.c
@@ -102,9 +102,115 @@ int em_nl_get_pds_doit(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
+static struct em_perf_domain *__em_nl_get_pd_table_id(struct nlattr **attrs)
+{
+	struct em_perf_domain *pd;
+	int id;
+
+	if (!attrs[EM_A_PD_TABLE_PD_ID])
+		return NULL;
+
+	id = nla_get_u32(attrs[EM_A_PD_TABLE_PD_ID]);
+	pd = em_perf_domain_get_by_id(id);
+	return pd;
+}
+
+static int __em_nl_get_pd_table_size(const struct em_perf_domain *pd)
+{
+	int id_sz, ps_sz;
+
+	id_sz = nla_total_size(sizeof(u32));		/* EM_A_PD_TABLE_PD_ID */
+	ps_sz = nla_total_size(0) +			/* EM_A_PD_TABLE_PS */
+		nla_total_size_64bit(sizeof(u64)) +	/* EM_A_PS_PERFORMANCE */
+		nla_total_size_64bit(sizeof(u64)) +	/* EM_A_PS_FREQUENCY */
+		nla_total_size_64bit(sizeof(u64)) +	/* EM_A_PS_POWER */
+		nla_total_size_64bit(sizeof(u64)) +	/* EM_A_PS_COST */
+		nla_total_size_64bit(sizeof(u64));	/* EM_A_PS_FLAGS */
+	ps_sz *= pd->nr_perf_states;
+
+	return nlmsg_total_size(genlmsg_msg_size(id_sz + ps_sz));
+}
+
+static int __em_nl_get_pd_table(struct sk_buff *msg, const struct em_perf_domain *pd)
+{
+	struct em_perf_state *table, *ps;
+	struct nlattr *entry;
+	int i;
+
+	if (nla_put_u32(msg, EM_A_PD_TABLE_PD_ID, pd->id))
+		goto out_err;
+
+	rcu_read_lock();
+	table = em_perf_state_from_pd((struct em_perf_domain *)pd);
+
+	for (i = 0; i < pd->nr_perf_states; i++) {
+		ps = &table[i];
+
+		entry = nla_nest_start(msg, EM_A_PD_TABLE_PS);
+		if (!entry)
+			goto out_unlock_ps;
+
+		if (nla_put_u64_64bit(msg, EM_A_PS_PERFORMANCE,
+				      ps->performance, EM_A_PS_PAD))
+			goto out_cancel_ps_nest;
+		if (nla_put_u64_64bit(msg, EM_A_PS_FREQUENCY,
+				      ps->frequency, EM_A_PS_PAD))
+			goto out_cancel_ps_nest;
+		if (nla_put_u64_64bit(msg, EM_A_PS_POWER,
+				      ps->power, EM_A_PS_PAD))
+			goto out_cancel_ps_nest;
+		if (nla_put_u64_64bit(msg, EM_A_PS_COST,
+				      ps->cost, EM_A_PS_PAD))
+			goto out_cancel_ps_nest;
+		if (nla_put_u64_64bit(msg, EM_A_PS_FLAGS,
+				      ps->flags, EM_A_PS_PAD))
+			goto out_cancel_ps_nest;
+
+		nla_nest_end(msg, entry);
+	}
+	rcu_read_unlock();
+	return 0;
+
+out_cancel_ps_nest:
+	nla_nest_cancel(msg, entry);
+out_unlock_ps:
+	rcu_read_unlock();
+out_err:
+	return -EMSGSIZE;
+}
+
 int em_nl_get_pd_table_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return -EOPNOTSUPP;
+	struct sk_buff *msg;
+	struct em_perf_domain *pd;
+	void *hdr;
+	int cmd = info->genlhdr->cmd;
+	int msg_sz, ret = -EMSGSIZE;
+
+	pd = __em_nl_get_pd_table_id(info->attrs);
+	if (!pd)
+		return -EINVAL;
+
+	msg_sz = __em_nl_get_pd_table_size(pd);
+
+	msg = genlmsg_new(msg_sz, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	hdr = genlmsg_put_reply(msg, info, &em_nl_family, 0, cmd);
+	if (!hdr)
+		goto out_free_msg;
+
+	ret = __em_nl_get_pd_table(msg, pd);
+	if (ret)
+		goto out_free_msg;
+
+	genlmsg_end(msg, hdr);
+	return genlmsg_reply(msg, info);
+
+out_free_msg:
+	nlmsg_free(msg);
+	return ret;
 }
 
 static int __init em_netlink_init(void)
-- 
2.51.0
Re: [PATCH RESEND v4 07/10] PM: EM: Implement em_nl_get_pd_table_doit()
Posted by Lukasz Luba 4 months ago

On 9/21/25 04:19, Changwoo Min wrote:
> When a userspace requests EM_CMD_GET_PD_TABLE with an ID of a performancei

s/performancei/performance/

> domain, the kernel reports back the energy model table of the specified
> performance domain. The message format of the response is as follows:
> 
> EM_A_PD_TABLE_PD_ID (NLA_U32)
> EM_A_PD_TABLE_PS (NLA_NESTED)*
>      EM_A_PS_PERFORMANCE (NLA_U64)
>      EM_A_PS_FREQUENCY (NLA_U64)
>      EM_A_PS_POWER (NLA_U64)
>      EM_A_PS_COST (NLA_U64)
>      EM_A_PS_FLAGS (NLA_U64)
> 
> where EM_A_PD_TABLE_PS can be repeated as many times as there are
> performance states (struct em_perf_state).
> 
> Signed-off-by: Changwoo Min <changwoo@igalia.com>
> ---
>   kernel/power/em_netlink.c | 108 +++++++++++++++++++++++++++++++++++++-
>   1 file changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/power/em_netlink.c b/kernel/power/em_netlink.c
> index 31b27c6fe3c9..59953cfedf78 100644
> --- a/kernel/power/em_netlink.c
> +++ b/kernel/power/em_netlink.c
> @@ -102,9 +102,115 @@ int em_nl_get_pds_doit(struct sk_buff *skb, struct genl_info *info)
>   	return ret;
>   }
>   
> +static struct em_perf_domain *__em_nl_get_pd_table_id(struct nlattr **attrs)
> +{
> +	struct em_perf_domain *pd;
> +	int id;
> +
> +	if (!attrs[EM_A_PD_TABLE_PD_ID])
> +		return NULL;
> +
> +	id = nla_get_u32(attrs[EM_A_PD_TABLE_PD_ID]);
> +	pd = em_perf_domain_get_by_id(id);
> +	return pd;
> +}
> +
> +static int __em_nl_get_pd_table_size(const struct em_perf_domain *pd)
> +{
> +	int id_sz, ps_sz;
> +
> +	id_sz = nla_total_size(sizeof(u32));		/* EM_A_PD_TABLE_PD_ID */
> +	ps_sz = nla_total_size(0) +			/* EM_A_PD_TABLE_PS */
> +		nla_total_size_64bit(sizeof(u64)) +	/* EM_A_PS_PERFORMANCE */
> +		nla_total_size_64bit(sizeof(u64)) +	/* EM_A_PS_FREQUENCY */
> +		nla_total_size_64bit(sizeof(u64)) +	/* EM_A_PS_POWER */
> +		nla_total_size_64bit(sizeof(u64)) +	/* EM_A_PS_COST */
> +		nla_total_size_64bit(sizeof(u64));	/* EM_A_PS_FLAGS */
> +	ps_sz *= pd->nr_perf_states;
> +
> +	return nlmsg_total_size(genlmsg_msg_size(id_sz + ps_sz));
> +}
> +
> +static int __em_nl_get_pd_table(struct sk_buff *msg, const struct em_perf_domain *pd)
> +{
> +	struct em_perf_state *table, *ps;
> +	struct nlattr *entry;
> +	int i;
> +
> +	if (nla_put_u32(msg, EM_A_PD_TABLE_PD_ID, pd->id))
> +		goto out_err;
> +
> +	rcu_read_lock();
> +	table = em_perf_state_from_pd((struct em_perf_domain *)pd);
> +
> +	for (i = 0; i < pd->nr_perf_states; i++) {
> +		ps = &table[i];
> +
> +		entry = nla_nest_start(msg, EM_A_PD_TABLE_PS);
> +		if (!entry)
> +			goto out_unlock_ps;
> +
> +		if (nla_put_u64_64bit(msg, EM_A_PS_PERFORMANCE,
> +				      ps->performance, EM_A_PS_PAD))
> +			goto out_cancel_ps_nest;
> +		if (nla_put_u64_64bit(msg, EM_A_PS_FREQUENCY,
> +				      ps->frequency, EM_A_PS_PAD))
> +			goto out_cancel_ps_nest;
> +		if (nla_put_u64_64bit(msg, EM_A_PS_POWER,
> +				      ps->power, EM_A_PS_PAD))
> +			goto out_cancel_ps_nest;
> +		if (nla_put_u64_64bit(msg, EM_A_PS_COST,
> +				      ps->cost, EM_A_PS_PAD))
> +			goto out_cancel_ps_nest;
> +		if (nla_put_u64_64bit(msg, EM_A_PS_FLAGS,
> +				      ps->flags, EM_A_PS_PAD))
> +			goto out_cancel_ps_nest;
> +
> +		nla_nest_end(msg, entry);
> +	}
> +	rcu_read_unlock();
> +	return 0;
> +
> +out_cancel_ps_nest:
> +	nla_nest_cancel(msg, entry);
> +out_unlock_ps:
> +	rcu_read_unlock();
> +out_err:
> +	return -EMSGSIZE;
> +}
> +
>   int em_nl_get_pd_table_doit(struct sk_buff *skb, struct genl_info *info)
>   {
> -	return -EOPNOTSUPP;
> +	struct sk_buff *msg;
> +	struct em_perf_domain *pd;
> +	void *hdr;
> +	int cmd = info->genlhdr->cmd;
> +	int msg_sz, ret = -EMSGSIZE;

Please put them in the reverse christmas tree order.

> +
> +	pd = __em_nl_get_pd_table_id(info->attrs);
> +	if (!pd)
> +		return -EINVAL;
> +
> +	msg_sz = __em_nl_get_pd_table_size(pd);
> +
> +	msg = genlmsg_new(msg_sz, GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	hdr = genlmsg_put_reply(msg, info, &em_nl_family, 0, cmd);
> +	if (!hdr)
> +		goto out_free_msg;
> +
> +	ret = __em_nl_get_pd_table(msg, pd);
> +	if (ret)
> +		goto out_free_msg;
> +
> +	genlmsg_end(msg, hdr);
> +	return genlmsg_reply(msg, info);
> +
> +out_free_msg:
> +	nlmsg_free(msg);
> +	return ret;
>   }
>   
>   static int __init em_netlink_init(void)

After fixing that cosmetic thing you can add:

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Re: [PATCH RESEND v4 07/10] PM: EM: Implement em_nl_get_pd_table_doit()
Posted by Changwoo Min 3 months, 4 weeks ago

On 10/10/25 19:01, Lukasz Luba wrote:
> 
> 
> On 9/21/25 04:19, Changwoo Min wrote:
>> When a userspace requests EM_CMD_GET_PD_TABLE with an ID of a 
>> performancei
> 
> s/performancei/performance/

Will fix it. Thanks!

[...]

>>   int em_nl_get_pd_table_doit(struct sk_buff *skb, struct genl_info 
>> *info)
>>   {
>> -    return -EOPNOTSUPP;
>> +    struct sk_buff *msg;
>> +    struct em_perf_domain *pd;
>> +    void *hdr;
>> +    int cmd = info->genlhdr->cmd;
>> +    int msg_sz, ret = -EMSGSIZE;
> 
> Please put them in the reverse christmas tree order.
> 

Sure, will fix it as suggested in the next version.

Regards,
Changwoo Min

> 
> After fixing that cosmetic thing you can add:
> 
> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
>