[NDCTL PATCH v5 0/3] ndctl: Add support and test for CXL Features support

[NDCTL PATCH v5 0/3] ndctl: Add support and test for CXL Features support

[Dave Jiang]

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 (3):
      cxl: Add cxl_bus_get_by_provider()
      cxl: Enumerate major/minor of FWCTL char device
      cxl/test: Add test for cxl features device

 Documentation/cxl/lib/libcxl.txt |  23 ++++
 cxl/lib/libcxl.c                 |  89 ++++++++++++
 cxl/lib/libcxl.sym               |   8 ++
 cxl/lib/private.h                |   8 ++
 cxl/libcxl.h                     |   7 +
 test/cxl-features-control.c      | 439 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/cxl-features.sh             |  31 +++++
 test/cxl-topology.sh             |   4 +
 test/meson.build                 |  45 ++++++
 9 files changed, 654 insertions(+)

[NDCTL PATCH v5 1/3] cxl: Add cxl_bus_get_by_provider()

[Dave Jiang]

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

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v5:
- Add documentation for API. (Alison)
---
 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

[NDCTL PATCH v5 2/3] cxl: Enumerate major/minor of FWCTL char device

[Dave Jiang]

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.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v5:
- Add documentation. (Alison)
- Alloc path on stack. (Alison)
- Deal with out of entry and no match case. (Alison)
- Make fwctl operations part of 'struct cxl_fwctl' (Dan)
---
 Documentation/cxl/lib/libcxl.txt | 21 +++++++++
 cxl/lib/libcxl.c                 | 78 ++++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym               |  3 ++
 cxl/lib/private.h                |  8 ++++
 cxl/libcxl.h                     |  5 ++
 5 files changed, 115 insertions(+)

diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 25ff406c2920..1e00e2dd1abc 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,9 @@ 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 and
+Firmware Contrl (FWCTL) is also enabled.
+
 === MEMDEV: Attributes
 ----
 int cxl_memdev_get_id(struct cxl_memdev *memdev);
@@ -185,6 +189,23 @@ 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 retrieving attributes related to 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/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index bab7343e8a4a..be54db1b9bb9 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,64 @@ static int add_cxl_memdev_fwl(struct cxl_memdev *memdev,
 	return -ENOMEM;
 }
 
