diff mbox series

[v1,11/19] cxl: Add support for get driver information

Message ID 20250122235159.2716036-12-dave.jiang@intel.com
State New
Headers show
Series cxl: Add CXL feature commands support via fwctl | expand

Commit Message

Dave Jiang Jan. 22, 2025, 11:50 p.m. UTC
Add definition for fwctl_ops->info() to return driver information. The
function will return a mask of the feature mailbox commands supported
by the fwctl char device.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v1:
- Add missed setting of *length for ->info().
- Use BIT() instead of enum directly.
---
 drivers/cxl/features.c   | 30 ++++++++++++++++++++++++++++--
 include/cxl/features.h   |  7 -------
 include/cxl/mailbox.h    |  1 +
 include/uapi/fwctl/cxl.h | 31 +++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 9 deletions(-)
 create mode 100644 include/uapi/fwctl/cxl.h

Comments

Jonathan Cameron Jan. 23, 2025, 6:09 p.m. UTC | #1
On Wed, 22 Jan 2025 16:50:42 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add definition for fwctl_ops->info() to return driver information. The
> function will return a mask of the feature mailbox commands supported
> by the fwctl char device.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

One follow through style thing otherwise lgtm
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
> new file mode 100644
> index 000000000000..79b822dbfafd
> --- /dev/null
> +++ b/include/uapi/fwctl/cxl.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (c) 2024, Intel Corporation
> + *
> + * These are definitions for the mailbox command interface of CXL subsystem.
> + */
> +#ifndef _UAPI_FWCTL_CXL_H_
> +#define _UAPI_FWCTL_CXL_H_
> +
> +#include <linux/types.h>
> +
> +enum feature_cmds {
> +	CXL_FEATURE_ID_GET_SUPPORTED_FEATURES = 0,
> +	CXL_FEATURE_ID_GET_FEATURE,
> +	CXL_FEATURE_ID_SET_FEATURE,
> +	CXL_FEATURE_ID_MAX,

No trailing comma on this terminating item.

> +};
> +
> +/**
> + * struct fwctl_info_cxl - ioctl(FWCTL_INFO) out_device_data
> + * @cmd_mask: Mask indicate which commands are supported based on 'enum feature_cmds'
> + *
> + * Return basic information about the FW interface available.
> + *
> + * nr_commands is number of hardware commands the driver supports. Use
> + * FWCTL_CMD_HW_INFO ioctl to request additional information.
> + */
> +struct fwctl_info_cxl {
> +	__u32 cmd_mask;
> +};
> +#endif
Dan Williams Jan. 25, 2025, 1:26 a.m. UTC | #2
Dave Jiang wrote:
> Add definition for fwctl_ops->info() to return driver information. The
> function will return a mask of the feature mailbox commands supported
> by the fwctl char device.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v1:
> - Add missed setting of *length for ->info().
> - Use BIT() instead of enum directly.
> ---
>  drivers/cxl/features.c   | 30 ++++++++++++++++++++++++++++--
>  include/cxl/features.h   |  7 -------
>  include/cxl/mailbox.h    |  1 +
>  include/uapi/fwctl/cxl.h | 31 +++++++++++++++++++++++++++++++
>  4 files changed, 60 insertions(+), 9 deletions(-)
>  create mode 100644 include/uapi/fwctl/cxl.h
[..]
> diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
> new file mode 100644
> index 000000000000..79b822dbfafd
> --- /dev/null
> +++ b/include/uapi/fwctl/cxl.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (c) 2024, Intel Corporation
> + *
> + * These are definitions for the mailbox command interface of CXL subsystem.
> + */
> +#ifndef _UAPI_FWCTL_CXL_H_
> +#define _UAPI_FWCTL_CXL_H_
> +
> +#include <linux/types.h>
> +
> +enum feature_cmds {
> +	CXL_FEATURE_ID_GET_SUPPORTED_FEATURES = 0,
> +	CXL_FEATURE_ID_GET_FEATURE,
> +	CXL_FEATURE_ID_SET_FEATURE,
> +	CXL_FEATURE_ID_MAX,
> +};
> +
> +/**
> + * struct fwctl_info_cxl - ioctl(FWCTL_INFO) out_device_data
> + * @cmd_mask: Mask indicate which commands are supported based on 'enum feature_cmds'
> + *
> + * Return basic information about the FW interface available.
> + *
> + * nr_commands is number of hardware commands the driver supports. Use
> + * FWCTL_CMD_HW_INFO ioctl to request additional information.
> + */
> +struct fwctl_info_cxl {
> +	__u32 cmd_mask;
> +};
> +#endif

I do not understand the value of this. If the fwctl device shows up at
all for CXL you already know the device must support Get Supported
Features, and Get Feature. Set Feature could be optionally be supported
by the device but Get Supported Features will already tell userspace if
a feature is writable via Set Feature Size in its output payload.

So cmd_mask is vestigial information.

I would just do this for now.

struct fwctl_info_cxl {
	__u32 reserved;
};

Make it zero so that we have place holder to provide future
meta-information about the interface, like an empty flags argument for a
new syscall.
diff mbox series

Patch

diff --git a/drivers/cxl/features.c b/drivers/cxl/features.c
index 5a2b771586d3..cc72e73ae8d6 100644
--- a/drivers/cxl/features.c
+++ b/drivers/cxl/features.c
@@ -9,6 +9,8 @@ 
 #include "cxl.h"
 #include "cxlmem.h"
 
+#define to_cxl_features_state(fwctl) container_of(fwctl, struct cxl_features_state, fwctl)
+
 static int cxlctl_open_uctx(struct fwctl_uctx *uctx)
 {
 	return 0;
@@ -18,10 +20,34 @@  static void cxlctl_close_uctx(struct fwctl_uctx *uctx)
 {
 }
 
+static void set_command_mask(struct cxl_features_state *cfs, u32 *mask)
+{
+	struct cxl_mailbox *cxl_mbox = cfs->features->cxl_mbox;
+
+	*mask = 0;
+	if (test_bit(CXL_FEATURE_ID_GET_SUPPORTED_FEATURES,
+		     cxl_mbox->feature_cmds))
+		*mask |= BIT(CXL_FEATURE_ID_GET_SUPPORTED_FEATURES);
+	if (test_bit(CXL_FEATURE_ID_GET_FEATURE, cxl_mbox->feature_cmds))
+		*mask |= BIT(CXL_FEATURE_ID_GET_FEATURE);
+	if (test_bit(CXL_FEATURE_ID_SET_FEATURE, cxl_mbox->feature_cmds))
+		*mask |= BIT(CXL_FEATURE_ID_SET_FEATURE);
+}
+
 static void *cxlctl_info(struct fwctl_uctx *uctx, size_t *length)
 {
-	/* Place holder */
-	return ERR_PTR(-EOPNOTSUPP);
+	struct fwctl_device *fwctl = uctx->fwctl;
+	struct cxl_features_state *cfs = to_cxl_features_state(fwctl);
+	struct fwctl_info_cxl *info;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	set_command_mask(cfs, &info->cmd_mask);
+	*length = sizeof(*info);
+
+	return info;
 }
 
 static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
diff --git a/include/cxl/features.h b/include/cxl/features.h
index 69697c069e63..e38ee777328d 100644
--- a/include/cxl/features.h
+++ b/include/cxl/features.h
@@ -40,13 +40,6 @@ 
 
 struct cxl_mailbox;
 
-enum feature_cmds {
-	CXL_FEATURE_ID_GET_SUPPORTED_FEATURES = 0,
-	CXL_FEATURE_ID_GET_FEATURE,
-	CXL_FEATURE_ID_SET_FEATURE,
-	CXL_FEATURE_ID_MAX,
-};
-
 struct cxl_features {
 	int id;
 	struct device dev;
diff --git a/include/cxl/mailbox.h b/include/cxl/mailbox.h
index 263fc346aeb1..1157b19175a5 100644
--- a/include/cxl/mailbox.h
+++ b/include/cxl/mailbox.h
@@ -4,6 +4,7 @@ 
 #define __CXL_MBOX_H__
 #include <linux/rcuwait.h>
 #include <cxl/features.h>
+#include <uapi/fwctl/cxl.h>
 #include <uapi/linux/cxl_mem.h>
 
 /**
diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
new file mode 100644
index 000000000000..79b822dbfafd
--- /dev/null
+++ b/include/uapi/fwctl/cxl.h
@@ -0,0 +1,31 @@ 
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2024, Intel Corporation
+ *
+ * These are definitions for the mailbox command interface of CXL subsystem.
+ */
+#ifndef _UAPI_FWCTL_CXL_H_
+#define _UAPI_FWCTL_CXL_H_
+
+#include <linux/types.h>
+
+enum feature_cmds {
+	CXL_FEATURE_ID_GET_SUPPORTED_FEATURES = 0,
+	CXL_FEATURE_ID_GET_FEATURE,
+	CXL_FEATURE_ID_SET_FEATURE,
+	CXL_FEATURE_ID_MAX,
+};
+
+/**
+ * struct fwctl_info_cxl - ioctl(FWCTL_INFO) out_device_data
+ * @cmd_mask: Mask indicate which commands are supported based on 'enum feature_cmds'
+ *
+ * Return basic information about the FW interface available.
+ *
+ * nr_commands is number of hardware commands the driver supports. Use
+ * FWCTL_CMD_HW_INFO ioctl to request additional information.
+ */
+struct fwctl_info_cxl {
+	__u32 cmd_mask;
+};
+#endif