Linux CXL
 help / color / mirror / Atom feed
* [NDCTL PATCH v7 0/4] ndctl: Add support and test for CXL Features support
@ 2025-05-19 20:00 Dave Jiang
  2025-05-19 20:00 ` [NDCTL PATCH v7 1/4] cxl: Add cxl_bus_get_by_provider() Dave Jiang
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Dave Jiang @ 2025-05-19 20:00 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: alison.schofield, Dan Williams

v7:
- Move enumeration of fwctl behind meson option 'fwctl'. (Dan)

v6:
- Rename cxl-features.control.c back to fwctl.c. (Dan)
- Move features behind a meson option. (Dan)
- See individual commits for specific changes from v5.

v5:
- Add documentation for exported symbols. (Alison)
- Create 'struct cxl_fwctl' as object under cxl_memdev. (Dan)
- Make command prep common code. (Alison)
- Rename fwctl.c to cxl-features-control.c. (Alison)
- See individual commits for specific changes from v4.

v4:
- Adjust to kernel changes of input/output structs
- Fixup skip/pass/fail logic
- Added new kernel headers detection and dependency in meson.build

v3:
- Update test to use opcode instead of command id.

v2:
- Drop features device enumeration
- Add discovery of char device under memdev

The series provides support of libcxl enumerating FWCTL character device
under the cxl_memdev device. It discovers the char device major
and minor numbers for the CXL features device in order to allow issuing
of ioctls to the device.

A unit test is added to locate cxl_memdev exported by the cxl_test
kernel module and issue all the supported ioctls to the associated
FWCTL char device to verify that all the ioctl paths are working as expected.

Kernel series: https://lore.kernel.org/linux-cxl/20250207233914.2375110-1-dave.jiang@intel.com/T/#t

Dave Jiang (4):
  cxl: Add cxl_bus_get_by_provider()
  cxl: Enumerate major/minor of FWCTL char device
  ndctl: Add features.h from kernel UAPI
  cxl/test: Add test for cxl features device

 Documentation/cxl/lib/libcxl.txt |  26 ++
 config.h.meson                   |   3 +
 cxl/fwctl/cxl.h                  |  56 ++++
 cxl/fwctl/features.h             | 179 +++++++++++++
 cxl/fwctl/fwctl.h                | 141 ++++++++++
 cxl/lib/libcxl.c                 |  92 +++++++
 cxl/lib/libcxl.sym               |   8 +
 cxl/lib/private.h                |   6 +
 cxl/libcxl.h                     |   7 +
 meson.build                      |   1 +
 meson_options.txt                |   2 +
 test/cxl-features.sh             |  31 +++
 test/fwctl.c                     | 439 +++++++++++++++++++++++++++++++
 test/meson.build                 |  19 ++
 14 files changed, 1010 insertions(+)
 create mode 100644 cxl/fwctl/cxl.h
 create mode 100644 cxl/fwctl/features.h
 create mode 100644 cxl/fwctl/fwctl.h
 create mode 100755 test/cxl-features.sh
 create mode 100644 test/fwctl.c


base-commit: 1850ddcbcbf9eebd343c6e87a2c55f3f5e3930c4
-- 
2.49.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [NDCTL PATCH v7 1/4] cxl: Add cxl_bus_get_by_provider()
  2025-05-19 20:00 [NDCTL PATCH v7 0/4] ndctl: Add support and test for CXL Features support Dave Jiang
@ 2025-05-19 20:00 ` Dave Jiang
  2025-05-19 20:00 ` [NDCTL PATCH v7 2/4] cxl: Enumerate major/minor of FWCTL char device Dave Jiang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Dave Jiang @ 2025-05-19 20:00 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: alison.schofield, Dan Williams

Add helper function cxl_bus_get_by_provider() in order to support unit
test that will utilize the API call.

Acked-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/cxl/lib/libcxl.txt |  2 ++
 cxl/lib/libcxl.c                 | 11 +++++++++++
 cxl/lib/libcxl.sym               |  5 +++++
 cxl/libcxl.h                     |  2 ++
 4 files changed, 20 insertions(+)

diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 40598a08b9f4..25ff406c2920 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -215,6 +215,8 @@ the associated bus object.
 const char *cxl_bus_get_provider(struct cxl_bus *bus);
 const char *cxl_bus_get_devname(struct cxl_bus *bus);
 int cxl_bus_get_id(struct cxl_bus *bus);
+struct cxl_bus *cxl_bus_get_by_provider(struct cxl_ctx *ctx,
+					const char *provider);
 ----
 
 The provider name of a bus is a persistent name that is independent of
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 63aa4ef3acdc..bab7343e8a4a 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -3358,6 +3358,17 @@ CXL_EXPORT struct cxl_ctx *cxl_bus_get_ctx(struct cxl_bus *bus)
 	return cxl_port_get_ctx(&bus->port);
 }
 
+CXL_EXPORT struct cxl_bus *cxl_bus_get_by_provider(struct cxl_ctx *ctx,
+						   const char *provider)
+{
+	struct cxl_bus *bus;
+
+	cxl_bus_foreach(ctx, bus)
+		if (strcmp(provider, cxl_bus_get_provider(bus)) == 0)
+			return bus;
+	return NULL;
+}
+
 CXL_EXPORT void cxl_cmd_unref(struct cxl_cmd *cmd)
 {
 	if (!cmd)
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 763151fbef59..4c9760f377e6 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -287,3 +287,8 @@ global:
 	cxl_memdev_trigger_poison_list;
 	cxl_region_trigger_poison_list;
 } LIBCXL_7;
+
+LIBCXL_9 {
+global:
+	cxl_bus_get_by_provider;
+} LIBECXL_8;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 43c082acd836..7a32b9b65736 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -122,6 +122,8 @@ int cxl_bus_get_id(struct cxl_bus *bus);
 struct cxl_port *cxl_bus_get_port(struct cxl_bus *bus);
 struct cxl_ctx *cxl_bus_get_ctx(struct cxl_bus *bus);
 int cxl_bus_disable_invalidate(struct cxl_bus *bus);
+struct cxl_bus *cxl_bus_get_by_provider(struct cxl_ctx *ctx,
+					const char *provider);
 
 #define cxl_bus_foreach(ctx, bus)                                              \
 	for (bus = cxl_bus_get_first(ctx); bus != NULL;                        \
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [NDCTL PATCH v7 2/4] cxl: Enumerate major/minor of FWCTL char device
  2025-05-19 20:00 [NDCTL PATCH v7 0/4] ndctl: Add support and test for CXL Features support Dave Jiang
  2025-05-19 20:00 ` [NDCTL PATCH v7 1/4] cxl: Add cxl_bus_get_by_provider() Dave Jiang
@ 2025-05-19 20:00 ` Dave Jiang
  2025-05-20 19:11   ` Alison Schofield
  2025-05-19 20:00 ` [NDCTL PATCH v7 3/4] ndctl: Add features.h from kernel UAPI Dave Jiang
  2025-05-19 20:00 ` [NDCTL PATCH v7 4/4] cxl/test: Add test for cxl features device Dave Jiang
  3 siblings, 1 reply; 10+ messages in thread
From: Dave Jiang @ 2025-05-19 20:00 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: alison.schofield, Dan Williams

Add major/minor discovery for the FWCTL character device that is associated
with supprting CXL Features under 'struct cxl_fwctl'. A 'struct cxl_fwctl'
may be installed under cxl_memdev if CXL Features are supported and FWCTL
is enabled. Add libcxl API functions to retrieve the major and minor of the
FWCTL character device.

Acked-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v7:
- Put the enumeration of fwctl behind 'fwctl' meson option
---
 Documentation/cxl/lib/libcxl.txt | 24 ++++++++++
 config.h.meson                   |  3 ++
 cxl/lib/libcxl.c                 | 81 ++++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym               |  3 ++
 cxl/lib/private.h                |  6 +++
 cxl/libcxl.h                     |  5 ++
 meson.build                      |  1 +
 meson_options.txt                |  2 +
 8 files changed, 125 insertions(+)

diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 25ff406c2920..2a512fd9d276 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -42,6 +42,7 @@ struct cxl_memdev *cxl_memdev_get_next(struct cxl_memdev *memdev);
 struct cxl_ctx *cxl_memdev_get_ctx(struct cxl_memdev *memdev);
 const char *cxl_memdev_get_host(struct cxl_memdev *memdev)
 struct cxl_memdev *cxl_endpoint_get_memdev(struct cxl_endpoint *endpoint);
+struct cxl_fwctl *cxl_memdev_get_fwctl(struct cxl_memdev *memdev);
 
 #define cxl_memdev_foreach(ctx, memdev) \
         for (memdev = cxl_memdev_get_first(ctx); \
@@ -59,6 +60,11 @@ specific memdev.
 The host of a memdev is the PCIe Endpoint device that registered its CXL
 capabilities with the Linux CXL core.
 
+A memdev may host a 'struct cxl_fwctl' if CXL Features are supported,
+CONFIG_CXL_FEATURES is enabled, and Firmware Control (FWCTL) is also enabled.
+By default the 'fwctl' option is enabled for CXL CLI. The discovery can
+be disabled by meson build option.
+
 === MEMDEV: Attributes
 ----
 int cxl_memdev_get_id(struct cxl_memdev *memdev);
@@ -185,6 +191,24 @@ device is in use. When CXL_SETPART_NEXTBOOT mode is set, the change
 in partitioning shall become the “next” configuration, to become
 active on the next device reset.
 
+FWCTL
+-----
+The object representing a Firmware Control (FWCTL) device is 'struct cxl_fwctl'.
+Library interfaces related to these devices have the prefix 'cxl_fwctl_'.
+These interfaces are associated with interacting with the FWCTL aspect of the
+memdev. And currently specifically with the CXL FWCTL character device that is
+a child of the memdev.
+
+=== FWCTL: Attributes
+----
+int cxl_fwctl_get_major(struct cxl_memdev *memdev);
+int cxl_fwctl_get_minor(struct cxl_memdev *memdev);
+----
+
+The character device node for Feature (firmware) control can be found by
+default at /dev/fwctl/fwctl%d, or created with a major / minor returned
+from cxl_memdev_get_fwctl_{major,minor}().
+
 BUSES
 -----
 The CXL Memory space is CPU and Device coherent. The address ranges that
diff --git a/config.h.meson b/config.h.meson
index 5441dff9fce7..f75db3e6360f 100644
--- a/config.h.meson
+++ b/config.h.meson
@@ -155,3 +155,6 @@
 #mesondefine DAXCTL_MODPROBE_DATA
 #mesondefine DAXCTL_MODPROBE_INSTALL
 #mesondefine PREFIX
+
+/* cxl fwctl support */
+#mesondefine ENABLE_FWCTL
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index bab7343e8a4a..d0709b90fef6 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -88,6 +88,7 @@ static void free_memdev(struct cxl_memdev *memdev, struct list_head *head)
 	free(memdev->dev_buf);
 	free(memdev->dev_path);
 	free(memdev->host_path);
+	free(memdev->fwctl);
 	free(memdev);
 }
 
@@ -1253,6 +1254,65 @@ static int add_cxl_memdev_fwl(struct cxl_memdev *memdev,
 	return -ENOMEM;
 }
 
+static const char fwctl_prefix[] = "fwctl";
+static int get_feature_chardev(struct cxl_memdev *memdev, const char *base,
+			       char *chardev_path)
+{
+	char *path = memdev->dev_buf;
+	struct dirent *entry;
+	bool found = false;
+	int rc = 0;
+	DIR *dir;
+
+	sprintf(path, "%s/fwctl/", base);
+	dir = opendir(path);
+	if (!dir)
+		return -errno;
+
+	while ((entry = readdir(dir)) != NULL) {
+		if (strncmp(entry->d_name, fwctl_prefix, strlen(fwctl_prefix)) == 0) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!entry || !found) {
+		rc = -ENOENT;
+		goto read_err;
+	}
+
+	sprintf(chardev_path, "/dev/fwctl/%s", entry->d_name);
+
+read_err:
+	closedir(dir);
+	return rc;
+}
+
+static int memdev_add_fwctl(struct cxl_memdev *memdev, const char *cxlmem_base)
+{
+	char *path = memdev->dev_buf;
+	struct cxl_fwctl *fwctl;
+	struct stat st;
+	int rc;
+
+	rc = get_feature_chardev(memdev, cxlmem_base, path);
+	if (rc)
+		return rc;
+
+	if (stat(path, &st) < 0)
+		return -errno;
+
+	fwctl = calloc(1, sizeof(*fwctl));
+	if (!fwctl)
+		return -ENOMEM;
+
+	fwctl->major = major(st.st_rdev);
+	fwctl->minor = minor(st.st_rdev);
+	memdev->fwctl = fwctl;
+
+	return 0;
+}
+
 static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 {
 	const char *devname = devpath_to_devname(cxlmem_base);
@@ -1353,6 +1413,12 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 		goto err_read;
 	memdev->buf_len = strlen(cxlmem_base) + 50;
 
+#ifdef ENABLE_FWCTL
+	/* If this fails, no Features support. */
+	if (memdev_add_fwctl(memdev, cxlmem_base))
+		dbg(ctx, "%s: no Features support.\n", devname);
+#endif
+
 	device_parse(ctx, cxlmem_base, "pmem", memdev, add_cxl_pmem);
 
 	if (add_cxl_memdev_fwl(memdev, cxlmem_base))
@@ -1515,6 +1581,21 @@ CXL_EXPORT int cxl_memdev_get_minor(struct cxl_memdev *memdev)
 	return memdev->minor;
 }
 
+CXL_EXPORT struct cxl_fwctl *cxl_memdev_get_fwctl(struct cxl_memdev *memdev)
+{
+	return memdev->fwctl;
+}
+
+CXL_EXPORT int cxl_fwctl_get_major(struct cxl_fwctl *fwctl)
+{
+	return fwctl->major;
+}
+
+CXL_EXPORT int cxl_fwctl_get_minor(struct cxl_fwctl *fwctl)
+{
+	return fwctl->minor;
+}
+
 CXL_EXPORT unsigned long long cxl_memdev_get_pmem_size(struct cxl_memdev *memdev)
 {
 	return memdev->pmem_size;
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 4c9760f377e6..3ad0cd06e25a 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -291,4 +291,7 @@ global:
 LIBCXL_9 {
 global:
 	cxl_bus_get_by_provider;
+	cxl_memdev_get_fwctl;
+	cxl_fwctl_get_major;
+	cxl_fwctl_get_minor;
 } LIBECXL_8;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index b6cd910e9335..7d5a1bcc14ac 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -34,6 +34,11 @@ enum cxl_fwl_loading {
 	CXL_FWL_LOADING_START,
 };
 
+struct cxl_fwctl {
+	int major;
+	int minor;
+};
+
 struct cxl_endpoint;
 struct cxl_memdev {
 	int id, major, minor;
@@ -56,6 +61,7 @@ struct cxl_memdev {
 	unsigned long long serial;
 	struct cxl_endpoint *endpoint;
 	struct cxl_fw_loader *fwl;
+	struct cxl_fwctl *fwctl;
 };
 
 struct cxl_dport {
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 7a32b9b65736..54d97d7bb501 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -59,6 +59,7 @@ static inline enum cxl_fwl_status cxl_fwl_status_from_ident(char *status)
 }
 
 struct cxl_memdev;
+struct cxl_fwctl;
 struct cxl_memdev *cxl_memdev_get_first(struct cxl_ctx *ctx);
 struct cxl_memdev *cxl_memdev_get_next(struct cxl_memdev *memdev);
 int cxl_memdev_get_id(struct cxl_memdev *memdev);
@@ -69,6 +70,7 @@ const char *cxl_memdev_get_host(struct cxl_memdev *memdev);
 struct cxl_bus *cxl_memdev_get_bus(struct cxl_memdev *memdev);
 int cxl_memdev_get_major(struct cxl_memdev *memdev);
 int cxl_memdev_get_minor(struct cxl_memdev *memdev);
+struct cxl_fwctl *cxl_memdev_get_fwctl(struct cxl_memdev *memdev);
 struct cxl_ctx *cxl_memdev_get_ctx(struct cxl_memdev *memdev);
 unsigned long long cxl_memdev_get_pmem_size(struct cxl_memdev *memdev);
 unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev);
@@ -81,6 +83,9 @@ int cxl_memdev_update_fw(struct cxl_memdev *memdev, const char *fw_path);
 int cxl_memdev_cancel_fw_update(struct cxl_memdev *memdev);
 int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev, int timeout_ms);
 
