diff mbox series

[02/11] PM: EM: Add a skeleton code for netlink notification.

Message ID 20250529001315.233492-3-changwoo@igalia.com
State New
Headers show
Series [01/11] PM: EM: Add ENERGY_MODEL_NETLINK Kconfig. | expand

Commit Message

Changwoo Min May 29, 2025, 12:13 a.m. UTC 
Add a boilerplate code for netlink notification to register and unregister
the new protocol family. It defines the supported commands and event types
and adds the minimalistic code for the protocol family registration.

Signed-off-by: Changwoo Min <changwoo@igalia.com>
---
 include/uapi/linux/energy_model.h | 40 ++++++++++++++++++
 kernel/power/Makefile             |  1 +
 kernel/power/em_netlink.c         | 69 +++++++++++++++++++++++++++++++
 kernel/power/em_netlink.h         | 26 ++++++++++++
 4 files changed, 136 insertions(+)
 create mode 100644 include/uapi/linux/energy_model.h
 create mode 100644 kernel/power/em_netlink.c
 create mode 100644 kernel/power/em_netlink.h

Comments

Lukas Wunner June 2, 2025, 7:53 p.m. UTC | #1 
> diff --git a/include/uapi/linux/energy_model.h b/include/uapi/linux/energy_model.h
> new file mode 100644
> index 000000000000..42a19e614c7d
> --- /dev/null
> +++ b/include/uapi/linux/energy_model.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_LINUX_ENERGY_MODEL_H
> +#define _UAPI_LINUX_ENERGY_MODEL_H
> +

It looks like you created the header file manually.  There is tooling
to auto-generate all the boilerplate code from a YAML description in
Documentation/netlink/specs/ and my (limited) understanding is that
using it is mandatory for all newly introduced Netlink protocols.

I just had to wrap my head around all that for SPDM (a device
authentication protocol), see the top-most commit on this branch,
which is in a WIP state though:

https://github.com/l1k/linux/commits/doe

Basically you create the uapi and kernel header files plus kernel source
like this:

tools/net/ynl/pyynl/ynl_gen_c.py --spec Documentation/netlink/specs/em.yaml \
  --mode uapi --header
tools/net/ynl/pyynl/ynl_gen_c.py --spec Documentation/netlink/specs/em.yaml \
  --mode kernel --header
tools/net/ynl/pyynl/ynl_gen_c.py --spec Documentation/netlink/specs/em.yaml \
  --mode kernel --source

And then you add both the YAML file as well as the generated files to
the commit.  The reason you have to do that is because Python is
optional for building the kernel per Documentation/process/changes.rst,
so the files cannot be generated at compile time.  It is possible though
to regenerate them with tools/net/ynl/ynl-regen.sh whenever the YAML file
is changed.

The tooling is somewhat brittle, see 396786af1cea.  In theory ynl_gen_c.py
is capable of auto-generating code for user space applications as well
but it crashed when parsing my YAML file.  So there are more bugs,
just haven't had the time yet to fix them.


> +int __init em_netlink_init(void)
> +{
> +	return genl_register_family(&em_genl_family);
> +}
> +
> +void __init em_netlink_exit(void)
> +{
> +	genl_unregister_family(&em_genl_family);
> +}
> +

It looks like em_netlink_exit() isn't invoked anywhere, so why define
it in the first place?  You only need this if the feature can be modular
(which it cannot - it's gated by a bool Kconfig option).  Then you'd
call em_netlink_exit() in module_exit().

Also, you may want to consider moving this to patch [03/11], where
em_netlink_init() is actually invoked.  And you may want to move the
postcore_initcall() to this file so that you can declare em_netlink_init()
static, don't need em_init() and don't need the empty inline stubs.

Thanks,

Lukas
diff mbox series

Patch

diff --git a/include/uapi/linux/energy_model.h b/include/uapi/linux/energy_model.h
new file mode 100644
index 000000000000..42a19e614c7d
--- /dev/null
+++ b/include/uapi/linux/energy_model.h
@@ -0,0 +1,40 @@ 
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_ENERGY_MODEL_H
+#define _UAPI_LINUX_ENERGY_MODEL_H
+
+/* Adding event notification support elements */
+#define EM_GENL_FAMILY_NAME		"energy_model"
+#define EM_GENL_VERSION			0x01
+#define EM_GENL_EVENT_GROUP_NAME	"event"
+
+/* Attributes of em_genl_family */
+enum em_genl_attr {
+	EM_GENL_ATTR_UNSPEC,
+	__EM_GENL_ATTR_MAX,
+};
+#define EM_GENL_ATTR_MAX (__EM_GENL_ATTR_MAX - 1)
+
+/* Events of em_genl_family */
+enum em_genl_event {
+	EM_GENL_EVENT_UNSPEC,
+	EM_GENL_EVENT_PD_CREATE,	/* Performance domain creation */
+	EM_GENL_EVENT_PD_DELETE,	/* Performance domain deletion */
+	EM_GENL_EVENT_PD_UPDATE,	/* The runtime EM table for the
+					   performance domain is updated */
+	__EM_GENL_EVENT_MAX,
+};
+#define EM_GENL_EVENT_MAX (__EM_GENL_EVENT_MAX - 1)
+
+/* Commands supported by the em_genl_family */
+enum em_genl_cmd {
+	EM_GENL_CMD_UNSPEC,
+	EM_GENL_CMD_PD_GET_ID,		/* Get the list of information
+					   for all performance domains */
+	EM_GENL_CMD_PD_GET_TBL,		/* Get the energy model table
+					   of a performance domain */
+	__EM_GENL_CMD_MAX,
+};
+#define EM_GENL_CMD_MAX (__EM_GENL_CMD_MAX - 1)
+
+
+#endif /* _UAPI_LINUX_ENERGY_MODEL_H */
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 874ad834dc8d..6bf157b5fffd 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -22,3 +22,4 @@  obj-$(CONFIG_PM_WAKELOCKS)	+= wakelock.o
 obj-$(CONFIG_MAGIC_SYSRQ)	+= poweroff.o
 
 obj-$(CONFIG_ENERGY_MODEL)	+= energy_model.o
+obj-$(CONFIG_ENERGY_MODEL_NETLINK) += em_netlink.o
diff --git a/kernel/power/em_netlink.c b/kernel/power/em_netlink.c
new file mode 100644
index 000000000000..30d83fb5a3a8
--- /dev/null
+++ b/kernel/power/em_netlink.c
@@ -0,0 +1,69 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *
+ * Generic netlink for energy model.
+ *
+ * Copyright (c) 2025 Valve Corporation.
+ * Author: Changwoo Min <changwoo@igalia.com>
+ */
+
+#define pr_fmt(fmt) "energy_model: " fmt
+
+#include <linux/energy_model.h>
+#include <net/sock.h>
+#include <net/genetlink.h>
+#include <uapi/linux/energy_model.h>
+
+#include "em_netlink.h"
+
+static const struct genl_multicast_group em_genl_mcgrps[] = {
+	[EM_GENL_EVENT_GROUP]  = { .name = EM_GENL_EVENT_GROUP_NAME,  },
+};
+
+static const struct nla_policy em_genl_policy[EM_GENL_ATTR_MAX + 1] = {
+};
+
+static struct genl_family em_genl_family;
+
+
+static int em_genl_cmd_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	return -ENOTSUPP;
+}
+
+static const struct genl_small_ops em_genl_ops[] = {
+	{
+		.cmd = EM_GENL_CMD_PD_GET_ID,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = em_genl_cmd_doit,
+	},
+	{
+		.cmd = EM_GENL_CMD_PD_GET_TBL,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = em_genl_cmd_doit,
+	},
+};
+
+static struct genl_family em_genl_family __ro_after_init = {
+	.hdrsize	= 0,
+	.name		= EM_GENL_FAMILY_NAME,
+	.version	= EM_GENL_VERSION,
+	.maxattr	= EM_GENL_ATTR_MAX,
+	.policy		= em_genl_policy,
+	.small_ops	= em_genl_ops,
+	.n_small_ops	= ARRAY_SIZE(em_genl_ops),
+	.resv_start_op	= __EM_GENL_CMD_MAX,
+	.mcgrps		= em_genl_mcgrps,
+	.n_mcgrps	= ARRAY_SIZE(em_genl_mcgrps),
+};
+
+int __init em_netlink_init(void)
+{
+	return genl_register_family(&em_genl_family);
+}
+
+void __init em_netlink_exit(void)
+{
+	genl_unregister_family(&em_genl_family);
+}
+
diff --git a/kernel/power/em_netlink.h b/kernel/power/em_netlink.h
new file mode 100644
index 000000000000..8cedc6495916
--- /dev/null
+++ b/kernel/power/em_netlink.h
@@ -0,0 +1,26 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *
+ * Generic netlink for energy model.
+ *
+ * Copyright (c) 2025 Valve Corporation.
+ * Author: Changwoo Min <changwoo@igalia.com>
+ */
+
+enum em_genl_multicast_groups {
+	EM_GENL_EVENT_GROUP = 0,
+	EM_GENL_MAX_GROUP = EM_GENL_EVENT_GROUP,
+};
+
+/* Netlink notification function */
+#ifdef CONFIG_ENERGY_MODEL_NETLINK
+int __init em_netlink_init(void);
+void __init em_netlink_exit(void);
+#else
+static inline int em_netlink_init(void)
+{
+	return 0;
+}
+
+static inline void em_netlink_exit(void) {}
+#endif /* CONFIG_ENERGY_MODEL_NETLINK */