+static const char fwctl_prefix[] = "fwctl";
+static int get_feature_chardev(const char *base, char *chardev_path)
+{
+	char path[CXL_PATH_MAX];
+	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)
+{
+	struct cxl_fwctl *fwctl;
+	char path[CXL_PATH_MAX];
+	struct stat st;
+	int rc;
+
+	rc = get_feature_chardev(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);
@@ -1279,6 +1338,10 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 	memdev->major = major(st.st_rdev);
 	memdev->minor = minor(st.st_rdev);
 
+	/* If this fails, no Features support. */
+	if (memdev_add_fwctl(memdev, cxlmem_base))
+		dbg(ctx, "%s: no Features support.\n", devname);
+
 	sprintf(path, "%s/pmem/size", cxlmem_base);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		memdev->pmem_size = strtoull(buf, NULL, 0);
@@ -1515,6 +1578,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..8bb3308cc036 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -13,6 +13,8 @@
 
 #define CXL_EXPORT __attribute__ ((visibility("default")))
 
+#define CXL_PATH_MAX	512
+
 struct cxl_pmem {
 	int id;
 	void *dev_buf;
@@ -34,6 +36,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 +63,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)
-- 
2.49.0

[NDCTL PATCH v5 3/3] cxl/test: Add test for cxl features device

[Dave Jiang]

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.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v5:
- Make command prep common. (Alison)
- Rename fwctl.c to cxl-features-control.c.  (Alison)
- Update test code to retrieve cxl_fwctl object.
- Create helper for aligned allocation and zeroing. (Alison)
- Correct check of open() return value. (Alison)
---
 test/cxl-features-control.c | 439 ++++++++++++++++++++++++++++++++++++
 test/cxl-features.sh        |  31 +++
 test/cxl-topology.sh        |   4 +
 test/meson.build            |  45 ++++
 4 files changed, 519 insertions(+)
 create mode 100644 test/cxl-features-control.c
 create mode 100755 test/cxl-features.sh

diff --git a/test/cxl-features-control.c b/test/cxl-features-control.c
new file mode 100644
index 000000000000..9f2af0258a0e
--- /dev/null
+++ b/test/cxl-features-control.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 <cxl/features.h>
+#include <fwctl/fwctl.h>
+#include <fwctl/cxl.h>
+#include <linux/uuid.h>
+#include <uuid/uuid.h>
+#include <util/bitmap.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/cxl-features.sh b/test/cxl-features.sh
new file mode 100755
index 000000000000..e648242a4a01
--- /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"/cxl-features-control
+
+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/cxl-topology.sh b/test/cxl-topology.sh
index e8b9f56543b5..90b9c98273db 100644
--- a/test/cxl-topology.sh
+++ b/test/cxl-topology.sh
@@ -172,11 +172,15 @@ done
 # validate host bridge tear down for the first 2 bridges
 for b in ${bridge[0]} ${bridge[1]}
 do
+	echo "XXX SHELL disable port $b to validate teardown" > /dev/kmsg
+
 	$CXL disable-port $b -f
 	json=$($CXL list -M -i -p $b)
 	count=$(jq "map(select(.state == \"disabled\")) | length" <<< $json)
 	((count == 4)) || err "$LINENO"
 
+	echo "XXX SHELL enable port $b to validate teardown" > /dev/kmsg
+
 	$CXL enable-port $b -m
 	json=$($CXL list -M -p $b)
 	count=$(jq "length" <<< $json)
diff --git a/test/meson.build b/test/meson.build
index d871e28e17ce..89e73c2575f6 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,
@@ -130,6 +137,33 @@ revoke_devmem = executable('revoke_devmem', testcore + [
   include_directories : root_inc,
 )
 
+fs = import('fs')
+
+feature_hdrs = [
+  '/usr/include/cxl/features.h',
+  '/usr/include/fwctl/cxl.h',
+  '/usr/include/fwctl/fwctl.h',
+]
+
+feat_hdrs_exist = true
+foreach file : feature_hdrs
+  if not fs.exists(file)
+    feat_hdrs_exist = false
+    break
+  endif
+endforeach
+
+if feat_hdrs_exist
+    features = executable('cxl-features-control', [
+        'cxl-features-control.c',
+    ],
+    include_directories : root_inc,
+    dependencies : libcxl_deps,
+    )
+else
+    features = []
+endif
+
 mmap = executable('mmap', 'mmap.c',)
 
 create = find_program('create.sh')
@@ -162,6 +196,10 @@ cxl_destroy_region = find_program('cxl-destroy-region.sh')
 cxl_qos_class = find_program('cxl-qos-class.sh')
 cxl_poison = find_program('cxl-poison.sh')
 
+if feat_hdrs_exist
+  cxl_features = find_program('cxl-features.sh')
+endif
+
 tests = [
   [ 'libndctl',               libndctl,		  'ndctl' ],
   [ 'dsm-fail',               dsm_fail,	      	  'ndctl' ],
@@ -196,6 +234,12 @@ tests = [
   [ 'cxl-poison.sh',          cxl_poison,         'cxl'   ],
 ]
 
+if feat_hdrs_exist
+  tests += [
+    [ 'cxl-features.sh',        cxl_features,       'cxl'   ],
+  ]
+endif
+
 if get_option('destructive').enabled()
   sub_section = find_program('sub-section.sh')
   dax_ext4 = find_program('dax-ext4.sh')
@@ -249,6 +293,7 @@ foreach t : tests
       daxdev_errors,
       dax_dev,
       mmap,
+      features,
     ],
     suite: t[2],
     timeout : 600,
-- 
2.49.0

Re: [NDCTL PATCH v5 2/3] cxl: Enumerate major/minor of FWCTL char device

[Alison Schofield]

On Fri, Apr 11, 2025 at 11:47:36AM -0700, Dave Jiang wrote:

big snip...

> +	char path[CXL_PATH_MAX];

A bit of minutiae, not directly related to your patch-

I see:
ndctl keys code, (ndctl/key.s,load-keys) simply use the PATH_MAX which
I believe comes from limits.h

The rest of ndctl is doing the calloc'ing, like you originally had it.
(~/git/ndctl$ git grep char | grep alloc)

Is it not safe, or is it wasterful, to make all use PATH_MAX?
Would it be safe-er to make all use a NDCTL_PATH_MAX?

This may be worth tidying up but not clear which way to go.

snip

Re: [NDCTL PATCH v5 2/3] cxl: Enumerate major/minor of FWCTL char device

[Dave Jiang]

On 4/11/25 1:53 PM, Alison Schofield wrote:
> On Fri, Apr 11, 2025 at 11:47:36AM -0700, Dave Jiang wrote:
> 
> big snip...
> 
>> +	char path[CXL_PATH_MAX];
> 
> A bit of minutiae, not directly related to your patch-
> 
> I see:
> ndctl keys code, (ndctl/key.s,load-keys) simply use the PATH_MAX which
> I believe comes from limits.h
> 
> The rest of ndctl is doing the calloc'ing, like you originally had it.
> (~/git/ndctl$ git grep char | grep alloc)

I get why the current code does the strlen(PATH) + N. It's just adding a little more to the existing discovered path since it is looking at specific sysfs paths. So in these cases, PATH_MAX may not be necessary.

DJ

> 
> Is it not safe, or is it wasterful, to make all use PATH_MAX?
> Would it be safe-er to make all use a NDCTL_PATH_MAX?
> 
> This may be worth tidying up but not clear which way to go.
> 
> snip
> 

Re: [NDCTL PATCH v5 0/3] ndctl: Add support and test for CXL Features support

[Alison Schofield]

On Fri, Apr 11, 2025 at 11:47:34AM -0700, Dave Jiang wrote:
> 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.

Thanks for the updates Dave!

For the series:
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Tested-by: Alison Schofield <alison.schofield@intel.com>

and now I will see if my b4-shazam-ness waterfalls my tags.


> 
> 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 (3):
>       cxl: Add cxl_bus_get_by_provider()
>       cxl: Enumerate major/minor of FWCTL char device
>       cxl/test: Add test for cxl features device
> 
>  Documentation/cxl/lib/libcxl.txt |  23 ++++
>  cxl/lib/libcxl.c                 |  89 ++++++++++++
>  cxl/lib/libcxl.sym               |   8 ++
>  cxl/lib/private.h                |   8 ++
>  cxl/libcxl.h                     |   7 +
>  test/cxl-features-control.c      | 439 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  test/cxl-features.sh             |  31 +++++
>  test/cxl-topology.sh             |   4 +
>  test/meson.build                 |  45 ++++++
>  9 files changed, 654 insertions(+)
> 
> 

Re: [NDCTL PATCH v5 2/3] cxl: Enumerate major/minor of FWCTL char device

[Dan Williams]

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.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v5:
> - Add documentation. (Alison)
> - Alloc path on stack. (Alison)
> - Deal with out of entry and no match case. (Alison)
> - Make fwctl operations part of 'struct cxl_fwctl' (Dan)
> ---
>  Documentation/cxl/lib/libcxl.txt | 21 +++++++++
>  cxl/lib/libcxl.c                 | 78 ++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym               |  3 ++
>  cxl/lib/private.h                |  8 ++++
>  cxl/libcxl.h                     |  5 ++
>  5 files changed, 115 insertions(+)
> 
> diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
> index 25ff406c2920..1e00e2dd1abc 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,9 @@ 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 and
> +Firmware Contrl (FWCTL) is also enabled.

s/Contrl/Control/

Also, do you mean CONFIG_CXL_FEATURES instead of "FWCTL", because
CONFIG_FWCTL is not sufficient.

> +
>  === MEMDEV: Attributes
>  ----
>  int cxl_memdev_get_id(struct cxl_memdev *memdev);
> @@ -185,6 +189,23 @@ 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 retrieving attributes related to the
> +CXL FWCTL character device that is a child of the memdev.

I assume any other fwctl helpers that might get added in the future
would use the object so I would give yourself some wiggle room for
cxl_fwctl to be used for more than just conveying chardev major / minor.

> +
> +=== 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/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index bab7343e8a4a..be54db1b9bb9 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,64 @@ static int add_cxl_memdev_fwl(struct cxl_memdev *memdev,
>  	return -ENOMEM;
>  }
>  
> +static const char fwctl_prefix[] = "fwctl";
> +static int get_feature_chardev(const char *base, char *chardev_path)
> +{
> +	char path[CXL_PATH_MAX];

This CXL_PATH_MAX approach stands out as different than all the other
path buffer management in the rest of the implementation.

In all other paths ->dev_buf has the scratch space for printing buffers.

It's not broken, but it's also odd for one off functions to switch
schemes.

Re: [NDCTL PATCH v5 3/3] cxl/test: Add test for cxl features device

[Dan Williams]

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.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v5:
> - Make command prep common. (Alison)
> - Rename fwctl.c to cxl-features-control.c.  (Alison)
> - Update test code to retrieve cxl_fwctl object.
> - Create helper for aligned allocation and zeroing. (Alison)
> - Correct check of open() return value. (Alison)
> ---
>  test/cxl-features-control.c | 439 ++++++++++++++++++++++++++++++++++++
>  test/cxl-features.sh        |  31 +++
>  test/cxl-topology.sh        |   4 +
>  test/meson.build            |  45 ++++
>  4 files changed, 519 insertions(+)
>  create mode 100644 test/cxl-features-control.c
>  create mode 100755 test/cxl-features.sh
> 
> diff --git a/test/cxl-features-control.c b/test/cxl-features-control.c

Not that this filename matters, but if someone was looking for FWCTL
examples with CXL filename does not shout "look here for
fwctl examples".

[..]
> diff --git a/test/cxl-features.sh b/test/cxl-features.sh
> new file mode 100755
> index 000000000000..e648242a4a01
> --- /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"/cxl-features-control
> +
> +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/cxl-topology.sh b/test/cxl-topology.sh
> index e8b9f56543b5..90b9c98273db 100644
> --- a/test/cxl-topology.sh
> +++ b/test/cxl-topology.sh
> @@ -172,11 +172,15 @@ done
>  # validate host bridge tear down for the first 2 bridges
>  for b in ${bridge[0]} ${bridge[1]}
>  do
> +	echo "XXX SHELL disable port $b to validate teardown" > /dev/kmsg
> +
>  	$CXL disable-port $b -f
>  	json=$($CXL list -M -i -p $b)
>  	count=$(jq "map(select(.state == \"disabled\")) | length" <<< $json)
>  	((count == 4)) || err "$LINENO"
>  
> +	echo "XXX SHELL enable port $b to validate teardown" > /dev/kmsg
> +

Are these unrelated changes? I do not see what new cxl-topology.sh print
statements has to do with this new fwctl test?

>  	$CXL enable-port $b -m
>  	json=$($CXL list -M -p $b)
>  	count=$(jq "length" <<< $json)
> diff --git a/test/meson.build b/test/meson.build
> index d871e28e17ce..89e73c2575f6 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,
> @@ -130,6 +137,33 @@ revoke_devmem = executable('revoke_devmem', testcore + [
>    include_directories : root_inc,
>  )
>  
> +fs = import('fs')
> +
> +feature_hdrs = [
> +  '/usr/include/cxl/features.h',
> +  '/usr/include/fwctl/cxl.h',
> +  '/usr/include/fwctl/fwctl.h',
> +]
> +
> +feat_hdrs_exist = true
> +foreach file : feature_hdrs
> +  if not fs.exists(file)
> +    feat_hdrs_exist = false
> +    break
> +  endif
> +endforeach
> +
> +if feat_hdrs_exist
> +    features = executable('cxl-features-control', [
> +        'cxl-features-control.c',
> +    ],
> +    include_directories : root_inc,
> +    dependencies : libcxl_deps,
> +    )
> +else
> +    features = []
> +endif

This does not look like typical meson conditional functionality. I would
expect is to match what happens for conditional "keyutils"
functionality.  A new "if get_option('fwctl').enabled()" option that
gates other dependencies.

You can add Acked-by for the series after addressing above comments.

Re: [NDCTL PATCH v5 3/3] cxl/test: Add test for cxl features device

[Dan Williams]

Dan Williams wrote:
[..]
> This does not look like typical meson conditional functionality. I would
> expect is to match what happens for conditional "keyutils"
> functionality.  A new "if get_option('fwctl').enabled()" option that
> gates other dependencies.

Recall that the include/uapi/linux/ndctl.h dependency is vendored in
ndctl/ndctl.h. Keep the same policy for all uapi dependencies. I.e. do
not make the ndctl need to reacy to whatever random version of a uapi
header exists ships in the build environment. Just vendor latest kernel
headers like include/uapi/cxl/features.h in the project directly.

Make the option to enable fwctl support for a meson option.