>-----Original Message-----
>From: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxxxxxxxx>
>Sent: Friday, October 20, 2023 11:59 AM
>To: intel-xe@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
>alexander.deucher@xxxxxxx; airlied@xxxxxxxxx; daniel@xxxxxxxx;
>joonas.lahtinen@xxxxxxxxxxxxxxx; ogabbay@xxxxxxxxxx; Tayar, Tomer (Habana)
><ttayar@xxxxxxxxx>; Hawking.Zhang@xxxxxxx;
>Harish.Kasiviswanathan@xxxxxxx; Felix.Kuehling@xxxxxxx;
>Luben.Tuikov@xxxxxxx; Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>
>Subject: [RFC v4 1/5] drm/netlink: Add netlink infrastructure
>
>Define the netlink registration interface and commands, attributes that
>can be commonly used across by drm drivers. This patch intends to use
>the generic netlink family to expose various stats of device. At present
>it defines some commands that shall be used to expose RAS error counters.
>
>v2:
>define common interfaces to genl netlink subsystem that all drm drivers
>can leverage.(Tomer Tayar)
>
>v3: drop DRIVER_NETLINK flag and use the driver_genl_ops structure to
>register to netlink subsystem (Daniel Vetter)
>
>v4:(Michael J. Ruhl)
>1. rename drm_genl_send to drm_genl_reply
>2. catch error from xa_store and handle appropriately
Hi Aravind,
This looks reasonable to me.
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
M
>Cc: Tomer Tayar <ttayar@xxxxxxxxx>
>Cc: Daniel Vetter <daniel@xxxxxxxx>
>Cc: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
>
>Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxxxxxxxx>
>---
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_drv.c | 7 ++
> drivers/gpu/drm/drm_netlink.c | 188
>+++++++++++++++++++++++++++++++++
> include/drm/drm_device.h | 8 ++
> include/drm/drm_drv.h | 7 ++
> include/drm/drm_netlink.h | 30 ++++++
> include/uapi/drm/drm_netlink.h | 83 +++++++++++++++
> 7 files changed, 324 insertions(+)
> create mode 100644 drivers/gpu/drm/drm_netlink.c
> create mode 100644 include/drm/drm_netlink.h
> create mode 100644 include/uapi/drm/drm_netlink.h
>
>diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>index ee64c51274ad..60864369adaa 100644
>--- a/drivers/gpu/drm/Makefile
>+++ b/drivers/gpu/drm/Makefile
>@@ -35,6 +35,7 @@ drm-y := \
> drm_mode_object.o \
> drm_modes.o \
> drm_modeset_lock.o \
>+ drm_netlink.o \
> drm_plane.o \
> drm_prime.o \
> drm_print.o \
>diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>index 535f16e7882e..31f55c1c7524 100644
>--- a/drivers/gpu/drm/drm_drv.c
>+++ b/drivers/gpu/drm/drm_drv.c
>@@ -937,6 +937,12 @@ int drm_dev_register(struct drm_device *dev,
>unsigned long flags)
> if (ret)
> goto err_minors;
>
>+ if (driver->genl_ops) {
>+ ret = drm_genl_register(dev);
>+ if (ret)
>+ goto err_minors;
>+ }
>+
> ret = create_compat_control_link(dev);
> if (ret)
> goto err_minors;
>@@ -1074,6 +1080,7 @@ static void drm_core_exit(void)
> {
> drm_privacy_screen_lookup_exit();
> accel_core_exit();
>+ drm_genl_exit();
> unregister_chrdev(DRM_MAJOR, "drm");
> debugfs_remove(drm_debugfs_root);
> drm_sysfs_destroy();
>diff --git a/drivers/gpu/drm/drm_netlink.c b/drivers/gpu/drm/drm_netlink.c
>new file mode 100644
>index 000000000000..8add249c1da3
>--- /dev/null
>+++ b/drivers/gpu/drm/drm_netlink.c
>@@ -0,0 +1,188 @@
>+// SPDX-License-Identifier: MIT
>+/*
>+ * Copyright © 2023 Intel Corporation
>+ */
>+
>+#include <drm/drm_device.h>
>+#include <drm/drm_drv.h>
>+#include <drm/drm_file.h>
>+#include <drm/drm_managed.h>
>+#include <drm/drm_netlink.h>
>+#include <drm/drm_print.h>
>+
>+DEFINE_XARRAY(drm_dev_xarray);
>+
>+/**
>+ * drm_genl_reply - response to a request
>+ * @msg: socket buffer
>+ * @info: receiver information
>+ * @usrhdr: pointer to user specific header in the message buffer
>+ *
>+ * RETURNS:
>+ * 0 on success and negative error code on failure
>+ */
>+int drm_genl_reply(struct sk_buff *msg, struct genl_info *info, void *usrhdr)
>+{
>+ int ret;
>+
>+ genlmsg_end(msg, usrhdr);
>+
>+ ret = genlmsg_reply(msg, info);
>+ if (ret)
>+ nlmsg_free(msg);
>+
>+ return ret;
>+}
>+EXPORT_SYMBOL(drm_genl_reply);
>+
>+/**
>+ * drm_genl_alloc_msg - allocate genl message buffer
>+ * @dev: drm_device for which the message is being allocated
>+ * @info: receiver information
>+ * @usrhdr: pointer to user specific header in the message buffer
>+ *
>+ * RETURNS:
>+ * pointer to new allocated buffer on success, NULL on failure
>+ */
>+struct sk_buff *
>+drm_genl_alloc_msg(struct drm_device *dev,
>+ struct genl_info *info,
>+ size_t msg_size, void **usrhdr)
>+{
>+ struct sk_buff *new_msg;
>+ new_msg = genlmsg_new(msg_size, GFP_KERNEL);
>+ if (!new_msg)
>+ return new_msg;
>+
>+ *usrhdr = genlmsg_put_reply(new_msg, info, &dev->drm_genl_family, 0, info->genlhdr->cmd);
>+ if (!*usrhdr) {
>+ nlmsg_free(new_msg);
>+ new_msg = NULL;
>+ }
>+
>+ return new_msg;
>+}
>+EXPORT_SYMBOL(drm_genl_alloc_msg);
>+
>+static struct drm_device *genl_to_dev(struct genl_info *info)
>+{
>+ return xa_load(&drm_dev_xarray, info->nlhdr->nlmsg_type);
>+}
>+
>+static int drm_genl_list_errors(struct sk_buff *msg, struct genl_info *info)
>+{
>+ struct drm_device *dev = genl_to_dev(info);
>+
>+ if (GENL_REQ_ATTR_CHECK(info, DRM_RAS_ATTR_REQUEST))
>+ return -EINVAL;
>+
>+ if (WARN_ON(!dev->driver->genl_ops[info->genlhdr->cmd].doit))
>+ return -EOPNOTSUPP;
>+
>+ return dev->driver->genl_ops[info->genlhdr->cmd].doit(dev, msg,
>info);
>+}
>+
>+static int drm_genl_read_error(struct sk_buff *msg, struct genl_info *info)
>+{
>+ struct drm_device *dev = genl_to_dev(info);
>+
>+ if (GENL_REQ_ATTR_CHECK(info, DRM_RAS_ATTR_ERROR_ID))
>+ return -EINVAL;
>+
>+ if (WARN_ON(!dev->driver->genl_ops[info->genlhdr->cmd].doit))
>+ return -EOPNOTSUPP;
>+
>+ return dev->driver->genl_ops[info->genlhdr->cmd].doit(dev, msg,
>info);
>+}
>+
>+/* attribute policies */
>+static const struct nla_policy drm_attr_policy_query[DRM_ATTR_MAX + 1] = {
>+ [DRM_RAS_ATTR_REQUEST] = { .type = NLA_U8 },
>+};
>+
>+static const struct nla_policy drm_attr_policy_read_one[DRM_ATTR_MAX + 1]
>= {
>+ [DRM_RAS_ATTR_ERROR_ID] = { .type = NLA_U64 },
>+};
>+
>+/* drm genl operations definition */
>+const struct genl_ops drm_genl_ops[] = {
>+ {
>+ .cmd = DRM_RAS_CMD_QUERY,
>+ .doit = drm_genl_list_errors,
>+ .policy = drm_attr_policy_query,
>+ },
>+ {
>+ .cmd = DRM_RAS_CMD_READ_ONE,
>+ .doit = drm_genl_read_error,
>+ .policy = drm_attr_policy_read_one,
>+ },
>+ {
>+ .cmd = DRM_RAS_CMD_READ_ALL,
>+ .doit = drm_genl_list_errors,
>+ .policy = drm_attr_policy_query,
>+ },
>+};
>+
>+static void drm_genl_family_init(struct drm_device *dev)
>+{
>+ /* Use drm primary node name eg: card0 to name the genl family */
>+ snprintf(dev->drm_genl_family.name, sizeof(dev->drm_genl_family.name), "%s", dev->primary->kdev->kobj.name);
>+ dev->drm_genl_family.version = DRM_GENL_VERSION;
>+ dev->drm_genl_family.parallel_ops = true;
>+ dev->drm_genl_family.ops = drm_genl_ops;
>+ dev->drm_genl_family.n_ops = ARRAY_SIZE(drm_genl_ops);
>+ dev->drm_genl_family.maxattr = DRM_ATTR_MAX;
>+ dev->drm_genl_family.module = dev->dev->driver->owner;
>+}
>+
>+static void drm_genl_deregister(struct drm_device *dev, void *arg)
>+{
>+ drm_dbg_driver(dev, "unregistering genl family %s\n", dev->drm_genl_family.name);
>+
>+ xa_erase(&drm_dev_xarray, dev->drm_genl_family.id);
>+
>+ genl_unregister_family(&dev->drm_genl_family);
>+}
>+
>+/**
>+ * drm_genl_register - Register genl family
>+ * @dev: drm_device for which genl family needs to be registered
>+ *
>+ * RETURNS:
>+ * 0 on success and negative error code on failure
>+ */
>+int drm_genl_register(struct drm_device *dev)
>+{
>+ int ret;
>+
>+ drm_genl_family_init(dev);
>+
>+ ret = genl_register_family(&dev->drm_genl_family);
>+ if (ret < 0) {
>+ drm_warn(dev, "genl family registration failed\n");
>+ return ret;
>+ }
>+
>+ drm_dbg_driver(dev, "genl family id %d and name %s\n", dev->drm_genl_family.id, dev->drm_genl_family.name);
>+
>+ ret = xa_err(xa_store(&drm_dev_xarray, dev->drm_genl_family.id, dev, GFP_KERNEL));
>+ if (ret)
>+ goto genl_unregister;
>+
>+ ret = drmm_add_action_or_reset(dev, drm_genl_deregister, NULL);
>+
>+ return ret;
>+
>+genl_unregister:
>+ genl_unregister_family(&dev->drm_genl_family);
>+ return ret;
>+}
>+
>+/**
>+ * drm_genl_exit: destroy drm_dev_xarray
>+ */
>+void drm_genl_exit(void)
>+{
>+ xa_destroy(&drm_dev_xarray);
>+}
>diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
>index c490977ee250..d3ae91b7714d 100644
>--- a/include/drm/drm_device.h
>+++ b/include/drm/drm_device.h
>@@ -8,6 +8,7 @@
>
> #include <drm/drm_legacy.h>
> #include <drm/drm_mode_config.h>
>+#include <drm/drm_netlink.h>
>
> struct drm_driver;
> struct drm_minor;
>@@ -318,6 +319,13 @@ struct drm_device {
> */
> struct dentry *debugfs_root;
>
>+ /**
>+ * @drm_genl_family:
>+ *
>+ * Generic netlink family registration structure.
>+ */
>+ struct genl_family drm_genl_family;
>+
> /* Everything below here is for legacy driver, never use! */
> /* private: */
> #if IS_ENABLED(CONFIG_DRM_LEGACY)
>diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>index e2640dc64e08..ebdb7850d235 100644
>--- a/include/drm/drm_drv.h
>+++ b/include/drm/drm_drv.h
>@@ -434,6 +434,13 @@ struct drm_driver {
> */
> const struct file_operations *fops;
>
>+ /**
>+ * @genl_ops:
>+ *
>+ * Drivers private callback to genl commands
>+ */
>+ const struct driver_genl_ops *genl_ops;
>+
> #ifdef CONFIG_DRM_LEGACY
> /* Everything below here is for legacy driver, never use! */
> /* private: */
>diff --git a/include/drm/drm_netlink.h b/include/drm/drm_netlink.h
>new file mode 100644
>index 000000000000..54527dae7847
>--- /dev/null
>+++ b/include/drm/drm_netlink.h
>@@ -0,0 +1,30 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2023 Intel Corporation
>+ */
>+
>+#ifndef __DRM_NETLINK_H__
>+#define __DRM_NETLINK_H__
>+
>+#include <linux/netdevice.h>
>+#include <net/genetlink.h>
>+#include <net/sock.h>
>+#include <uapi/drm/drm_netlink.h>
>+
>+struct drm_device;
>+
>+struct driver_genl_ops {
>+ int (*doit)(struct drm_device *dev,
>+ struct sk_buff *skb,
>+ struct genl_info *info);
>+};
>+
>+int drm_genl_register(struct drm_device *dev);
>+void drm_genl_exit(void);
>+int drm_genl_reply(struct sk_buff *msg, struct genl_info *info, void *usrhdr);
>+struct sk_buff *
>+drm_genl_alloc_msg(struct drm_device *dev,
>+ struct genl_info *info,
>+ size_t msg_size, void **usrhdr);
>+#endif
>+
>diff --git a/include/uapi/drm/drm_netlink.h b/include/uapi/drm/drm_netlink.h
>new file mode 100644
>index 000000000000..aab42147a20e
>--- /dev/null
>+++ b/include/uapi/drm/drm_netlink.h
>@@ -0,0 +1,83 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright 2023 Intel Corporation
>+ *
>+ * Permission is hereby granted, free of charge, to any person obtaining a
>+ * copy of this software and associated documentation files (the "Software"),
>+ * to deal in the Software without restriction, including without limitation
>+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>+ * and/or sell copies of the Software, and to permit persons to whom the
>+ * Software is furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice (including the next
>+ * paragraph) shall be included in all copies or substantial portions of the
>+ * Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
>EVENT SHALL
>+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
>DAMAGES OR
>+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>OTHERWISE,
>+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>USE OR
>+ * OTHER DEALINGS IN THE SOFTWARE.
>+ */
>+
>+#ifndef _DRM_NETLINK_H_
>+#define _DRM_NETLINK_H_
>+
>+#define DRM_GENL_VERSION 1
>+
>+#if defined(__cplusplus)
>+extern "C" {
>+#endif
>+
>+/**
>+ * enum drm_genl_error_cmds - Supported error commands
>+ *
>+ */
>+enum drm_genl_error_cmds {
>+ DRM_CMD_UNSPEC,
>+ /** @DRM_RAS_CMD_QUERY: Command to list all errors names with
>config-id */
>+ DRM_RAS_CMD_QUERY,
>+ /** @DRM_RAS_CMD_READ_ONE: Command to get a counter for a
>specific error */
>+ DRM_RAS_CMD_READ_ONE,
>+ /** @DRM_RAS_CMD_READ_ALL: Command to get counters of all
>errors */
>+ DRM_RAS_CMD_READ_ALL,
>+
>+ __DRM_CMD_MAX,
>+ DRM_CMD_MAX = __DRM_CMD_MAX - 1,
>+};
>+
>+/**
>+ * enum drm_error_attr - Attributes to use with drm_genl_error_cmds
>+ *
>+ */
>+enum drm_error_attr {
>+ DRM_ATTR_UNSPEC,
>+ DRM_ATTR_PAD = DRM_ATTR_UNSPEC,
>+ /**
>+ * @DRM_RAS_ATTR_REQUEST: Should be used with
>DRM_RAS_CMD_QUERY,
>+ * DRM_RAS_CMD_READ_ALL
>+ */
>+ DRM_RAS_ATTR_REQUEST, /* NLA_U8 */
>+ /**
>+ * @DRM_RAS_ATTR_QUERY_REPLY: First Nested attributed sent as a
>+ * response to DRM_RAS_CMD_QUERY, DRM_RAS_CMD_READ_ALL
>commands.
>+ */
>+ DRM_RAS_ATTR_QUERY_REPLY, /*NLA_NESTED*/
>+ /** @DRM_RAS_ATTR_ERROR_NAME: Used to pass error name */
>+ DRM_RAS_ATTR_ERROR_NAME, /* NLA_NUL_STRING */
>+ /** @DRM_RAS_ATTR_ERROR_ID: Used to pass error id */
>+ DRM_RAS_ATTR_ERROR_ID, /* NLA_U64 */
>+ /** @DRM_RAS_ATTR_ERROR_VALUE: Used to pass error value */
>+ DRM_RAS_ATTR_ERROR_VALUE, /* NLA_U64 */
>+
>+ __DRM_ATTR_MAX,
>+ DRM_ATTR_MAX = __DRM_ATTR_MAX - 1,
>+};
>+
>+#if defined(__cplusplus)
>+}
>+#endif
>+
>+#endif
>--
>2.25.1