+int cxl_fwctl_get_major(struct cxl_fwctl *fwctl);
+int cxl_fwctl_get_minor(struct cxl_fwctl *fwctl);
+
 /* ABI spelling mistakes are forever */
 static inline const char *cxl_memdev_get_firmware_version(
 		struct cxl_memdev *memdev)
diff --git a/meson.build b/meson.build
index 3171ef1024ea..7bc40c7df12a 100644
--- a/meson.build
+++ b/meson.build
@@ -237,6 +237,7 @@ conf.set('ENABLE_DESTRUCTIVE', get_option('destructive').enabled())
 conf.set('ENABLE_LOGGING', get_option('logging').enabled())
 conf.set('ENABLE_DEBUG', get_option('dbg').enabled())
 conf.set('ENABLE_LIBTRACEFS', get_option('libtracefs').enabled())
+conf.set('ENABLE_FWCTL', get_option('fwctl').enabled())
 
 typeof_code = '''
   void func() {
diff --git a/meson_options.txt b/meson_options.txt
index 5c41b1abe1f1..acc19be4ff0a 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -28,3 +28,5 @@ option('iniparserdir', type : 'string',
        description : 'Path containing the iniparser header files')
 option('modprobedatadir', type : 'string',
        description : '''${sysconfdir}/modprobe.d/''')
+option('fwctl', type : 'feature', value : 'enabled',
+  description : 'enable firmware control')
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [NDCTL PATCH v7 3/4] ndctl: Add features.h from kernel UAPI
  2025-05-19 20:00 [NDCTL PATCH v7 0/4] ndctl: Add support and test for CXL Features support Dave Jiang
  2025-05-19 20:00 ` [NDCTL PATCH v7 1/4] cxl: Add cxl_bus_get_by_provider() Dave Jiang
  2025-05-19 20:00 ` [NDCTL PATCH v7 2/4] cxl: Enumerate major/minor of FWCTL char device Dave Jiang
@ 2025-05-19 20:00 ` Dave Jiang
  2025-05-20 19:08   ` Alison Schofield
  2025-05-19 20:00 ` [NDCTL PATCH v7 4/4] cxl/test: Add test for cxl features device Dave Jiang
  3 siblings, 1 reply; 10+ messages in thread
From: Dave Jiang @ 2025-05-19 20:00 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: alison.schofield

Pull in the kernel UAPI header files from CXL Features support and FWCTL.
This is needed to support building the CXL Features unit test.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/fwctl/cxl.h      |  56 ++++++++++++++
 cxl/fwctl/features.h | 179 +++++++++++++++++++++++++++++++++++++++++++
 cxl/fwctl/fwctl.h    | 141 ++++++++++++++++++++++++++++++++++
 3 files changed, 376 insertions(+)
 create mode 100644 cxl/fwctl/cxl.h
 create mode 100644 cxl/fwctl/features.h
 create mode 100644 cxl/fwctl/fwctl.h

diff --git a/cxl/fwctl/cxl.h b/cxl/fwctl/cxl.h
new file mode 100644
index 000000000000..43f522f0cdcd
--- /dev/null
+++ b/cxl/fwctl/cxl.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2024-2025 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>
+#include <linux/stddef.h>
+#include <cxl/features.h>
+
+/**
+ * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input for CXL
+ * @opcode: CXL mailbox command opcode
+ * @flags: Flags for the command (input).
+ * @op_size: Size of input payload.
+ * @reserved1: Reserved. Must be 0s.
+ * @get_sup_feats_in: Get Supported Features input
+ * @get_feat_in: Get Feature input
+ * @set_feat_in: Set Feature input
+ */
+struct fwctl_rpc_cxl {
+	__struct_group(fwctl_rpc_cxl_hdr, hdr, /* no attrs */,
+		__u32 opcode;
+		__u32 flags;
+		__u32 op_size;
+		__u32 reserved1;
+	);
+	union {
+		struct cxl_mbox_get_sup_feats_in get_sup_feats_in;
+		struct cxl_mbox_get_feat_in get_feat_in;
+		struct cxl_mbox_set_feat_in set_feat_in;
+	};
+};
+
+/**
+ * struct fwctl_rpc_cxl_out - ioctl(FWCTL_RPC) output for CXL
+ * @size: Size of the output payload
+ * @retval: Return value from device
+ * @get_sup_feats_out: Get Supported Features output
+ * @payload: raw byte stream of payload
+ */
+struct fwctl_rpc_cxl_out {
+	__struct_group(fwctl_rpc_cxl_out_hdr, hdr, /* no attrs */,
+		__u32 size;
+		__u32 retval;
+	);
+	union {
+		struct cxl_mbox_get_sup_feats_out get_sup_feats_out;
+		__DECLARE_FLEX_ARRAY(__u8, payload);
+	};
+};
+
+#endif
diff --git a/cxl/fwctl/features.h b/cxl/fwctl/features.h
new file mode 100644
index 000000000000..490606d7694b
--- /dev/null
+++ b/cxl/fwctl/features.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2024,2025, Intel Corporation
+ *
+ * These are definitions for the mailbox command interface of CXL subsystem.
+ */
+#ifndef _UAPI_CXL_FEATURES_H_
+#define _UAPI_CXL_FEATURES_H_
+
+#include <linux/types.h>
+
+typedef unsigned char __uapi_uuid_t[16];
+
+#ifdef __KERNEL__
+#include <linux/uuid.h>
+/*
+ * Note, __uapi_uuid_t is 1-byte aligned on modern compilers and 4-byte
+ * aligned on others. Ensure that __uapi_uuid_t in a struct is placed at
+ * a 4-byte aligned offset, or the structure is packed, to ensure
+ * consistent padding.
+ */
+static_assert(sizeof(__uapi_uuid_t) == sizeof(uuid_t));
+#define __uapi_uuid_t uuid_t
+#endif
+
+/*
+ * struct cxl_mbox_get_sup_feats_in - Get Supported Features input
+ *
+ * @count: bytes of Feature data to return in output
+ * @start_idx: index of first requested Supported Feature Entry, 0 based.
+ * @reserved: reserved field, must be 0s.
+ *
+ * Get Supported Features (0x500h) CXL r3.2 8.2.9.6.1 command.
+ * Input block for Get support Feature
+ */
+struct cxl_mbox_get_sup_feats_in {
+	__le32 count;
+	__le16 start_idx;
+	__u8 reserved[2];
+} __attribute__ ((__packed__));
+
+/* CXL spec r3.2 Table 8-87 command effects */
+#define CXL_CMD_CONFIG_CHANGE_COLD_RESET	BIT(0)
+#define CXL_CMD_CONFIG_CHANGE_IMMEDIATE		BIT(1)
+#define CXL_CMD_DATA_CHANGE_IMMEDIATE		BIT(2)
+#define CXL_CMD_POLICY_CHANGE_IMMEDIATE		BIT(3)
+#define CXL_CMD_LOG_CHANGE_IMMEDIATE		BIT(4)
+#define CXL_CMD_SECURITY_STATE_CHANGE		BIT(5)
+#define CXL_CMD_BACKGROUND			BIT(6)
+#define CXL_CMD_BGCMD_ABORT_SUPPORTED		BIT(7)
+#define CXL_CMD_EFFECTS_VALID			BIT(9)
+#define CXL_CMD_CONFIG_CHANGE_CONV_RESET	BIT(10)
+#define CXL_CMD_CONFIG_CHANGE_CXL_RESET		BIT(11)
+#define CXL_CMD_EFFECTS_RESERVED		GENMASK(15, 12)
+
+/*
+ * struct cxl_feat_entry - Supported Feature Entry
+ * @uuid: UUID of the Feature
+ * @id: id to identify the feature. 0 based
+ * @get_feat_size: max bytes required for Get Feature command for this Feature
+ * @set_feat_size: max bytes required for Set Feature command for this Feature
+ * @flags: attribute flags
+ * @get_feat_ver: Get Feature version
+ * @set_feat_ver: Set Feature version
+ * @effects: Set Feature command effects
+ * @reserved: reserved, must be 0
+ *
+ * CXL spec r3.2 Table 8-109
+ * Get Supported Features Supported Feature Entry
+ */
+struct cxl_feat_entry {
+	__uapi_uuid_t uuid;
+	__le16 id;
+	__le16 get_feat_size;
+	__le16 set_feat_size;
+	__le32 flags;
+	__u8 get_feat_ver;
+	__u8 set_feat_ver;
+	__le16 effects;
+	__u8 reserved[18];
+} __attribute__ ((__packed__));
+
+/* @flags field for 'struct cxl_feat_entry' */
+#define CXL_FEATURE_F_CHANGEABLE		BIT(0)
+#define CXL_FEATURE_F_PERSIST_FW_UPDATE		BIT(4)
+#define CXL_FEATURE_F_DEFAULT_SEL		BIT(5)
+#define CXL_FEATURE_F_SAVED_SEL			BIT(6)
+
+/*
+ * struct cxl_mbox_get_sup_feats_out - Get Supported Features output
+ * @num_entries: number of Supported Feature Entries returned
+ * @supported_feats: number of supported Features
+ * @reserved: reserved, must be 0s.
+ * @ents: Supported Feature Entries array
+ *
+ * CXL spec r3.2 Table 8-108
+ * Get supported Features Output Payload
+ */
+struct cxl_mbox_get_sup_feats_out {
+	__struct_group(cxl_mbox_get_sup_feats_out_hdr, hdr, /* no attrs */,
+		__le16 num_entries;
+		__le16 supported_feats;
+		__u8 reserved[4];
+	);
+	struct cxl_feat_entry ents[] __counted_by_le(num_entries);
+} __attribute__ ((__packed__));
+
+/*
+ * Get Feature CXL spec r3.2 Spec 8.2.9.6.2
+ */
+
+/*
+ * struct cxl_mbox_get_feat_in - Get Feature input
+ * @uuid: UUID for Feature
+ * @offset: offset of the first byte in Feature data for output payload
+ * @count: count in bytes of Feature data returned
+ * @selection: 0 current value, 1 default value, 2 saved value
+ *
+ * CXL spec r3.2 section 8.2.9.6.2 Table 8-99
+ */
+struct cxl_mbox_get_feat_in {
+	__uapi_uuid_t uuid;
+	__le16 offset;
+	__le16 count;
+	__u8 selection;
+} __attribute__ ((__packed__));
+
+/*
+ * enum cxl_get_feat_selection - selection field of Get Feature input
+ */
+enum cxl_get_feat_selection {
+	CXL_GET_FEAT_SEL_CURRENT_VALUE,
+	CXL_GET_FEAT_SEL_DEFAULT_VALUE,
+	CXL_GET_FEAT_SEL_SAVED_VALUE,
+	CXL_GET_FEAT_SEL_MAX
+};
+
+/*
+ * Set Feature CXL spec r3.2  8.2.9.6.3
+ */
+
+/*
+ * struct cxl_mbox_set_feat_in - Set Features input
+ * @uuid: UUID for Feature
+ * @flags: set feature flags
+ * @offset: byte offset of Feature data to update
+ * @version: Feature version of the data in Feature Data
+ * @rsvd: reserved, must be 0s.
+ * @feat_data: raw byte stream of Features data to update
+ *
+ * CXL spec r3.2 section 8.2.9.6.3 Table 8-101
+ */
+struct cxl_mbox_set_feat_in {
+	__struct_group(cxl_mbox_set_feat_hdr, hdr, /* no attrs */,
+		__uapi_uuid_t uuid;
+		__le32 flags;
+		__le16 offset;
+		__u8 version;
+		__u8 rsvd[9];
+	);
+	__u8 feat_data[];
+}  __packed;
+
+/*
+ * enum cxl_set_feat_flag_data_transfer - Set Feature flags field
+ */
+enum cxl_set_feat_flag_data_transfer {
+	CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER = 0,
+	CXL_SET_FEAT_FLAG_INITIATE_DATA_TRANSFER,
+	CXL_SET_FEAT_FLAG_CONTINUE_DATA_TRANSFER,
+	CXL_SET_FEAT_FLAG_FINISH_DATA_TRANSFER,
+	CXL_SET_FEAT_FLAG_ABORT_DATA_TRANSFER,
+	CXL_SET_FEAT_FLAG_DATA_TRANSFER_MAX
+};
+
+#define CXL_SET_FEAT_FLAG_DATA_TRANSFER_MASK		GENMASK(2, 0)
+#define CXL_SET_FEAT_FLAG_DATA_SAVED_ACROSS_RESET	BIT(3)
+
+#endif
diff --git a/cxl/fwctl/fwctl.h b/cxl/fwctl/fwctl.h
new file mode 100644
index 000000000000..716ac0eee42d
--- /dev/null
+++ b/cxl/fwctl/fwctl.h
@@ -0,0 +1,141 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* Copyright (c) 2024-2025, NVIDIA CORPORATION & AFFILIATES.
+ */
+#ifndef _UAPI_FWCTL_H
+#define _UAPI_FWCTL_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+#define FWCTL_TYPE 0x9A
+
+/**
+ * DOC: General ioctl format
+ *
+ * The ioctl interface follows a general format to allow for extensibility. Each
+ * ioctl is passed a structure pointer as the argument providing the size of
+ * the structure in the first u32. The kernel checks that any structure space
+ * beyond what it understands is 0. This allows userspace to use the backward
+ * compatible portion while consistently using the newer, larger, structures.
+ *
+ * ioctls use a standard meaning for common errnos:
+ *
+ *  - ENOTTY: The IOCTL number itself is not supported at all
+ *  - E2BIG: The IOCTL number is supported, but the provided structure has
+ *    non-zero in a part the kernel does not understand.
+ *  - EOPNOTSUPP: The IOCTL number is supported, and the structure is
+ *    understood, however a known field has a value the kernel does not
+ *    understand or support.
+ *  - EINVAL: Everything about the IOCTL was understood, but a field is not
+ *    correct.
+ *  - ENOMEM: Out of memory.
+ *  - ENODEV: The underlying device has been hot-unplugged and the FD is
+ *            orphaned.
+ *
+ * As well as additional errnos, within specific ioctls.
+ */
+enum {
+	FWCTL_CMD_BASE = 0,
+	FWCTL_CMD_INFO = 0,
+	FWCTL_CMD_RPC = 1,
+};
+
+enum fwctl_device_type {
+	FWCTL_DEVICE_TYPE_ERROR = 0,
+	FWCTL_DEVICE_TYPE_MLX5 = 1,
+	FWCTL_DEVICE_TYPE_CXL = 2,
+	FWCTL_DEVICE_TYPE_PDS = 4,
+};
+
+/**
+ * struct fwctl_info - ioctl(FWCTL_INFO)
+ * @size: sizeof(struct fwctl_info)
+ * @flags: Must be 0
+ * @out_device_type: Returns the type of the device from enum fwctl_device_type
+ * @device_data_len: On input the length of the out_device_data memory. On
+ *	output the size of the kernel's device_data which may be larger or
+ *	smaller than the input. Maybe 0 on input.
+ * @out_device_data: Pointer to a memory of device_data_len bytes. Kernel will
+ *	fill the entire memory, zeroing as required.
+ *
+ * Returns basic information about this fwctl instance, particularly what driver
+ * is being used to define the device_data format.
+ */
+struct fwctl_info {
+	__u32 size;
+	__u32 flags;
+	__u32 out_device_type;
+	__u32 device_data_len;
+	__aligned_u64 out_device_data;
+};
+#define FWCTL_INFO _IO(FWCTL_TYPE, FWCTL_CMD_INFO)
+
+/**
+ * enum fwctl_rpc_scope - Scope of access for the RPC
+ *
+ * Refer to fwctl.rst for a more detailed discussion of these scopes.
+ */
+enum fwctl_rpc_scope {
+	/**
+	 * @FWCTL_RPC_CONFIGURATION: Device configuration access scope
+	 *
+	 * Read/write access to device configuration. When configuration
+	 * is written to the device it remains in a fully supported state.
+	 */
+	FWCTL_RPC_CONFIGURATION = 0,
+	/**
+	 * @FWCTL_RPC_DEBUG_READ_ONLY: Read only access to debug information
+	 *
+	 * Readable debug information. Debug information is compatible with
+	 * kernel lockdown, and does not disclose any sensitive information. For
+	 * instance exposing any encryption secrets from this information is
+	 * forbidden.
+	 */
+	FWCTL_RPC_DEBUG_READ_ONLY = 1,
+	/**
+	 * @FWCTL_RPC_DEBUG_WRITE: Writable access to lockdown compatible debug information
+	 *
+	 * Allows write access to data in the device which may leave a fully
+	 * supported state. This is intended to permit intensive and possibly
+	 * invasive debugging. This scope will taint the kernel.
+	 */
+	FWCTL_RPC_DEBUG_WRITE = 2,
+	/**
+	 * @FWCTL_RPC_DEBUG_WRITE_FULL: Write access to all debug information
+	 *
+	 * Allows read/write access to everything. Requires CAP_SYS_RAW_IO, so
+	 * it is not required to follow lockdown principals. If in doubt
+	 * debugging should be placed in this scope. This scope will taint the
+	 * kernel.
+	 */
+	FWCTL_RPC_DEBUG_WRITE_FULL = 3,
+};
+
+/**
+ * struct fwctl_rpc - ioctl(FWCTL_RPC)
+ * @size: sizeof(struct fwctl_rpc)
+ * @scope: One of enum fwctl_rpc_scope, required scope for the RPC
+ * @in_len: Length of the in memory
+ * @out_len: Length of the out memory
+ * @in: Request message in device specific format
+ * @out: Response message in device specific format
+ *
+ * Deliver a Remote Procedure Call to the device FW and return the response. The
+ * call's parameters and return are marshaled into linear buffers of memory. Any
+ * errno indicates that delivery of the RPC to the device failed. Return status
+ * originating in the device during a successful delivery must be encoded into
+ * out.
+ *
+ * The format of the buffers matches the out_device_type from FWCTL_INFO.
+ */
+struct fwctl_rpc {
+	__u32 size;
+	__u32 scope;
+	__u32 in_len;
+	__u32 out_len;
+	__aligned_u64 in;
+	__aligned_u64 out;
+};
+#define FWCTL_RPC _IO(FWCTL_TYPE, FWCTL_CMD_RPC)
+
+#endif
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [NDCTL PATCH v7 4/4] cxl/test: Add test for cxl features device
  2025-05-19 20:00 [NDCTL PATCH v7 0/4] ndctl: Add support and test for CXL Features support Dave Jiang
                   ` (2 preceding siblings ...)
  2025-05-19 20:00 ` [NDCTL PATCH v7 3/4] ndctl: Add features.h from kernel UAPI Dave Jiang
@ 2025-05-19 20:00 ` Dave Jiang
  2025-05-20 19:25   ` Alison Schofield
  3 siblings, 1 reply; 10+ messages in thread
From: Dave Jiang @ 2025-05-19 20:00 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: alison.schofield, Dan Williams

Add a unit test to verify the features ioctl commands. Test support added
for locating a features device, retrieve and verify the supported features
commands, retrieve specific feature command data, retrieve test feature
data, and write and verify test feature data.

Acked-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/fwctl/cxl.h      |   2 +-
 test/cxl-features.sh |  31 +++
 test/fwctl.c         | 439 +++++++++++++++++++++++++++++++++++++++++++
 test/meson.build     |  19 ++
 4 files changed, 490 insertions(+), 1 deletion(-)
 create mode 100755 test/cxl-features.sh
 create mode 100644 test/fwctl.c

diff --git a/cxl/fwctl/cxl.h b/cxl/fwctl/cxl.h
index 43f522f0cdcd..c560b2a1181d 100644
--- a/cxl/fwctl/cxl.h
+++ b/cxl/fwctl/cxl.h
@@ -9,7 +9,7 @@
 
 #include <linux/types.h>
 #include <linux/stddef.h>
-#include <cxl/features.h>
+#include "features.h"
 
 /**
  * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input for CXL
diff --git a/test/cxl-features.sh b/test/cxl-features.sh
new file mode 100755
index 000000000000..3498fa08be53
--- /dev/null
+++ b/test/cxl-features.sh
@@ -0,0 +1,31 @@
+#!/bin/bash -Ex
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2025 Intel Corporation. All rights reserved.
+
+rc=77
+# 237 is -ENODEV
+ERR_NODEV=237
+
+. $(dirname $0)/common
+FEATURES="$TEST_PATH"/fwctl
+
+trap 'err $LINENO' ERR
+
+modprobe cxl_test
+
+test -x "$FEATURES" || do_skip "no CXL Features Contrl"
+# disable trap
+trap - $(compgen -A signal)
+"$FEATURES"
+rc=$?
+
+echo "error: $rc"
+if [ "$rc" -eq "$ERR_NODEV" ]; then
+	do_skip "no CXL FWCTL char dev"
+elif [ "$rc" -ne 0 ]; then
+	echo "fail: $LINENO" && exit 1
+fi
+
+trap 'err $LINENO' ERR
+
+_cxl_cleanup
diff --git a/test/fwctl.c b/test/fwctl.c
new file mode 100644
index 000000000000..7a780e718872
--- /dev/null
+++ b/test/fwctl.c
@@ -0,0 +1,439 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2024-2025 Intel Corporation. All rights reserved.
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <endian.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <syslog.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <cxl/libcxl.h>
+#include <linux/uuid.h>
+#include <uuid/uuid.h>
+#include <util/bitmap.h>
+#include <cxl/fwctl/features.h>
+#include <cxl/fwctl/fwctl.h>
+#include <cxl/fwctl/cxl.h>
+
+static const char provider[] = "cxl_test";
+
+UUID_DEFINE(test_uuid,
+	    0xff, 0xff, 0xff, 0xff,
+	    0xff, 0xff,
+	    0xff, 0xff,
+	    0xff, 0xff,
+	    0xff, 0xff, 0xff, 0xff, 0xff, 0xff
+);
+
+#define CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES	0x0500
+#define CXL_MBOX_OPCODE_GET_FEATURE		0x0501
+#define CXL_MBOX_OPCODE_SET_FEATURE		0x0502
+
+#define GET_FEAT_SIZE	4
+#define SET_FEAT_SIZE	4
+#define EFFECTS_MASK	(BIT(0) | BIT(9))
+
+#define MAX_TEST_FEATURES	1
+#define DEFAULT_TEST_DATA	0xdeadbeef
+#define DEFAULT_TEST_DATA2	0xabcdabcd
+
+struct test_feature {
+	uuid_t uuid;
+	size_t get_size;
+	size_t set_size;
+};
+
+static int send_command(int fd, struct fwctl_rpc *rpc, struct fwctl_rpc_cxl_out *out)
+{
+	if (ioctl(fd, FWCTL_RPC, rpc) == -1) {
+		fprintf(stderr, "RPC ioctl error: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	if (out->retval) {
+		fprintf(stderr, "operation returned failure: %d\n", out->retval);
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
+static int get_scope(u16 opcode)
+{
+	switch (opcode) {
+	case CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES:
+	case CXL_MBOX_OPCODE_GET_FEATURE:
+		return FWCTL_RPC_CONFIGURATION;
+	case CXL_MBOX_OPCODE_SET_FEATURE:
+		return FWCTL_RPC_DEBUG_WRITE_FULL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static size_t hw_op_size(u16 opcode)
+{
+	switch (opcode) {
+	case CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES:
+		return sizeof(struct cxl_mbox_get_sup_feats_in);
+	case CXL_MBOX_OPCODE_GET_FEATURE:
+		return sizeof(struct cxl_mbox_get_feat_in);
+	case CXL_MBOX_OPCODE_SET_FEATURE:
+		return sizeof(struct cxl_mbox_set_feat_in) + sizeof(u32);
+	default:
+		return SIZE_MAX;
+	}
+}
+
+static void free_rpc(struct fwctl_rpc *rpc)
+{
+	void *in, *out;
+
+	in = (void *)rpc->in;
+	out = (void *)rpc->out;
+	free(in);
+	free(out);
+	free(rpc);
+}
+
+static void *zmalloc_aligned(size_t align, size_t size)
+{
+	void *ptr;
+	int rc;
+
+	rc = posix_memalign((void **)&ptr, align, size);
+	if (rc)
+		return NULL;
+	memset(ptr, 0, size);
+
+	return ptr;
+}
+
+static struct fwctl_rpc *get_prepped_command(size_t in_size, size_t out_size,
+					     u16 opcode)
+{
+	struct fwctl_rpc_cxl_out *out;
+	struct fwctl_rpc_cxl *in;
+	struct fwctl_rpc *rpc;
+	size_t op_size;
+	int scope;
+
+	rpc = zmalloc_aligned(16, sizeof(*rpc));
+	if (!rpc)
+		return NULL;
+
+	in = zmalloc_aligned(16, in_size);
+	if (!in)
+		goto free_rpc;
+
+	out = zmalloc_aligned(16, out_size);
+	if (!out)
+		goto free_in;
+
+	in->opcode = opcode;
+
+	op_size = hw_op_size(opcode);
+	if (op_size == SIZE_MAX)
+		goto free_in;
+
+	in->op_size = op_size;
+
+	rpc->size = sizeof(*rpc);
+	scope = get_scope(opcode);
+	if (scope < 0)
+		goto free_all;
+
+	rpc->scope = scope;
+
+	rpc->in_len = in_size;
+	rpc->out_len = out_size;
+	rpc->in = (uint64_t)(uint64_t *)in;
+	rpc->out = (uint64_t)(uint64_t *)out;
+
+	return rpc;
+
+free_all:
+	free(out);
+free_in:
+	free(in);
+free_rpc:
+	free(rpc);
+	return NULL;
+}
+
+static int cxl_fwctl_rpc_get_test_feature(int fd, struct test_feature *feat_ctx,
+					  const uint32_t expected_data)
+{
+	struct cxl_mbox_get_feat_in *feat_in;
+	struct fwctl_rpc_cxl_out *out;
+	size_t out_size, in_size;
+	struct fwctl_rpc_cxl *in;
+	struct fwctl_rpc *rpc;
+	uint32_t val;
+	void *data;
+	int rc;
+
+	in_size = sizeof(*in) + sizeof(*feat_in);
+	out_size = sizeof(*out) + feat_ctx->get_size;
+
+	rpc = get_prepped_command(in_size, out_size,
+				  CXL_MBOX_OPCODE_GET_FEATURE);
+	if (!rpc)
+		return -ENXIO;
+
+	in = (struct fwctl_rpc_cxl *)rpc->in;
+	out = (struct fwctl_rpc_cxl_out *)rpc->out;
+
+	feat_in = &in->get_feat_in;
+	uuid_copy(feat_in->uuid, feat_ctx->uuid);
+	feat_in->count = feat_ctx->get_size;
+
+	rc = send_command(fd, rpc, out);
+	if (rc)
+		goto out;
+
+	data = out->payload;
+	val = le32toh(*(__le32 *)data);
+	if (memcmp(&val, &expected_data, sizeof(val)) != 0) {
+		rc = -ENXIO;
+		goto out;
+	}
+
+out:
+	free_rpc(rpc);
+	return rc;
+}
+
+static int cxl_fwctl_rpc_set_test_feature(int fd, struct test_feature *feat_ctx)
+{
+	struct cxl_mbox_set_feat_in *feat_in;
+	struct fwctl_rpc_cxl_out *out;
+	size_t in_size, out_size;
+	struct fwctl_rpc_cxl *in;
+	struct fwctl_rpc *rpc;
+	uint32_t val;
+	void *data;
+	int rc;
+
+	in_size = sizeof(*in) + sizeof(*feat_in) + sizeof(val);
+	out_size = sizeof(*out) + sizeof(val);
+	rpc = get_prepped_command(in_size, out_size,
+				  CXL_MBOX_OPCODE_SET_FEATURE);
+	if (!rpc)
+		return -ENXIO;
+
+	in = (struct fwctl_rpc_cxl *)rpc->in;
+	out = (struct fwctl_rpc_cxl_out *)rpc->out;
+	feat_in = &in->set_feat_in;
+	uuid_copy(feat_in->uuid, feat_ctx->uuid);
+	data = feat_in->feat_data;
+	val = DEFAULT_TEST_DATA2;
+	*(uint32_t *)data = htole32(val);
+	feat_in->flags = CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER;
+
+	rc = send_command(fd, rpc, out);
+	if (rc)
+		goto out;
+
+	rc = cxl_fwctl_rpc_get_test_feature(fd, feat_ctx, DEFAULT_TEST_DATA2);
+	if (rc) {
+		fprintf(stderr, "Failed ioctl to get feature verify: %d\n", rc);
+		goto out;
+	}
+
+out:
+	free_rpc(rpc);
+	return rc;
+}
+
+static int cxl_fwctl_rpc_get_supported_features(int fd, struct test_feature *feat_ctx)
+{
+	struct cxl_mbox_get_sup_feats_out *feat_out;
+	struct cxl_mbox_get_sup_feats_in *feat_in;
+	struct fwctl_rpc_cxl_out *out;
+	struct cxl_feat_entry *entry;
+	size_t out_size, in_size;
+	struct fwctl_rpc_cxl *in;
+	struct fwctl_rpc *rpc;
+	int feats, rc;
+
+	in_size = sizeof(*in) + sizeof(*feat_in);
+	out_size = sizeof(*out) + sizeof(*feat_out);
+	/* First query, to get number of features w/o per feature data */
+	rpc = get_prepped_command(in_size, out_size,
+				  CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES);
+	if (!rpc)
+		return -ENXIO;
+
+	/* No need to fill in feat_in first go as we are passing in all 0's */
+
+	out = (struct fwctl_rpc_cxl_out *)rpc->out;
+	rc = send_command(fd, rpc, out);
+	if (rc)
+		goto out;
+
+	feat_out = &out->get_sup_feats_out;
+	feats = le16toh(feat_out->supported_feats);
+	if (feats != MAX_TEST_FEATURES) {
+		fprintf(stderr, "Test device has greater than %d test features.\n",
+			MAX_TEST_FEATURES);
+		rc = -ENXIO;
+		goto out;
+	}
+
+	free_rpc(rpc);
+
+	/* Going second round to retrieve each feature details */
+	in_size = sizeof(*in) + sizeof(*feat_in);
+	out_size = sizeof(*out) + sizeof(*feat_out);
+	out_size += feats * sizeof(*entry);
+	rpc = get_prepped_command(in_size, out_size,
+				  CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES);
+	if (!rpc)
+		return -ENXIO;
+
+	in = (struct fwctl_rpc_cxl *)rpc->in;
+	out = (struct fwctl_rpc_cxl_out *)rpc->out;
+	feat_in = &in->get_sup_feats_in;
+	feat_in->count = htole32(feats * sizeof(*entry));
+
+	rc = send_command(fd, rpc, out);
+	if (rc)
+		goto out;
+
+	feat_out = &out->get_sup_feats_out;
+	feats = le16toh(feat_out->supported_feats);
+	if (feats != MAX_TEST_FEATURES) {
+		fprintf(stderr, "Test device has greater than %u test features.\n",
+			MAX_TEST_FEATURES);
+		rc = -ENXIO;
+		goto out;
+	}
+
+	if (le16toh(feat_out->num_entries) != MAX_TEST_FEATURES) {
+		fprintf(stderr, "Test device did not return expected entries. %u\n",
+			le16toh(feat_out->num_entries));
+		rc = -ENXIO;
+		goto out;
+	}
+
+	entry = &feat_out->ents[0];
+	if (uuid_compare(test_uuid, entry->uuid) != 0) {
+		fprintf(stderr, "Test device did not export expected test feature.\n");
+		rc = -ENXIO;
+		goto out;
+	}
+
+	if (le16toh(entry->get_feat_size) != GET_FEAT_SIZE ||
+	    le16toh(entry->set_feat_size) != SET_FEAT_SIZE) {
+		fprintf(stderr, "Test device feature in/out size incorrect.\n");
+		rc = -ENXIO;
+		goto out;
+	}
+
+	if (le16toh(entry->effects) != EFFECTS_MASK) {
+		fprintf(stderr, "Test device set effects incorrect\n");
+		rc = -ENXIO;
+		goto out;
+	}
+
+	uuid_copy(feat_ctx->uuid, entry->uuid);
+	feat_ctx->get_size = le16toh(entry->get_feat_size);
+	feat_ctx->set_size = le16toh(entry->set_feat_size);
+
+out:
+	free_rpc(rpc);
+	return rc;
+}
+
+static int test_fwctl_features(struct cxl_memdev *memdev)
+{
+	struct test_feature feat_ctx;
+	unsigned int major, minor;
+	struct cxl_fwctl *fwctl;
+	int fd, rc;
+	char path[256];
+
+	fwctl = cxl_memdev_get_fwctl(memdev);
+	if (!fwctl)
+		return -ENODEV;
+
+	major = cxl_fwctl_get_major(fwctl);
+	minor = cxl_fwctl_get_minor(fwctl);
+
+	if (!major && !minor)
+		return -ENODEV;
+
+	sprintf(path, "/dev/char/%d:%d", major, minor);
+
+	fd = open(path, O_RDONLY, 0644);
+	if (fd < 0) {
+		fprintf(stderr, "Failed to open: %d\n", -errno);
+		return -errno;
+	}
+
+	rc = cxl_fwctl_rpc_get_supported_features(fd, &feat_ctx);
+	if (rc) {
+		fprintf(stderr, "Failed ioctl to get supported features: %d\n", rc);
+		goto out;
+	}
+
+	rc = cxl_fwctl_rpc_get_test_feature(fd, &feat_ctx, DEFAULT_TEST_DATA);
+	if (rc) {
+		fprintf(stderr, "Failed ioctl to get feature: %d\n", rc);
+		goto out;
+	}
+
+	rc = cxl_fwctl_rpc_set_test_feature(fd, &feat_ctx);
+	if (rc) {
+		fprintf(stderr, "Failed ioctl to set feature: %d\n", rc);
+		goto out;
+	}
+
+out:
+	close(fd);
+	return rc;
+}
+
+static int test_fwctl(struct cxl_ctx *ctx, struct cxl_bus *bus)
+{
+	struct cxl_memdev *memdev;
+
+	cxl_memdev_foreach(ctx, memdev) {
+		if (cxl_memdev_get_bus(memdev) != bus)
+			continue;
+		return test_fwctl_features(memdev);
+	}
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	struct cxl_ctx *ctx;
+	struct cxl_bus *bus;
+	int rc;
+
+	rc = cxl_new(&ctx);
+	if (rc < 0)
+		return rc;
+
+	cxl_set_log_priority(ctx, LOG_DEBUG);
+
+	bus = cxl_bus_get_by_provider(ctx, provider);
+	if (!bus) {
+		fprintf(stderr, "%s: unable to find bus (%s)\n",
+			argv[0], provider);
+		rc = -EINVAL;
+		goto out;
+	}
+
+	rc = test_fwctl(ctx, bus);
+
+out:
+	cxl_unref(ctx);
+	return rc;
+}
diff --git a/test/meson.build b/test/meson.build
index 2fd7df5211dd..93b1d78671ef 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -17,6 +17,13 @@ ndctl_deps = libndctl_deps + [
   versiondep,
 ]
 
+libcxl_deps = [
+  cxl_dep,
+  ndctl_dep,
+  uuid,
+  kmod,
+]
+
 libndctl = executable('libndctl', testcore + [ 'libndctl.c'],
   dependencies : libndctl_deps,
   include_directories : root_inc,
@@ -235,6 +242,18 @@ if get_option('keyutils').enabled()
   ]
 endif
 
+uuid_dep = dependency('uuid', required: false)
+if get_option('fwctl').enabled() and uuid_dep.found()
+  fwctl = executable('fwctl', 'fwctl.c',
+    dependencies : libcxl_deps,
+    include_directories : root_inc,
+  )
+  cxl_features = find_program('cxl-features.sh')
+  tests += [
+    [ 'cxl-features.sh',        cxl_features,       'cxl'   ],
+  ]
+endif
+
 foreach t : tests
   test(t[0], t[1],
     is_parallel : false,
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [NDCTL PATCH v7 3/4] ndctl: Add features.h from kernel UAPI
  2025-05-19 20:00 ` [NDCTL PATCH v7 3/4] ndctl: Add features.h from kernel UAPI Dave Jiang
@ 2025-05-20 19:08   ` Alison Schofield
  0 siblings, 0 replies; 10+ messages in thread
From: Alison Schofield @ 2025-05-20 19:08 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-cxl, nvdimm

On Mon, May 19, 2025 at 01:00:53PM -0700, Dave Jiang wrote:
> Pull in the kernel UAPI header files from CXL Features support and FWCTL.
> This is needed to support building the CXL Features unit test.


__counted_by's got supported in GCC 14.1 and appended failure was with 12.2.1.
Suggest conditionally stubbing it out like this:

diff --git a/cxl/fwctl/features.h b/cxl/fwctl/features.h
index 490606d7694b..b71a2fdf2040 100644
--- a/cxl/fwctl/features.h
+++ b/cxl/fwctl/features.h
@@ -9,6 +9,14 @@
 
 #include <linux/types.h>
 
+/*
+ * Vendored Version: Do not use a compiler attribute that may not be available
+ * to user space. counted_by_'s are supported in GCC 14.1
+ */
+#ifndef __counted_by_le
+#define __counted_by_le(count)
+#endif
+


FAILED: test/fwctl.p/fwctl.c.o 
cc -Itest/fwctl.p -Itest -I../test -I. -I.. -Indctl -I../ndctl -I/usr/include/uuid -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=gnu99 -O0 -g -Wchar-subscripts -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wshadow -Wsign-compare -Wstrict-prototypes -Wtype-limits -Wmaybe-uninitialized -Wdeclaration-after-statement -Wunused-result -include config.h -MD -MQ test/fwctl.p/fwctl.c.o -MF test/fwctl.p/fwctl.c.o.d -o test/fwctl.p/fwctl.c.o -c ../test/fwctl.c
In file included from ../test/fwctl.c:17:
../cxl/fwctl/features.h:105:38: error: expected ‘:’, ‘,’, ‘;’, ‘}’ or ‘__attribute__’ before ‘__counted_by_le’
  105 |         struct cxl_feat_entry ents[] __counted_by_le(num_entries);
      |                                      ^~~~~~~~~~~~~~~
../test/fwctl.c: In function ‘cxl_fwctl_rpc_get_supported_features’:
../test/fwctl.c:323:26: error: ‘struct cxl_mbox_get_sup_feats_out’ has no member named ‘ents’
  323 |         entry = &feat_out->ents[0];
      |                          ^~
[91/254] Generating symbol file daxctl/lib/libdaxctl.so.1.1.6.p/libdaxctl.so.1.1.6.symbols
ninja: build stopped: subcommand failed.

snip


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [NDCTL PATCH v7 2/4] cxl: Enumerate major/minor of FWCTL char device
  2025-05-19 20:00 ` [NDCTL PATCH v7 2/4] cxl: Enumerate major/minor of FWCTL char device Dave Jiang
@ 2025-05-20 19:11   ` Alison Schofield
  0 siblings, 0 replies; 10+ messages in thread
From: Alison Schofield @ 2025-05-20 19:11 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-cxl, nvdimm, Dan Williams

On Mon, May 19, 2025 at 01:00:52PM -0700, Dave Jiang wrote:
> Add major/minor discovery for the FWCTL character device that is associated
> with supprting CXL Features under 'struct cxl_fwctl'. A 'struct cxl_fwctl'
> may be installed under cxl_memdev if CXL Features are supported and FWCTL
> is enabled. Add libcxl API functions to retrieve the major and minor of the
> FWCTL character device.

with this:  meson configure -Dfwctl=disabled
getting this:

[119/252] Compiling C object cxl/lib/libcxl.so.1.0.8.p/libcxl.c.o
../cxl/lib/libcxl.c:1291:12: warning: ‘memdev_add_fwctl’ defined but not used [-Wunused-function]
 1291 | static int memdev_add_fwctl(struct cxl_memdev *memdev, const char *cxlmem_base)
      |            ^~~~~~~~~~~~~~~~


snip

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [NDCTL PATCH v7 4/4] cxl/test: Add test for cxl features device
  2025-05-19 20:00 ` [NDCTL PATCH v7 4/4] cxl/test: Add test for cxl features device Dave Jiang
@ 2025-05-20 19:25   ` Alison Schofield
  2025-05-21 23:22     ` Dave Jiang
  0 siblings, 1 reply; 10+ messages in thread
From: Alison Schofield @ 2025-05-20 19:25 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-cxl, nvdimm, Dan Williams

On Mon, May 19, 2025 at 01:00:54PM -0700, Dave Jiang wrote:
> Add a unit test to verify the features ioctl commands. Test support added
> for locating a features device, retrieve and verify the supported features
> commands, retrieve specific feature command data, retrieve test feature
> data, and write and verify test feature data.
> 
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  cxl/fwctl/cxl.h      |   2 +-
>  test/cxl-features.sh |  31 +++
>  test/fwctl.c         | 439 +++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build     |  19 ++
>  4 files changed, 490 insertions(+), 1 deletion(-)
>  create mode 100755 test/cxl-features.sh
>  create mode 100644 test/fwctl.c
> 
> diff --git a/cxl/fwctl/cxl.h b/cxl/fwctl/cxl.h
> index 43f522f0cdcd..c560b2a1181d 100644
> --- a/cxl/fwctl/cxl.h
> +++ b/cxl/fwctl/cxl.h
> @@ -9,7 +9,7 @@
>  
>  #include <linux/types.h>
>  #include <linux/stddef.h>
> -#include <cxl/features.h>
> +#include "features.h"
>  
>  /**
>   * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input for CXL
> diff --git a/test/cxl-features.sh b/test/cxl-features.sh
> new file mode 100755
> index 000000000000..3498fa08be53
> --- /dev/null
> +++ b/test/cxl-features.sh
> @@ -0,0 +1,31 @@
> +#!/bin/bash -Ex
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2025 Intel Corporation. All rights reserved.
> +
> +rc=77
> +# 237 is -ENODEV
> +ERR_NODEV=237
> +
> +. $(dirname $0)/common
> +FEATURES="$TEST_PATH"/fwctl
> +
> +trap 'err $LINENO' ERR
> +
> +modprobe cxl_test
> +
> +test -x "$FEATURES" || do_skip "no CXL Features Contrl"

Seems like a comment to help understand skip - 

# fwctl test is omitted when ndctl is built with -Dfwctl=disabled
# or the kernel is built without CONFIG_CXL_FEATURES enabled.

Now the above is assuming the .sh got in the test list and is 
not left out completely with -Dfwctl=disabled. Going to look.


> +# disable trap
> +trap - $(compgen -A signal)
> +"$FEATURES"
> +rc=$?
> +
> +echo "error: $rc"
> +if [ "$rc" -eq "$ERR_NODEV" ]; then
> +	do_skip "no CXL FWCTL char dev"
> +elif [ "$rc" -ne 0 ]; then
> +	echo "fail: $LINENO" && exit 1
> +fi
> +
> +trap 'err $LINENO' ERR
> +
> +_cxl_cleanup
> diff --git a/test/fwctl.c b/test/fwctl.c
> new file mode 100644
> index 000000000000..7a780e718872
> --- /dev/null
> +++ b/test/fwctl.c
> @@ -0,0 +1,439 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2024-2025 Intel Corporation. All rights reserved.
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <endian.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <syslog.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include <cxl/libcxl.h>
> +#include <linux/uuid.h>
> +#include <uuid/uuid.h>
> +#include <util/bitmap.h>
> +#include <cxl/fwctl/features.h>
> +#include <cxl/fwctl/fwctl.h>
> +#include <cxl/fwctl/cxl.h>
> +
> +static const char provider[] = "cxl_test";
> +
> +UUID_DEFINE(test_uuid,
> +	    0xff, 0xff, 0xff, 0xff,
> +	    0xff, 0xff,
> +	    0xff, 0xff,
> +	    0xff, 0xff,
> +	    0xff, 0xff, 0xff, 0xff, 0xff, 0xff
> +);
> +
> +#define CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES	0x0500
> +#define CXL_MBOX_OPCODE_GET_FEATURE		0x0501
> +#define CXL_MBOX_OPCODE_SET_FEATURE		0x0502
> +
> +#define GET_FEAT_SIZE	4
> +#define SET_FEAT_SIZE	4
> +#define EFFECTS_MASK	(BIT(0) | BIT(9))
> +
> +#define MAX_TEST_FEATURES	1
> +#define DEFAULT_TEST_DATA	0xdeadbeef
> +#define DEFAULT_TEST_DATA2	0xabcdabcd
> +
> +struct test_feature {
> +	uuid_t uuid;
> +	size_t get_size;
> +	size_t set_size;
> +};
> +
> +static int send_command(int fd, struct fwctl_rpc *rpc, struct fwctl_rpc_cxl_out *out)
> +{
> +	if (ioctl(fd, FWCTL_RPC, rpc) == -1) {
> +		fprintf(stderr, "RPC ioctl error: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	if (out->retval) {
> +		fprintf(stderr, "operation returned failure: %d\n", out->retval);
> +		return -ENXIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int get_scope(u16 opcode)
> +{
> +	switch (opcode) {
> +	case CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES:
> +	case CXL_MBOX_OPCODE_GET_FEATURE:
> +		return FWCTL_RPC_CONFIGURATION;
> +	case CXL_MBOX_OPCODE_SET_FEATURE:
> +		return FWCTL_RPC_DEBUG_WRITE_FULL;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static size_t hw_op_size(u16 opcode)
> +{
> +	switch (opcode) {
> +	case CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES:
> +		return sizeof(struct cxl_mbox_get_sup_feats_in);
> +	case CXL_MBOX_OPCODE_GET_FEATURE:
> +		return sizeof(struct cxl_mbox_get_feat_in);
> +	case CXL_MBOX_OPCODE_SET_FEATURE:
> +		return sizeof(struct cxl_mbox_set_feat_in) + sizeof(u32);
> +	default:
> +		return SIZE_MAX;
> +	}
> +}
> +
> +static void free_rpc(struct fwctl_rpc *rpc)
> +{
> +	void *in, *out;
> +
> +	in = (void *)rpc->in;
> +	out = (void *)rpc->out;
> +	free(in);
> +	free(out);
> +	free(rpc);
> +}
> +
> +static void *zmalloc_aligned(size_t align, size_t size)
> +{
> +	void *ptr;
> +	int rc;
> +
> +	rc = posix_memalign((void **)&ptr, align, size);
> +	if (rc)
> +		return NULL;
> +	memset(ptr, 0, size);
> +
> +	return ptr;
> +}
> +
> +static struct fwctl_rpc *get_prepped_command(size_t in_size, size_t out_size,
> +					     u16 opcode)
> +{
> +	struct fwctl_rpc_cxl_out *out;
> +	struct fwctl_rpc_cxl *in;
> +	struct fwctl_rpc *rpc;
> +	size_t op_size;
> +	int scope;
> +
> +	rpc = zmalloc_aligned(16, sizeof(*rpc));
> +	if (!rpc)
> +		return NULL;
> +
> +	in = zmalloc_aligned(16, in_size);
> +	if (!in)
> +		goto free_rpc;
> +
> +	out = zmalloc_aligned(16, out_size);
> +	if (!out)
> +		goto free_in;
> +
> +	in->opcode = opcode;
> +
> +	op_size = hw_op_size(opcode);
> +	if (op_size == SIZE_MAX)
> +		goto free_in;
> +
> +	in->op_size = op_size;
> +
> +	rpc->size = sizeof(*rpc);
> +	scope = get_scope(opcode);
> +	if (scope < 0)
> +		goto free_all;
> +
> +	rpc->scope = scope;
> +
> +	rpc->in_len = in_size;
> +	rpc->out_len = out_size;
> +	rpc->in = (uint64_t)(uint64_t *)in;
> +	rpc->out = (uint64_t)(uint64_t *)out;
> +
> +	return rpc;
> +
> +free_all:
> +	free(out);
> +free_in:
> +	free(in);
> +free_rpc:
> +	free(rpc);
> +	return NULL;
> +}
> +
> +static int cxl_fwctl_rpc_get_test_feature(int fd, struct test_feature *feat_ctx,
> +					  const uint32_t expected_data)
> +{
> +	struct cxl_mbox_get_feat_in *feat_in;
> +	struct fwctl_rpc_cxl_out *out;
> +	size_t out_size, in_size;
> +	struct fwctl_rpc_cxl *in;
> +	struct fwctl_rpc *rpc;
> +	uint32_t val;
> +	void *data;
> +	int rc;
> +
> +	in_size = sizeof(*in) + sizeof(*feat_in);
> +	out_size = sizeof(*out) + feat_ctx->get_size;
> +
> +	rpc = get_prepped_command(in_size, out_size,
> +				  CXL_MBOX_OPCODE_GET_FEATURE);
> +	if (!rpc)
> +		return -ENXIO;
> +
> +	in = (struct fwctl_rpc_cxl *)rpc->in;
> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> +
> +	feat_in = &in->get_feat_in;
> +	uuid_copy(feat_in->uuid, feat_ctx->uuid);
> +	feat_in->count = feat_ctx->get_size;
> +
> +	rc = send_command(fd, rpc, out);
> +	if (rc)
> +		goto out;
> +
> +	data = out->payload;
> +	val = le32toh(*(__le32 *)data);
> +	if (memcmp(&val, &expected_data, sizeof(val)) != 0) {
> +		rc = -ENXIO;
> +		goto out;
> +	}
> +
> +out:
> +	free_rpc(rpc);
> +	return rc;
> +}
> +
> +static int cxl_fwctl_rpc_set_test_feature(int fd, struct test_feature *feat_ctx)
> +{
> +	struct cxl_mbox_set_feat_in *feat_in;
> +	struct fwctl_rpc_cxl_out *out;
> +	size_t in_size, out_size;
> +	struct fwctl_rpc_cxl *in;
> +	struct fwctl_rpc *rpc;
> +	uint32_t val;
> +	void *data;
> +	int rc;
> +
> +	in_size = sizeof(*in) + sizeof(*feat_in) + sizeof(val);
> +	out_size = sizeof(*out) + sizeof(val);
> +	rpc = get_prepped_command(in_size, out_size,
> +				  CXL_MBOX_OPCODE_SET_FEATURE);
> +	if (!rpc)
> +		return -ENXIO;
> +
> +	in = (struct fwctl_rpc_cxl *)rpc->in;
> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> +	feat_in = &in->set_feat_in;
> +	uuid_copy(feat_in->uuid, feat_ctx->uuid);
> +	data = feat_in->feat_data;
> +	val = DEFAULT_TEST_DATA2;
> +	*(uint32_t *)data = htole32(val);
> +	feat_in->flags = CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER;
> +
> +	rc = send_command(fd, rpc, out);
> +	if (rc)
> +		goto out;
> +
> +	rc = cxl_fwctl_rpc_get_test_feature(fd, feat_ctx, DEFAULT_TEST_DATA2);
> +	if (rc) {
> +		fprintf(stderr, "Failed ioctl to get feature verify: %d\n", rc);
> +		goto out;
> +	}
> +
> +out:
> +	free_rpc(rpc);
> +	return rc;
> +}
> +
> +static int cxl_fwctl_rpc_get_supported_features(int fd, struct test_feature *feat_ctx)
> +{
> +	struct cxl_mbox_get_sup_feats_out *feat_out;
> +	struct cxl_mbox_get_sup_feats_in *feat_in;
> +	struct fwctl_rpc_cxl_out *out;
> +	struct cxl_feat_entry *entry;
> +	size_t out_size, in_size;
> +	struct fwctl_rpc_cxl *in;
> +	struct fwctl_rpc *rpc;
> +	int feats, rc;
> +
> +	in_size = sizeof(*in) + sizeof(*feat_in);
> +	out_size = sizeof(*out) + sizeof(*feat_out);
> +	/* First query, to get number of features w/o per feature data */
> +	rpc = get_prepped_command(in_size, out_size,
> +				  CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES);
> +	if (!rpc)
> +		return -ENXIO;
> +
> +	/* No need to fill in feat_in first go as we are passing in all 0's */
> +
> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> +	rc = send_command(fd, rpc, out);
> +	if (rc)
> +		goto out;
> +
> +	feat_out = &out->get_sup_feats_out;
> +	feats = le16toh(feat_out->supported_feats);
> +	if (feats != MAX_TEST_FEATURES) {
> +		fprintf(stderr, "Test device has greater than %d test features.\n",
> +			MAX_TEST_FEATURES);
> +		rc = -ENXIO;
> +		goto out;
> +	}
> +
> +	free_rpc(rpc);
> +
> +	/* Going second round to retrieve each feature details */
> +	in_size = sizeof(*in) + sizeof(*feat_in);
> +	out_size = sizeof(*out) + sizeof(*feat_out);
> +	out_size += feats * sizeof(*entry);
> +	rpc = get_prepped_command(in_size, out_size,
> +				  CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES);
> +	if (!rpc)
> +		return -ENXIO;
> +
> +	in = (struct fwctl_rpc_cxl *)rpc->in;
> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> +	feat_in = &in->get_sup_feats_in;
> +	feat_in->count = htole32(feats * sizeof(*entry));
> +
> +	rc = send_command(fd, rpc, out);
> +	if (rc)
> +		goto out;
> +
> +	feat_out = &out->get_sup_feats_out;
> +	feats = le16toh(feat_out->supported_feats);
> +	if (feats != MAX_TEST_FEATURES) {
> +		fprintf(stderr, "Test device has greater than %u test features.\n",
> +			MAX_TEST_FEATURES);
> +		rc = -ENXIO;
> +		goto out;
> +	}
> +
> +	if (le16toh(feat_out->num_entries) != MAX_TEST_FEATURES) {
> +		fprintf(stderr, "Test device did not return expected entries. %u\n",
> +			le16toh(feat_out->num_entries));
> +		rc = -ENXIO;
> +		goto out;
> +	}
> +
> +	entry = &feat_out->ents[0];
> +	if (uuid_compare(test_uuid, entry->uuid) != 0) {
> +		fprintf(stderr, "Test device did not export expected test feature.\n");
> +		rc = -ENXIO;
> +		goto out;
> +	}
> +
> +	if (le16toh(entry->get_feat_size) != GET_FEAT_SIZE ||
> +	    le16toh(entry->set_feat_size) != SET_FEAT_SIZE) {
> +		fprintf(stderr, "Test device feature in/out size incorrect.\n");
> +		rc = -ENXIO;
> +		goto out;
> +	}
> +
> +	if (le16toh(entry->effects) != EFFECTS_MASK) {
> +		fprintf(stderr, "Test device set effects incorrect\n");
> +		rc = -ENXIO;
> +		goto out;
> +	}
> +
> +	uuid_copy(feat_ctx->uuid, entry->uuid);
> +	feat_ctx->get_size = le16toh(entry->get_feat_size);
> +	feat_ctx->set_size = le16toh(entry->set_feat_size);
> +
> +out:
> +	free_rpc(rpc);
> +	return rc;
> +}
> +
> +static int test_fwctl_features(struct cxl_memdev *memdev)
> +{
> +	struct test_feature feat_ctx;
> +	unsigned int major, minor;
> +	struct cxl_fwctl *fwctl;
> +	int fd, rc;
> +	char path[256];
> +
> +	fwctl = cxl_memdev_get_fwctl(memdev);
> +	if (!fwctl)
> +		return -ENODEV;
> +
> +	major = cxl_fwctl_get_major(fwctl);
> +	minor = cxl_fwctl_get_minor(fwctl);
> +
> +	if (!major && !minor)
> +		return -ENODEV;
> +
> +	sprintf(path, "/dev/char/%d:%d", major, minor);
> +
> +	fd = open(path, O_RDONLY, 0644);
> +	if (fd < 0) {
> +		fprintf(stderr, "Failed to open: %d\n", -errno);
> +		return -errno;
> +	}
> +
> +	rc = cxl_fwctl_rpc_get_supported_features(fd, &feat_ctx);
> +	if (rc) {
> +		fprintf(stderr, "Failed ioctl to get supported features: %d\n", rc);
> +		goto out;
> +	}
> +
> +	rc = cxl_fwctl_rpc_get_test_feature(fd, &feat_ctx, DEFAULT_TEST_DATA);
> +	if (rc) {
> +		fprintf(stderr, "Failed ioctl to get feature: %d\n", rc);
> +		goto out;
> +	}
> +
> +	rc = cxl_fwctl_rpc_set_test_feature(fd, &feat_ctx);
> +	if (rc) {
> +		fprintf(stderr, "Failed ioctl to set feature: %d\n", rc);
> +		goto out;
> +	}
> +
> +out:
> +	close(fd);
> +	return rc;
> +}
> +
> +static int test_fwctl(struct cxl_ctx *ctx, struct cxl_bus *bus)
> +{
> +	struct cxl_memdev *memdev;
> +
> +	cxl_memdev_foreach(ctx, memdev) {
> +		if (cxl_memdev_get_bus(memdev) != bus)
> +			continue;
> +		return test_fwctl_features(memdev);
> +	}
> +
> +	return 0;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	struct cxl_ctx *ctx;
> +	struct cxl_bus *bus;
> +	int rc;
> +
> +	rc = cxl_new(&ctx);
> +	if (rc < 0)
> +		return rc;
> +
> +	cxl_set_log_priority(ctx, LOG_DEBUG);
> +
> +	bus = cxl_bus_get_by_provider(ctx, provider);
> +	if (!bus) {
> +		fprintf(stderr, "%s: unable to find bus (%s)\n",
> +			argv[0], provider);
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +
> +	rc = test_fwctl(ctx, bus);
> +
> +out:
> +	cxl_unref(ctx);
> +	return rc;
> +}
> diff --git a/test/meson.build b/test/meson.build
> index 2fd7df5211dd..93b1d78671ef 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -17,6 +17,13 @@ ndctl_deps = libndctl_deps + [
>    versiondep,
>  ]
>  
> +libcxl_deps = [
> +  cxl_dep,
> +  ndctl_dep,
> +  uuid,
> +  kmod,
> +]
> +
>  libndctl = executable('libndctl', testcore + [ 'libndctl.c'],
>    dependencies : libndctl_deps,
>    include_directories : root_inc,
> @@ -235,6 +242,18 @@ if get_option('keyutils').enabled()
>    ]
>  endif
>  
> +uuid_dep = dependency('uuid', required: false)
> +if get_option('fwctl').enabled() and uuid_dep.found()
> +  fwctl = executable('fwctl', 'fwctl.c',
> +    dependencies : libcxl_deps,
> +    include_directories : root_inc,
> +  )
> +  cxl_features = find_program('cxl-features.sh')
> +  tests += [
> +    [ 'cxl-features.sh',        cxl_features,       'cxl'   ],
> +  ]
> +endif
> +
>  foreach t : tests
>    test(t[0], t[1],
>      is_parallel : false,
> -- 
> 2.49.0
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [NDCTL PATCH v7 4/4] cxl/test: Add test for cxl features device
  2025-05-20 19:25   ` Alison Schofield
@ 2025-05-21 23:22     ` Dave Jiang
  2025-05-22  0:18       ` Alison Schofield
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Jiang @ 2025-05-21 23:22 UTC (permalink / raw)
  To: Alison Schofield; +Cc: linux-cxl, nvdimm, Dan Williams



On 5/20/25 12:25 PM, Alison Schofield wrote:
> On Mon, May 19, 2025 at 01:00:54PM -0700, Dave Jiang wrote:
>> Add a unit test to verify the features ioctl commands. Test support added
>> for locating a features device, retrieve and verify the supported features
>> commands, retrieve specific feature command data, retrieve test feature
>> data, and write and verify test feature data.
>>
>> Acked-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  cxl/fwctl/cxl.h      |   2 +-
>>  test/cxl-features.sh |  31 +++
>>  test/fwctl.c         | 439 +++++++++++++++++++++++++++++++++++++++++++
>>  test/meson.build     |  19 ++
>>  4 files changed, 490 insertions(+), 1 deletion(-)
>>  create mode 100755 test/cxl-features.sh
>>  create mode 100644 test/fwctl.c
>>
>> diff --git a/cxl/fwctl/cxl.h b/cxl/fwctl/cxl.h
>> index 43f522f0cdcd..c560b2a1181d 100644
>> --- a/cxl/fwctl/cxl.h
>> +++ b/cxl/fwctl/cxl.h
>> @@ -9,7 +9,7 @@
>>  
>>  #include <linux/types.h>
>>  #include <linux/stddef.h>
>> -#include <cxl/features.h>
>> +#include "features.h"
>>  
>>  /**
>>   * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input for CXL
>> diff --git a/test/cxl-features.sh b/test/cxl-features.sh
>> new file mode 100755
>> index 000000000000..3498fa08be53
>> --- /dev/null
>> +++ b/test/cxl-features.sh
>> @@ -0,0 +1,31 @@
>> +#!/bin/bash -Ex
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2025 Intel Corporation. All rights reserved.
>> +
>> +rc=77
>> +# 237 is -ENODEV
>> +ERR_NODEV=237
>> +
>> +. $(dirname $0)/common
>> +FEATURES="$TEST_PATH"/fwctl
>> +
>> +trap 'err $LINENO' ERR
>> +
>> +modprobe cxl_test
>> +
>> +test -x "$FEATURES" || do_skip "no CXL Features Contrl"
> 
> Seems like a comment to help understand skip - 
> 
> # fwctl test is omitted when ndctl is built with -Dfwctl=disabled
> # or the kernel is built without CONFIG_CXL_FEATURES enabled.
> 
> Now the above is assuming the .sh got in the test list and is 
> not left out completely with -Dfwctl=disabled. Going to look.

It's excluded. So comment probably not needed. We skip if there's no fwctl device later, which points to no kernel config enabled.

DJ

> 
> 
>> +# disable trap
>> +trap - $(compgen -A signal)
>> +"$FEATURES"
>> +rc=$?
>> +
>> +echo "error: $rc"
>> +if [ "$rc" -eq "$ERR_NODEV" ]; then
>> +	do_skip "no CXL FWCTL char dev"
>> +elif [ "$rc" -ne 0 ]; then
>> +	echo "fail: $LINENO" && exit 1
>> +fi
>> +
>> +trap 'err $LINENO' ERR
>> +
>> +_cxl_cleanup
>> diff --git a/test/fwctl.c b/test/fwctl.c
>> new file mode 100644
>> index 000000000000..7a780e718872
>> --- /dev/null
>> +++ b/test/fwctl.c
>> @@ -0,0 +1,439 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (C) 2024-2025 Intel Corporation. All rights reserved.
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <stdio.h>
>> +#include <endian.h>
>> +#include <stdint.h>
>> +#include <stdlib.h>
>> +#include <syslog.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include <sys/ioctl.h>
>> +#include <cxl/libcxl.h>
>> +#include <linux/uuid.h>
>> +#include <uuid/uuid.h>
>> +#include <util/bitmap.h>
>> +#include <cxl/fwctl/features.h>
>> +#include <cxl/fwctl/fwctl.h>
>> +#include <cxl/fwctl/cxl.h>
>> +
>> +static const char provider[] = "cxl_test";
>> +
>> +UUID_DEFINE(test_uuid,
>> +	    0xff, 0xff, 0xff, 0xff,
>> +	    0xff, 0xff,
>> +	    0xff, 0xff,
>> +	    0xff, 0xff,
>> +	    0xff, 0xff, 0xff, 0xff, 0xff, 0xff
>> +);
>> +
>> +#define CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES	0x0500
>> +#define CXL_MBOX_OPCODE_GET_FEATURE		0x0501
>> +#define CXL_MBOX_OPCODE_SET_FEATURE		0x0502
>> +
>> +#define GET_FEAT_SIZE	4
>> +#define SET_FEAT_SIZE	4
>> +#define EFFECTS_MASK	(BIT(0) | BIT(9))
>> +
>> +#define MAX_TEST_FEATURES	1
>> +#define DEFAULT_TEST_DATA	0xdeadbeef
>> +#define DEFAULT_TEST_DATA2	0xabcdabcd
>> +
>> +struct test_feature {
>> +	uuid_t uuid;
>> +	size_t get_size;
>> +	size_t set_size;
>> +};
>> +
>> +static int send_command(int fd, struct fwctl_rpc *rpc, struct fwctl_rpc_cxl_out *out)
>> +{
>> +	if (ioctl(fd, FWCTL_RPC, rpc) == -1) {
>> +		fprintf(stderr, "RPC ioctl error: %s\n", strerror(errno));
>> +		return -errno;
>> +	}
>> +
>> +	if (out->retval) {
>> +		fprintf(stderr, "operation returned failure: %d\n", out->retval);
>> +		return -ENXIO;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int get_scope(u16 opcode)
>> +{
>> +	switch (opcode) {
>> +	case CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES:
>> +	case CXL_MBOX_OPCODE_GET_FEATURE:
>> +		return FWCTL_RPC_CONFIGURATION;
>> +	case CXL_MBOX_OPCODE_SET_FEATURE:
>> +		return FWCTL_RPC_DEBUG_WRITE_FULL;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>> +static size_t hw_op_size(u16 opcode)
>> +{
>> +	switch (opcode) {
>> +	case CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES:
>> +		return sizeof(struct cxl_mbox_get_sup_feats_in);
>> +	case CXL_MBOX_OPCODE_GET_FEATURE:
>> +		return sizeof(struct cxl_mbox_get_feat_in);
>> +	case CXL_MBOX_OPCODE_SET_FEATURE:
>> +		return sizeof(struct cxl_mbox_set_feat_in) + sizeof(u32);
>> +	default:
>> +		return SIZE_MAX;
>> +	}
>> +}
>> +
>> +static void free_rpc(struct fwctl_rpc *rpc)
>> +{
>> +	void *in, *out;
>> +
>> +	in = (void *)rpc->in;
>> +	out = (void *)rpc->out;
>> +	free(in);
>> +	free(out);
>> +	free(rpc);
>> +}
>> +
>> +static void *zmalloc_aligned(size_t align, size_t size)
>> +{
>> +	void *ptr;
>> +	int rc;
>> +
>> +	rc = posix_memalign((void **)&ptr, align, size);
>> +	if (rc)
>> +		return NULL;
>> +	memset(ptr, 0, size);
>> +
>> +	return ptr;
>> +}
>> +
>> +static struct fwctl_rpc *get_prepped_command(size_t in_size, size_t out_size,
>> +					     u16 opcode)
>> +{
>> +	struct fwctl_rpc_cxl_out *out;
>> +	struct fwctl_rpc_cxl *in;
>> +	struct fwctl_rpc *rpc;
>> +	size_t op_size;
>> +	int scope;
>> +
>> +	rpc = zmalloc_aligned(16, sizeof(*rpc));
>> +	if (!rpc)
>> +		return NULL;
>> +
>> +	in = zmalloc_aligned(16, in_size);
>> +	if (!in)
>> +		goto free_rpc;
>> +
>> +	out = zmalloc_aligned(16, out_size);
>> +	if (!out)
>> +		goto free_in;
>> +
>> +	in->opcode = opcode;
>> +
>> +	op_size = hw_op_size(opcode);
>> +	if (op_size == SIZE_MAX)
>> +		goto free_in;
>> +
>> +	in->op_size = op_size;
>> +
>> +	rpc->size = sizeof(*rpc);
>> +	scope = get_scope(opcode);
>> +	if (scope < 0)
>> +		goto free_all;
>> +
>> +	rpc->scope = scope;
>> +
>> +	rpc->in_len = in_size;
>> +	rpc->out_len = out_size;
>> +	rpc->in = (uint64_t)(uint64_t *)in;
>> +	rpc->out = (uint64_t)(uint64_t *)out;
>> +
>> +	return rpc;
>> +
>> +free_all:
>> +	free(out);
>> +free_in:
>> +	free(in);
>> +free_rpc:
>> +	free(rpc);
>> +	return NULL;
>> +}
>> +
>> +static int cxl_fwctl_rpc_get_test_feature(int fd, struct test_feature *feat_ctx,
>> +					  const uint32_t expected_data)
>> +{
>> +	struct cxl_mbox_get_feat_in *feat_in;
>> +	struct fwctl_rpc_cxl_out *out;
>> +	size_t out_size, in_size;
>> +	struct fwctl_rpc_cxl *in;
>> +	struct fwctl_rpc *rpc;
>> +	uint32_t val;
>> +	void *data;
>> +	int rc;
>> +
>> +	in_size = sizeof(*in) + sizeof(*feat_in);
>> +	out_size = sizeof(*out) + feat_ctx->get_size;
>> +
>> +	rpc = get_prepped_command(in_size, out_size,
>> +				  CXL_MBOX_OPCODE_GET_FEATURE);
>> +	if (!rpc)
>> +		return -ENXIO;
>> +
>> +	in = (struct fwctl_rpc_cxl *)rpc->in;
>> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
>> +
>> +	feat_in = &in->get_feat_in;
>> +	uuid_copy(feat_in->uuid, feat_ctx->uuid);
>> +	feat_in->count = feat_ctx->get_size;
>> +
>> +	rc = send_command(fd, rpc, out);
>> +	if (rc)
>> +		goto out;
>> +
>> +	data = out->payload;
>> +	val = le32toh(*(__le32 *)data);
>> +	if (memcmp(&val, &expected_data, sizeof(val)) != 0) {
>> +		rc = -ENXIO;
>> +		goto out;
>> +	}
>> +
>> +out:
>> +	free_rpc(rpc);
>> +	return rc;
>> +}
>> +
>> +static int cxl_fwctl_rpc_set_test_feature(int fd, struct test_feature *feat_ctx)
>> +{
>> +	struct cxl_mbox_set_feat_in *feat_in;
>> +	struct fwctl_rpc_cxl_out *out;
>> +	size_t in_size, out_size;
>> +	struct fwctl_rpc_cxl *in;
>> +	struct fwctl_rpc *rpc;
>> +	uint32_t val;
>> +	void *data;
>> +	int rc;
>> +
>> +	in_size = sizeof(*in) + sizeof(*feat_in) + sizeof(val);
>> +	out_size = sizeof(*out) + sizeof(val);
>> +	rpc = get_prepped_command(in_size, out_size,
>> +				  CXL_MBOX_OPCODE_SET_FEATURE);
>> +	if (!rpc)
>> +		return -ENXIO;
>> +
>> +	in = (struct fwctl_rpc_cxl *)rpc->in;
>> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
>> +	feat_in = &in->set_feat_in;
>> +	uuid_copy(feat_in->uuid, feat_ctx->uuid);
>> +	data = feat_in->feat_data;
>> +	val = DEFAULT_TEST_DATA2;
>> +	*(uint32_t *)data = htole32(val);
>> +	feat_in->flags = CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER;
>> +
>> +	rc = send_command(fd, rpc, out);
>> +	if (rc)
>> +		goto out;
>> +
>> +	rc = cxl_fwctl_rpc_get_test_feature(fd, feat_ctx, DEFAULT_TEST_DATA2);
>> +	if (rc) {
>> +		fprintf(stderr, "Failed ioctl to get feature verify: %d\n", rc);
>> +		goto out;
>> +	}
>> +
>> +out:
>> +	free_rpc(rpc);
>> +	return rc;
>> +}
>> +
>> +static int cxl_fwctl_rpc_get_supported_features(int fd, struct test_feature *feat_ctx)
>> +{
>> +	struct cxl_mbox_get_sup_feats_out *feat_out;
>> +	struct cxl_mbox_get_sup_feats_in *feat_in;
>> +	struct fwctl_rpc_cxl_out *out;
>> +	struct cxl_feat_entry *entry;
>> +	size_t out_size, in_size;
>> +	struct fwctl_rpc_cxl *in;
>> +	struct fwctl_rpc *rpc;
>> +	int feats, rc;
>> +
>> +	in_size = sizeof(*in) + sizeof(*feat_in);
>> +	out_size = sizeof(*out) + sizeof(*feat_out);
>> +	/* First query, to get number of features w/o per feature data */
>> +	rpc = get_prepped_command(in_size, out_size,
>> +				  CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES);
>> +	if (!rpc)
>> +		return -ENXIO;
>> +
>> +	/* No need to fill in feat_in first go as we are passing in all 0's */
>> +
>> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
>> +	rc = send_command(fd, rpc, out);
>> +	if (rc)
>> +		goto out;
>> +
>> +	feat_out = &out->get_sup_feats_out;
>> +	feats = le16toh(feat_out->supported_feats);
>> +	if (feats != MAX_TEST_FEATURES) {
>> +		fprintf(stderr, "Test device has greater than %d test features.\n",
>> +			MAX_TEST_FEATURES);
>> +		rc = -ENXIO;
>> +		goto out;
>> +	}
>> +
>> +	free_rpc(rpc);
>> +
>> +	/* Going second round to retrieve each feature details */
>> +	in_size = sizeof(*in) + sizeof(*feat_in);
>> +	out_size = sizeof(*out) + sizeof(*feat_out);
>> +	out_size += feats * sizeof(*entry);
>> +	rpc = get_prepped_command(in_size, out_size,
>> +				  CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES);
>> +	if (!rpc)
>> +		return -ENXIO;
>> +
>> +	in = (struct fwctl_rpc_cxl *)rpc->in;
>> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
>> +	feat_in = &in->get_sup_feats_in;
>> +	feat_in->count = htole32(feats * sizeof(*entry));
>> +
>> +	rc = send_command(fd, rpc, out);
>> +	if (rc)
>> +		goto out;
>> +
>> +	feat_out = &out->get_sup_feats_out;
>> +	feats = le16toh(feat_out->supported_feats);
>> +	if (feats != MAX_TEST_FEATURES) {
>> +		fprintf(stderr, "Test device has greater than %u test features.\n",
>> +			MAX_TEST_FEATURES);
>> +		rc = -ENXIO;
>> +		goto out;
>> +	}
>> +
>> +	if (le16toh(feat_out->num_entries) != MAX_TEST_FEATURES) {
>> +		fprintf(stderr, "Test device did not return expected entries. %u\n",
>> +			le16toh(feat_out->num_entries));
>> +		rc = -ENXIO;
>> +		goto out;
>> +	}
>> +
>> +	entry = &feat_out->ents[0];
>> +	if (uuid_compare(test_uuid, entry->uuid) != 0) {
>> +		fprintf(stderr, "Test device did not export expected test feature.\n");
>> +		rc = -ENXIO;
>> +		goto out;
>> +	}
>> +
>> +	if (le16toh(entry->get_feat_size) != GET_FEAT_SIZE ||
>> +	    le16toh(entry->set_feat_size) != SET_FEAT_SIZE) {
>> +		fprintf(stderr, "Test device feature in/out size incorrect.\n");
>> +		rc = -ENXIO;
>> +		goto out;
>> +	}
>> +
>> +	if (le16toh(entry->effects) != EFFECTS_MASK) {
>> +		fprintf(stderr, "Test device set effects incorrect\n");
>> +		rc = -ENXIO;
>> +		goto out;
>> +	}
>> +
>> +	uuid_copy(feat_ctx->uuid, entry->uuid);
>> +	feat_ctx->get_size = le16toh(entry->get_feat_size);
>> +	feat_ctx->set_size = le16toh(entry->set_feat_size);
>> +
>> +out:
>> +	free_rpc(rpc);
>> +	return rc;
>> +}
>> +
>> +static int test_fwctl_features(struct cxl_memdev *memdev)
>> +{
>> +	struct test_feature feat_ctx;
>> +	unsigned int major, minor;
>> +	struct cxl_fwctl *fwctl;
>> +	int fd, rc;
>> +	char path[256];
>> +
>> +	fwctl = cxl_memdev_get_fwctl(memdev);
>> +	if (!fwctl)
>> +		return -ENODEV;
>> +
>> +	major = cxl_fwctl_get_major(fwctl);
>> +	minor = cxl_fwctl_get_minor(fwctl);
>> +
>> +	if (!major && !minor)
>> +		return -ENODEV;
>> +
>> +	sprintf(path, "/dev/char/%d:%d", major, minor);
>> +
>> +	fd = open(path, O_RDONLY, 0644);
>> +	if (fd < 0) {
>> +		fprintf(stderr, "Failed to open: %d\n", -errno);
>> +		return -errno;
>> +	}
>> +
>> +	rc = cxl_fwctl_rpc_get_supported_features(fd, &feat_ctx);
>> +	if (rc) {
>> +		fprintf(stderr, "Failed ioctl to get supported features: %d\n", rc);
>> +		goto out;
>> +	}
>> +
>> +	rc = cxl_fwctl_rpc_get_test_feature(fd, &feat_ctx, DEFAULT_TEST_DATA);
>> +	if (rc) {
>> +		fprintf(stderr, "Failed ioctl to get feature: %d\n", rc);
>> +		goto out;
>> +	}
>> +
>> +	rc = cxl_fwctl_rpc_set_test_feature(fd, &feat_ctx);
>> +	if (rc) {
>> +		fprintf(stderr, "Failed ioctl to set feature: %d\n", rc);
>> +		goto out;
>> +	}
>> +
>> +out:
>> +	close(fd);
>> +	return rc;
>> +}
>> +
>> +static int test_fwctl(struct cxl_ctx *ctx, struct cxl_bus *bus)
>> +{
>> +	struct cxl_memdev *memdev;
>> +
>> +	cxl_memdev_foreach(ctx, memdev) {
>> +		if (cxl_memdev_get_bus(memdev) != bus)
>> +			continue;
>> +		return test_fwctl_features(memdev);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +	struct cxl_ctx *ctx;
>> +	struct cxl_bus *bus;
>> +	int rc;
>> +
>> +	rc = cxl_new(&ctx);
>> +	if (rc < 0)
>> +		return rc;
>> +
>> +	cxl_set_log_priority(ctx, LOG_DEBUG);
>> +
>> +	bus = cxl_bus_get_by_provider(ctx, provider);
>> +	if (!bus) {
>> +		fprintf(stderr, "%s: unable to find bus (%s)\n",
>> +			argv[0], provider);
>> +		rc = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	rc = test_fwctl(ctx, bus);
>> +
>> +out:
>> +	cxl_unref(ctx);
>> +	return rc;
>> +}
>> diff --git a/test/meson.build b/test/meson.build
>> index 2fd7df5211dd..93b1d78671ef 100644
>> --- a/test/meson.build
>> +++ b/test/meson.build
>> @@ -17,6 +17,13 @@ ndctl_deps = libndctl_deps + [
>>    versiondep,
>>  ]
>>  
>> +libcxl_deps = [
>> +  cxl_dep,
>> +  ndctl_dep,
>> +  uuid,
>> +  kmod,
>> +]
>> +
>>  libndctl = executable('libndctl', testcore + [ 'libndctl.c'],
>>    dependencies : libndctl_deps,
>>    include_directories : root_inc,
>> @@ -235,6 +242,18 @@ if get_option('keyutils').enabled()
>>    ]
>>  endif
>>  
>> +uuid_dep = dependency('uuid', required: false)
>> +if get_option('fwctl').enabled() and uuid_dep.found()
>> +  fwctl = executable('fwctl', 'fwctl.c',
>> +    dependencies : libcxl_deps,
>> +    include_directories : root_inc,
>> +  )
>> +  cxl_features = find_program('cxl-features.sh')
>> +  tests += [
>> +    [ 'cxl-features.sh',        cxl_features,       'cxl'   ],
>> +  ]
>> +endif
>> +
>>  foreach t : tests
>>    test(t[0], t[1],
>>      is_parallel : false,
>> -- 
>> 2.49.0
>>
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [NDCTL PATCH v7 4/4] cxl/test: Add test for cxl features device
  2025-05-21 23:22     ` Dave Jiang
@ 2025-05-22  0:18       ` Alison Schofield
  0 siblings, 0 replies; 10+ messages in thread
From: Alison Schofield @ 2025-05-22  0:18 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-cxl, nvdimm, Dan Williams

On Wed, May 21, 2025 at 04:22:19PM -0700, Dave Jiang wrote:
> 
> 
> On 5/20/25 12:25 PM, Alison Schofield wrote:
> > On Mon, May 19, 2025 at 01:00:54PM -0700, Dave Jiang wrote:
> >> Add a unit test to verify the features ioctl commands. Test support added
> >> for locating a features device, retrieve and verify the supported features
> >> commands, retrieve specific feature command data, retrieve test feature
> >> data, and write and verify test feature data.
> >>
> >> Acked-by: Dan Williams <dan.j.williams@intel.com>
> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> >> ---
> >>  cxl/fwctl/cxl.h      |   2 +-
> >>  test/cxl-features.sh |  31 +++
> >>  test/fwctl.c         | 439 +++++++++++++++++++++++++++++++++++++++++++
> >>  test/meson.build     |  19 ++
> >>  4 files changed, 490 insertions(+), 1 deletion(-)
> >>  create mode 100755 test/cxl-features.sh
> >>  create mode 100644 test/fwctl.c
> >>
> >> diff --git a/cxl/fwctl/cxl.h b/cxl/fwctl/cxl.h
> >> index 43f522f0cdcd..c560b2a1181d 100644
> >> --- a/cxl/fwctl/cxl.h
> >> +++ b/cxl/fwctl/cxl.h
> >> @@ -9,7 +9,7 @@
> >>  
> >>  #include <linux/types.h>
> >>  #include <linux/stddef.h>
> >> -#include <cxl/features.h>
> >> +#include "features.h"
> >>  
> >>  /**
> >>   * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input for CXL
> >> diff --git a/test/cxl-features.sh b/test/cxl-features.sh
> >> new file mode 100755
> >> index 000000000000..3498fa08be53
> >> --- /dev/null
> >> +++ b/test/cxl-features.sh
> >> @@ -0,0 +1,31 @@
> >> +#!/bin/bash -Ex
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# Copyright (C) 2025 Intel Corporation. All rights reserved.
> >> +
> >> +rc=77
> >> +# 237 is -ENODEV
> >> +ERR_NODEV=237
> >> +
> >> +. $(dirname $0)/common
> >> +FEATURES="$TEST_PATH"/fwctl
> >> +
> >> +trap 'err $LINENO' ERR
> >> +
> >> +modprobe cxl_test
> >> +
> >> +test -x "$FEATURES" || do_skip "no CXL Features Contrl"
> > 
> > Seems like a comment to help understand skip - 
> > 
> > # fwctl test is omitted when ndctl is built with -Dfwctl=disabled
> > # or the kernel is built without CONFIG_CXL_FEATURES enabled.
> > 
> > Now the above is assuming the .sh got in the test list and is 
> > not left out completely with -Dfwctl=disabled. Going to look.
> 
> It's excluded. So comment probably not needed. We skip if there's no fwctl device later, which points to no kernel config enabled.

OK. I've come around to agree on this one. My concern was folks running
the unit test foolishly thinking they had tested everything while this
test was silently skipped. But that's an unlikely because doing the
-Dfwctl=disable is a pretty intentional act. Also folks trying to run
complete suites are aware of the enables/disables because they use
them to turn on the destructive tests. I'll be quiet on this one now.

Thanks!

> 
> DJ
> 
> > 
> > 
> >> +# disable trap
> >> +trap - $(compgen -A signal)
> >> +"$FEATURES"
> >> +rc=$?
> >> +
> >> +echo "error: $rc"
> >> +if [ "$rc" -eq "$ERR_NODEV" ]; then
> >> +	do_skip "no CXL FWCTL char dev"
> >> +elif [ "$rc" -ne 0 ]; then
> >> +	echo "fail: $LINENO" && exit 1
> >> +fi
> >> +
> >> +trap 'err $LINENO' ERR
> >> +
> >> +_cxl_cleanup
> >> diff --git a/test/fwctl.c b/test/fwctl.c
> >> new file mode 100644
> >> index 000000000000..7a780e718872
> >> --- /dev/null
> >> +++ b/test/fwctl.c
> >> @@ -0,0 +1,439 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +// Copyright (C) 2024-2025 Intel Corporation. All rights reserved.
> >> +#include <errno.h>
> >> +#include <fcntl.h>
> >> +#include <stdio.h>
> >> +#include <endian.h>
> >> +#include <stdint.h>
> >> +#include <stdlib.h>
> >> +#include <syslog.h>
> >> +#include <string.h>
> >> +#include <unistd.h>
> >> +#include <sys/ioctl.h>
> >> +#include <cxl/libcxl.h>
> >> +#include <linux/uuid.h>
> >> +#include <uuid/uuid.h>
> >> +#include <util/bitmap.h>
> >> +#include <cxl/fwctl/features.h>
> >> +#include <cxl/fwctl/fwctl.h>
> >> +#include <cxl/fwctl/cxl.h>
> >> +
> >> +static const char provider[] = "cxl_test";
> >> +
> >> +UUID_DEFINE(test_uuid,
> >> +	    0xff, 0xff, 0xff, 0xff,
> >> +	    0xff, 0xff,
> >> +	    0xff, 0xff,
> >> +	    0xff, 0xff,
> >> +	    0xff, 0xff, 0xff, 0xff, 0xff, 0xff
> >> +);
> >> +
> >> +#define CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES	0x0500
> >> +#define CXL_MBOX_OPCODE_GET_FEATURE		0x0501
> >> +#define CXL_MBOX_OPCODE_SET_FEATURE		0x0502
> >> +
> >> +#define GET_FEAT_SIZE	4
> >> +#define SET_FEAT_SIZE	4
> >> +#define EFFECTS_MASK	(BIT(0) | BIT(9))
> >> +
> >> +#define MAX_TEST_FEATURES	1
> >> +#define DEFAULT_TEST_DATA	0xdeadbeef
> >> +#define DEFAULT_TEST_DATA2	0xabcdabcd
> >> +
> >> +struct test_feature {
> >> +	uuid_t uuid;
> >> +	size_t get_size;
> >> +	size_t set_size;
> >> +};
> >> +
> >> +static int send_command(int fd, struct fwctl_rpc *rpc, struct fwctl_rpc_cxl_out *out)
> >> +{
> >> +	if (ioctl(fd, FWCTL_RPC, rpc) == -1) {
> >> +		fprintf(stderr, "RPC ioctl error: %s\n", strerror(errno));
> >> +		return -errno;
> >> +	}
> >> +
> >> +	if (out->retval) {
> >> +		fprintf(stderr, "operation returned failure: %d\n", out->retval);
> >> +		return -ENXIO;
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int get_scope(u16 opcode)
> >> +{
> >> +	switch (opcode) {
> >> +	case CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES:
> >> +	case CXL_MBOX_OPCODE_GET_FEATURE:
> >> +		return FWCTL_RPC_CONFIGURATION;
> >> +	case CXL_MBOX_OPCODE_SET_FEATURE:
> >> +		return FWCTL_RPC_DEBUG_WRITE_FULL;
> >> +	default:
> >> +		return -EINVAL;
> >> +	}
> >> +}
> >> +
> >> +static size_t hw_op_size(u16 opcode)
> >> +{
> >> +	switch (opcode) {
> >> +	case CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES:
> >> +		return sizeof(struct cxl_mbox_get_sup_feats_in);
> >> +	case CXL_MBOX_OPCODE_GET_FEATURE:
> >> +		return sizeof(struct cxl_mbox_get_feat_in);
> >> +	case CXL_MBOX_OPCODE_SET_FEATURE:
> >> +		return sizeof(struct cxl_mbox_set_feat_in) + sizeof(u32);
> >> +	default:
> >> +		return SIZE_MAX;
> >> +	}
> >> +}
> >> +
> >> +static void free_rpc(struct fwctl_rpc *rpc)
> >> +{
> >> +	void *in, *out;
> >> +
> >> +	in = (void *)rpc->in;
> >> +	out = (void *)rpc->out;
> >> +	free(in);
> >> +	free(out);
> >> +	free(rpc);
> >> +}
> >> +
> >> +static void *zmalloc_aligned(size_t align, size_t size)
> >> +{
> >> +	void *ptr;
> >> +	int rc;
> >> +
> >> +	rc = posix_memalign((void **)&ptr, align, size);
> >> +	if (rc)
> >> +		return NULL;
> >> +	memset(ptr, 0, size);
> >> +
> >> +	return ptr;
> >> +}
> >> +
> >> +static struct fwctl_rpc *get_prepped_command(size_t in_size, size_t out_size,
> >> +					     u16 opcode)
> >> +{
> >> +	struct fwctl_rpc_cxl_out *out;
> >> +	struct fwctl_rpc_cxl *in;
> >> +	struct fwctl_rpc *rpc;
> >> +	size_t op_size;
> >> +	int scope;
> >> +
> >> +	rpc = zmalloc_aligned(16, sizeof(*rpc));
> >> +	if (!rpc)
> >> +		return NULL;
> >> +
> >> +	in = zmalloc_aligned(16, in_size);
> >> +	if (!in)
> >> +		goto free_rpc;
> >> +
> >> +	out = zmalloc_aligned(16, out_size);
> >> +	if (!out)
> >> +		goto free_in;
> >> +
> >> +	in->opcode = opcode;
> >> +
> >> +	op_size = hw_op_size(opcode);
> >> +	if (op_size == SIZE_MAX)
> >> +		goto free_in;
> >> +
> >> +	in->op_size = op_size;
> >> +
> >> +	rpc->size = sizeof(*rpc);
> >> +	scope = get_scope(opcode);
> >> +	if (scope < 0)
> >> +		goto free_all;
> >> +
> >> +	rpc->scope = scope;
> >> +
> >> +	rpc->in_len = in_size;
> >> +	rpc->out_len = out_size;
> >> +	rpc->in = (uint64_t)(uint64_t *)in;
> >> +	rpc->out = (uint64_t)(uint64_t *)out;
> >> +
> >> +	return rpc;
> >> +
> >> +free_all:
> >> +	free(out);
> >> +free_in:
> >> +	free(in);
> >> +free_rpc:
> >> +	free(rpc);
> >> +	return NULL;
> >> +}
> >> +
> >> +static int cxl_fwctl_rpc_get_test_feature(int fd, struct test_feature *feat_ctx,
> >> +					  const uint32_t expected_data)
> >> +{
> >> +	struct cxl_mbox_get_feat_in *feat_in;
> >> +	struct fwctl_rpc_cxl_out *out;
> >> +	size_t out_size, in_size;
> >> +	struct fwctl_rpc_cxl *in;
> >> +	struct fwctl_rpc *rpc;
> >> +	uint32_t val;
> >> +	void *data;
> >> +	int rc;
> >> +
> >> +	in_size = sizeof(*in) + sizeof(*feat_in);
> >> +	out_size = sizeof(*out) + feat_ctx->get_size;
> >> +
> >> +	rpc = get_prepped_command(in_size, out_size,
> >> +				  CXL_MBOX_OPCODE_GET_FEATURE);
> >> +	if (!rpc)
> >> +		return -ENXIO;
> >> +
> >> +	in = (struct fwctl_rpc_cxl *)rpc->in;
> >> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> >> +
> >> +	feat_in = &in->get_feat_in;
> >> +	uuid_copy(feat_in->uuid, feat_ctx->uuid);
> >> +	feat_in->count = feat_ctx->get_size;
> >> +
> >> +	rc = send_command(fd, rpc, out);
> >> +	if (rc)
> >> +		goto out;
> >> +
> >> +	data = out->payload;
> >> +	val = le32toh(*(__le32 *)data);
> >> +	if (memcmp(&val, &expected_data, sizeof(val)) != 0) {
> >> +		rc = -ENXIO;
> >> +		goto out;
> >> +	}
> >> +
> >> +out:
> >> +	free_rpc(rpc);
> >> +	return rc;
> >> +}
> >> +
> >> +static int cxl_fwctl_rpc_set_test_feature(int fd, struct test_feature *feat_ctx)
> >> +{
> >> +	struct cxl_mbox_set_feat_in *feat_in;
> >> +	struct fwctl_rpc_cxl_out *out;
> >> +	size_t in_size, out_size;
> >> +	struct fwctl_rpc_cxl *in;
> >> +	struct fwctl_rpc *rpc;
> >> +	uint32_t val;
> >> +	void *data;
> >> +	int rc;
> >> +
> >> +	in_size = sizeof(*in) + sizeof(*feat_in) + sizeof(val);
> >> +	out_size = sizeof(*out) + sizeof(val);
> >> +	rpc = get_prepped_command(in_size, out_size,
> >> +				  CXL_MBOX_OPCODE_SET_FEATURE);
> >> +	if (!rpc)
> >> +		return -ENXIO;
> >> +
> >> +	in = (struct fwctl_rpc_cxl *)rpc->in;
> >> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> >> +	feat_in = &in->set_feat_in;
> >> +	uuid_copy(feat_in->uuid, feat_ctx->uuid);
> >> +	data = feat_in->feat_data;
> >> +	val = DEFAULT_TEST_DATA2;
> >> +	*(uint32_t *)data = htole32(val);
> >> +	feat_in->flags = CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER;
> >> +
> >> +	rc = send_command(fd, rpc, out);
> >> +	if (rc)
> >> +		goto out;
> >> +
> >> +	rc = cxl_fwctl_rpc_get_test_feature(fd, feat_ctx, DEFAULT_TEST_DATA2);
> >> +	if (rc) {
> >> +		fprintf(stderr, "Failed ioctl to get feature verify: %d\n", rc);
> >> +		goto out;
> >> +	}
> >> +
> >> +out:
> >> +	free_rpc(rpc);
> >> +	return rc;
> >> +}
> >> +
> >> +static int cxl_fwctl_rpc_get_supported_features(int fd, struct test_feature *feat_ctx)
> >> +{
> >> +	struct cxl_mbox_get_sup_feats_out *feat_out;
> >> +	struct cxl_mbox_get_sup_feats_in *feat_in;
> >> +	struct fwctl_rpc_cxl_out *out;
> >> +	struct cxl_feat_entry *entry;
> >> +	size_t out_size, in_size;
> >> +	struct fwctl_rpc_cxl *in;
> >> +	struct fwctl_rpc *rpc;
> >> +	int feats, rc;
> >> +
> >> +	in_size = sizeof(*in) + sizeof(*feat_in);
> >> +	out_size = sizeof(*out) + sizeof(*feat_out);
> >> +	/* First query, to get number of features w/o per feature data */
> >> +	rpc = get_prepped_command(in_size, out_size,
> >> +				  CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES);
> >> +	if (!rpc)
> >> +		return -ENXIO;
> >> +
> >> +	/* No need to fill in feat_in first go as we are passing in all 0's */
> >> +
> >> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> >> +	rc = send_command(fd, rpc, out);
> >> +	if (rc)
> >> +		goto out;
> >> +
> >> +	feat_out = &out->get_sup_feats_out;
> >> +	feats = le16toh(feat_out->supported_feats);
> >> +	if (feats != MAX_TEST_FEATURES) {
> >> +		fprintf(stderr, "Test device has greater than %d test features.\n",
> >> +			MAX_TEST_FEATURES);
> >> +		rc = -ENXIO;
> >> +		goto out;
> >> +	}
> >> +
> >> +	free_rpc(rpc);
> >> +
> >> +	/* Going second round to retrieve each feature details */
> >> +	in_size = sizeof(*in) + sizeof(*feat_in);
> >> +	out_size = sizeof(*out) + sizeof(*feat_out);
> >> +	out_size += feats * sizeof(*entry);
> >> +	rpc = get_prepped_command(in_size, out_size,
> >> +				  CXL_MBOX_OPCODE_GET_SUPPORTED_FEATURES);
> >> +	if (!rpc)
> >> +		return -ENXIO;
> >> +
> >> +	in = (struct fwctl_rpc_cxl *)rpc->in;
> >> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> >> +	feat_in = &in->get_sup_feats_in;
> >> +	feat_in->count = htole32(feats * sizeof(*entry));
> >> +
> >> +	rc = send_command(fd, rpc, out);
> >> +	if (rc)
> >> +		goto out;
> >> +
> >> +	feat_out = &out->get_sup_feats_out;
> >> +	feats = le16toh(feat_out->supported_feats);
> >> +	if (feats != MAX_TEST_FEATURES) {
> >> +		fprintf(stderr, "Test device has greater than %u test features.\n",
> >> +			MAX_TEST_FEATURES);
> >> +		rc = -ENXIO;
> >> +		goto out;
> >> +	}
> >> +
> >> +	if (le16toh(feat_out->num_entries) != MAX_TEST_FEATURES) {
> >> +		fprintf(stderr, "Test device did not return expected entries. %u\n",
> >> +			le16toh(feat_out->num_entries));
> >> +		rc = -ENXIO;
> >> +		goto out;
> >> +	}
> >> +
> >> +	entry = &feat_out->ents[0];
> >> +	if (uuid_compare(test_uuid, entry->uuid) != 0) {
> >> +		fprintf(stderr, "Test device did not export expected test feature.\n");
> >> +		rc = -ENXIO;
> >> +		goto out;
> >> +	}
> >> +
> >> +	if (le16toh(entry->get_feat_size) != GET_FEAT_SIZE ||
> >> +	    le16toh(entry->set_feat_size) != SET_FEAT_SIZE) {
> >> +		fprintf(stderr, "Test device feature in/out size incorrect.\n");
> >> +		rc = -ENXIO;
> >> +		goto out;
> >> +	}
> >> +
> >> +	if (le16toh(entry->effects) != EFFECTS_MASK) {
> >> +		fprintf(stderr, "Test device set effects incorrect\n");
> >> +		rc = -ENXIO;
> >> +		goto out;
> >> +	}
> >> +
> >> +	uuid_copy(feat_ctx->uuid, entry->uuid);
> >> +	feat_ctx->get_size = le16toh(entry->get_feat_size);
> >> +	feat_ctx->set_size = le16toh(entry->set_feat_size);
> >> +
> >> +out:
> >> +	free_rpc(rpc);
> >> +	return rc;
> >> +}
> >> +
> >> +static int test_fwctl_features(struct cxl_memdev *memdev)
> >> +{
> >> +	struct test_feature feat_ctx;
> >> +	unsigned int major, minor;
> >> +	struct cxl_fwctl *fwctl;
> >> +	int fd, rc;
> >> +	char path[256];
> >> +
> >> +	fwctl = cxl_memdev_get_fwctl(memdev);
> >> +	if (!fwctl)
> >> +		return -ENODEV;
> >> +
> >> +	major = cxl_fwctl_get_major(fwctl);
> >> +	minor = cxl_fwctl_get_minor(fwctl);
> >> +
> >> +	if (!major && !minor)
> >> +		return -ENODEV;
> >> +
> >> +	sprintf(path, "/dev/char/%d:%d", major, minor);
> >> +
> >> +	fd = open(path, O_RDONLY, 0644);
> >> +	if (fd < 0) {
> >> +		fprintf(stderr, "Failed to open: %d\n", -errno);
> >> +		return -errno;
> >> +	}
> >> +
> >> +	rc = cxl_fwctl_rpc_get_supported_features(fd, &feat_ctx);
> >> +	if (rc) {
> >> +		fprintf(stderr, "Failed ioctl to get supported features: %d\n", rc);
> >> +		goto out;
> >> +	}
> >> +
> >> +	rc = cxl_fwctl_rpc_get_test_feature(fd, &feat_ctx, DEFAULT_TEST_DATA);
> >> +	if (rc) {
> >> +		fprintf(stderr, "Failed ioctl to get feature: %d\n", rc);
> >> +		goto out;
> >> +	}
> >> +
> >> +	rc = cxl_fwctl_rpc_set_test_feature(fd, &feat_ctx);
> >> +	if (rc) {
> >> +		fprintf(stderr, "Failed ioctl to set feature: %d\n", rc);
> >> +		goto out;
> >> +	}
> >> +
> >> +out:
> >> +	close(fd);
> >> +	return rc;
> >> +}
> >> +
> >> +static int test_fwctl(struct cxl_ctx *ctx, struct cxl_bus *bus)
> >> +{
> >> +	struct cxl_memdev *memdev;
> >> +
> >> +	cxl_memdev_foreach(ctx, memdev) {
> >> +		if (cxl_memdev_get_bus(memdev) != bus)
> >> +			continue;
> >> +		return test_fwctl_features(memdev);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +int main(int argc, char *argv[])
> >> +{
> >> +	struct cxl_ctx *ctx;
> >> +	struct cxl_bus *bus;
> >> +	int rc;
> >> +
> >> +	rc = cxl_new(&ctx);
> >> +	if (rc < 0)
> >> +		return rc;
> >> +
> >> +	cxl_set_log_priority(ctx, LOG_DEBUG);
> >> +
> >> +	bus = cxl_bus_get_by_provider(ctx, provider);
> >> +	if (!bus) {
> >> +		fprintf(stderr, "%s: unable to find bus (%s)\n",
> >> +			argv[0], provider);
> >> +		rc = -EINVAL;
> >> +		goto out;
> >> +	}
> >> +
> >> +	rc = test_fwctl(ctx, bus);
> >> +
> >> +out:
> >> +	cxl_unref(ctx);
> >> +	return rc;
> >> +}
> >> diff --git a/test/meson.build b/test/meson.build
> >> index 2fd7df5211dd..93b1d78671ef 100644
> >> --- a/test/meson.build
> >> +++ b/test/meson.build
> >> @@ -17,6 +17,13 @@ ndctl_deps = libndctl_deps + [
> >>    versiondep,
> >>  ]
> >>  
> >> +libcxl_deps = [
> >> +  cxl_dep,
> >> +  ndctl_dep,
> >> +  uuid,
> >> +  kmod,
> >> +]
> >> +
> >>  libndctl = executable('libndctl', testcore + [ 'libndctl.c'],
> >>    dependencies : libndctl_deps,
> >>    include_directories : root_inc,
> >> @@ -235,6 +242,18 @@ if get_option('keyutils').enabled()
> >>    ]
> >>  endif
> >>  
> >> +uuid_dep = dependency('uuid', required: false)
> >> +if get_option('fwctl').enabled() and uuid_dep.found()
> >> +  fwctl = executable('fwctl', 'fwctl.c',
> >> +    dependencies : libcxl_deps,
> >> +    include_directories : root_inc,
> >> +  )
> >> +  cxl_features = find_program('cxl-features.sh')
> >> +  tests += [
> >> +    [ 'cxl-features.sh',        cxl_features,       'cxl'   ],
> >> +  ]
> >> +endif
> >> +
> >>  foreach t : tests
> >>    test(t[0], t[1],
> >>      is_parallel : false,
> >> -- 
> >> 2.49.0
> >>
> >>
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-05-22  0:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-19 20:00 [NDCTL PATCH v7 0/4] ndctl: Add support and test for CXL Features support Dave Jiang
2025-05-19 20:00 ` [NDCTL PATCH v7 1/4] cxl: Add cxl_bus_get_by_provider() Dave Jiang
2025-05-19 20:00 ` [NDCTL PATCH v7 2/4] cxl: Enumerate major/minor of FWCTL char device Dave Jiang
2025-05-20 19:11   ` Alison Schofield
2025-05-19 20:00 ` [NDCTL PATCH v7 3/4] ndctl: Add features.h from kernel UAPI Dave Jiang
2025-05-20 19:08   ` Alison Schofield
2025-05-19 20:00 ` [NDCTL PATCH v7 4/4] cxl/test: Add test for cxl features device Dave Jiang
2025-05-20 19:25   ` Alison Schofield
2025-05-21 23:22     ` Dave Jiang
2025-05-22  0:18       ` Alison Schofield

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox