Linux CXL
 help / color / mirror / Atom feed
* [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl
@ 2025-02-18 22:54 Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 01/14] cxl: Enumerate feature commands Dave Jiang
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed

v5:
- Add devm action to fwctl. (Dan)
- Fix return of NULL instead of ERR_PTR() for get features. (Ming)
- Make cxl_get/set_feature() internal to cxl/core (Dan)
- Add missing NULL check for cxl_set_feature() (Dan)
- Remove cxlmd back pointer from fwctl. (Dan)
- Add missing CONFIG_FWCTL dependency to Kconfig.
- Fix index of for loop for fwctl get features. (Ming)
- See individual patches for detailed changes from v4.

v4:
- Rebase to FWCTL series v4 based on v6.14-rc1
- Merge patch 1/16 from v3 into cxl/next ahead. (Jonathan)
- Move everything to core/features.c and behind CONFIG_CXL_FEATURES. (Dan)
- Split the registration of features into 2 parts. Do the registration in
  PCI probe. One before memdev enumeration to enable kernel feature support,
  and one after memdev enumeration to support FWCTL. (Dan)
- Fix incrememt of pointer after copying feature data. (Ming)
- Add check for no immediate change effect and no reset change effects. (Dan, Jonathan)
- Remove IDs and use opcodes directly. (Dan)
- See individual patches for detailed changes from v3.

v3:
- Rearrange code and shift code forward to reduce diffs. (Jonathan)
- Use struct_size() in appropriate locations. (Jonathan)
- Remove usage of __weak and refactor accordingly. (Jason)
- Return NULL for allocation functions. (Jonathan)
- See individual patches for detailed changes from v2.

v2:
- Drop features device and driver. Move features enabling to cxl_memdev. (Dan)
- Drop sysfs attribute of number of features. (Dan)
- Drop moving of include/uapi/linux/cxl_mem.h. (Dan)
- Set get driver info ioctl to return reserved 32bit. Set behavior of issue
  ioctl successful as indicating Feature commands supported.
- Set 'Set Feature Size' to 0 for kernel exclusive Features.
- See individual patches for detailed changes from v1.
- Hide FWCTL bits behind CONFIG_CXL_FWCTL. (Dan)

v1:
- Create a CXL features driver to handle all feature command related bits
  including FWCTL support. (Dan)
- Tested against CXL CLI unit tests for FWCTL.
- See individual patches for detailed changes from RFC v2.

RFC v2:
- Dropped 1/13 and 2/13 from previous version. Merged upstream already.
- Combined changes from Shiju for "get supported features"
- Addressed comments from Jonathan and Jason
- See specific changes in individual patch revision history
- Added hardware command info to FWCTL
- Added filtering to set feature command
- Added documentation

This series add support for CXL feature commands using the FWCTL framework [1].
The code has been tested with CXL CLI unit tests for FWCTL.

Patches 1-6 are shared with EDAC series [1] and there will be an immutable branch
once accepted post review.

CXL Features support is added behind the CXL mem driver. The 3 mailbox commands
"Get Supported Features", "Get Feature", and "Set Feature" are implemented. The
"Get" commands are under the FWCTL_RPC_CONFIGURATION policy, the "Set" command
checks the policy depending on the set effects of the feature defined by the
device via the Feature set effects field. All mailbox commands for CXL provides
an effects table that describes the effects of a command when performed on the
device.  For CXL features, there is also an effects field that describes the
effects a feature set operation has on the device per feature. The security
policy is checked against this feature specific effects field. 

The code is based off of v4 of FWCTL series [2] posted by Jason and rebased on top of
v6.14-rc1 plus 1 patch in
cxl/next 43ca2463df9d ("cxl: Refactor user ioctl command path from mds to mailbox").
A kernel branch [3] is provided for convience of testing and review.

[1]: https://lore.kernel.org/linux-cxl/20250106121017.1620-1-shiju.jose@huawei.com/T/#t
[2]: https://lore.kernel.org/linux-cxl/0-v3-960f17f90f17+516-fwctl_jgg@nvidia.com/#r
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl/fwctl

---
Dave Jiang (12):
      cxl: Enumerate feature commands
      cxl: Add Get Supported Features command for kernel usage
      cxl/test: Add Get Supported Features mailbox command support
      cxl: Setup exclusive CXL features that are reserved for the kernel
      cxl: Add FWCTL support to CXL
      cxl: Move cxl feature command structs to user header
      cxl: Add support for fwctl RPC command to enable CXL feature commands
      cxl: Add support to handle user feature commands for get feature
      cxl: Add support to handle user feature commands for set feature
      cxl/test: Add Get Feature support to cxl_test
      cxl/test: Add Set Feature support to cxl_test
      fwctl/cxl: Add documentation to FWCTL CXL

Shiju Jose (2):
      cxl/mbox: Add GET_FEATURE mailbox command
      cxl/mbox: Add SET_FEATURE mailbox command

 Documentation/userspace-api/fwctl/fwctl-cxl.rst | 144 +++++++++
 Documentation/userspace-api/fwctl/index.rst     |   1 +
 MAINTAINERS                                     |   1 +
 drivers/cxl/Kconfig                             |  12 +
 drivers/cxl/core/Makefile                       |   1 +
 drivers/cxl/core/core.h                         |  11 +
 drivers/cxl/core/features.c                     | 710 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/core/mbox.c                         |  36 ++-
 drivers/cxl/cxlmem.h                            |   7 +
 drivers/cxl/pci.c                               |   8 +
 include/cxl/features.h                          |  91 ++++++
 include/cxl/mailbox.h                           |   3 +
 include/uapi/cxl/features.h                     | 128 ++++++++
 include/uapi/fwctl/cxl.h                        |  63 ++++
 include/uapi/fwctl/fwctl.h                      |   1 +
 tools/testing/cxl/Kbuild                        |   1 +
 tools/testing/cxl/test/mem.c                    | 185 ++++++++++++
 17 files changed, 1402 insertions(+), 1 deletion(-)

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

* [PATCH v6 01/14] cxl: Enumerate feature commands
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-20 15:37   ` Shiju Jose
  2025-02-18 22:54 ` [PATCH v6 02/14] cxl: Add Get Supported Features command for kernel usage Dave Jiang
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

Add feature commands enumeration code in order to detect and enumerate
the 3 feature related commands "get supported features", "get feature",
and "set feature". The enumeration will help determine whether the driver
can issue any of the 3 commands to the device.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/mbox.c | 36 +++++++++++++++++++++++++++++++++++-
 drivers/cxl/cxlmem.h    |  3 +++
 include/cxl/features.h  | 13 +++++++++++++
 include/cxl/mailbox.h   |  3 +++
 4 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 include/cxl/features.h

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 9c1b9e353e3e..78c5346e3e89 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -706,6 +706,35 @@ static int cxl_xfer_log(struct cxl_memdev_state *mds, uuid_t *uuid,
 	return 0;
 }
 
+static int check_features_opcodes(u16 opcode, int *ro_cmds, int *wr_cmds)
+{
+	switch (opcode) {
+	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
+	case CXL_MBOX_OP_GET_FEATURE:
+		(*ro_cmds)++;
+		return 1;
+	case CXL_MBOX_OP_SET_FEATURE:
+		(*wr_cmds)++;
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+/* 'Get Supported Features' and 'Get Feature' */
+#define MAX_FEATURES_READ_CMDS	2
+static void set_features_cap(struct cxl_mailbox *cxl_mbox,
+			     int ro_cmds, int wr_cmds)
+{
+	/* Setting up Features capability while walking the CEL */
+	if (ro_cmds == MAX_FEATURES_READ_CMDS) {
+		if (wr_cmds)
+			cxl_mbox->feat_cap = CXL_FEATURES_RW;
+		else
+			cxl_mbox->feat_cap = CXL_FEATURES_RO;
+	}
+}
+
 /**
  * cxl_walk_cel() - Walk through the Command Effects Log.
  * @mds: The driver data for the operation
@@ -721,7 +750,7 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
 	struct cxl_cel_entry *cel_entry;
 	const int cel_entries = size / sizeof(*cel_entry);
 	struct device *dev = mds->cxlds.dev;
-	int i;
+	int i, ro_cmds = 0, wr_cmds = 0;
 
 	cel_entry = (struct cxl_cel_entry *) cel;
 
@@ -735,6 +764,9 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
 			enabled++;
 		}
 
+		enabled += check_features_opcodes(opcode, &ro_cmds,
+						  &wr_cmds);
+
 		if (cxl_is_poison_command(opcode)) {
 			cxl_set_poison_cmd_enabled(&mds->poison, opcode);
 			enabled++;
@@ -748,6 +780,8 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
 		dev_dbg(dev, "Opcode 0x%04x %s\n", opcode,
 			enabled ? "enabled" : "unsupported by driver");
 	}
+
+	set_features_cap(cxl_mbox, ro_cmds, wr_cmds);
 }
 
 static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_memdev_state *mds)
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index a0a49809cd76..55c55685cb39 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -490,6 +490,9 @@ enum cxl_opcode {
 	CXL_MBOX_OP_GET_LOG_CAPS	= 0x0402,
 	CXL_MBOX_OP_CLEAR_LOG           = 0x0403,
 	CXL_MBOX_OP_GET_SUP_LOG_SUBLIST = 0x0405,
+	CXL_MBOX_OP_GET_SUPPORTED_FEATURES	= 0x0500,
+	CXL_MBOX_OP_GET_FEATURE		= 0x0501,
+	CXL_MBOX_OP_SET_FEATURE		= 0x0502,
 	CXL_MBOX_OP_IDENTIFY		= 0x4000,
 	CXL_MBOX_OP_GET_PARTITION_INFO	= 0x4100,
 	CXL_MBOX_OP_SET_PARTITION_INFO	= 0x4101,
diff --git a/include/cxl/features.h b/include/cxl/features.h
new file mode 100644
index 000000000000..357d3acf8429
--- /dev/null
+++ b/include/cxl/features.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright(c) 2024-2025 Intel Corporation. */
+#ifndef __CXL_FEATURES_H__
+#define __CXL_FEATURES_H__
+
+/* Feature commands capability supported by a device */
+enum cxl_features_capability {
+	CXL_FEATURES_NONE = 0,
+	CXL_FEATURES_RO,
+	CXL_FEATURES_RW,
+};
+
+#endif
diff --git a/include/cxl/mailbox.h b/include/cxl/mailbox.h
index cc894f07a435..c4e99e2e3a9d 100644
--- a/include/cxl/mailbox.h
+++ b/include/cxl/mailbox.h
@@ -3,6 +3,7 @@
 #ifndef __CXL_MBOX_H__
 #define __CXL_MBOX_H__
 #include <linux/rcuwait.h>
+#include <cxl/features.h>
 #include <uapi/linux/cxl_mem.h>
 
 /**
@@ -51,6 +52,7 @@ struct cxl_mbox_cmd {
  * @mbox_mutex: mutex protects device mailbox and firmware
  * @mbox_wait: rcuwait for mailbox
  * @mbox_send: @dev specific transport for transmitting mailbox commands
+ * @feat_cap: Features capability
  */
 struct cxl_mailbox {
 	struct device *host;
@@ -60,6 +62,7 @@ struct cxl_mailbox {
 	struct mutex mbox_mutex; /* lock to protect mailbox context */
 	struct rcuwait mbox_wait;
 	int (*mbox_send)(struct cxl_mailbox *cxl_mbox, struct cxl_mbox_cmd *cmd);
+	enum cxl_features_capability feat_cap;
 };
 
 int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host);
-- 
2.48.1


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

* [PATCH v6 02/14] cxl: Add Get Supported Features command for kernel usage
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 01/14] cxl: Enumerate feature commands Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-20 15:37   ` Shiju Jose
  2025-02-18 22:54 ` [PATCH v6 03/14] cxl/test: Add Get Supported Features mailbox command support Dave Jiang
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

CXL spec r3.2 8.2.9.6.1 Get Supported Features (Opcode 0500h)
The command retrieve the list of supported device-specific features
(identified by UUID) and general information about each Feature.

The driver will retrieve the Feature entries in order to make checks and
provide information for the Get Feature and Set Feature command. One of
the main piece of information retrieved are the effects a Set Feature
command would have for a particular feature. The retrieved Feature
entries are stored in the cxl_mailbox context.

The setup of Features is initiated via devm_cxl_setup_features() during the
pci probe function before the cxl_memdev is enumerated.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v6:
- Remove @cap comment from 'cxl_features_state' (Shiju)
---
 drivers/cxl/Kconfig          |  11 +++
 drivers/cxl/core/Makefile    |   1 +
 drivers/cxl/core/features.c  | 175 +++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlmem.h         |   4 +
 drivers/cxl/pci.c            |   4 +
 include/cxl/features.h       |  87 +++++++++++++++++
 tools/testing/cxl/Kbuild     |   1 +
 tools/testing/cxl/test/mem.c |   4 +
 8 files changed, 287 insertions(+)
 create mode 100644 drivers/cxl/core/features.c

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 876469e23f7a..ad2e796e4ac6 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -102,6 +102,17 @@ config CXL_MEM
 
 	  If unsure say 'm'.
 
+config CXL_FEATURES
+	bool "CXL: Features"
+	depends on CXL_PCI
+	help
+	  Enable support for CXL Features. A CXL device that includes a mailbox
+	  supports commands that allows listing, getting, and setting of
+	  optionally defined features such as memory sparing or post package
+	  sparing. Vendors may define custom features for the device.
+
+	  If unsure say 'n'
+
 config CXL_PORT
 	default CXL_BUS
 	tristate
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 9259bcc6773c..b0bfbd9eac9b 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -16,3 +16,4 @@ cxl_core-y += pmu.o
 cxl_core-y += cdat.o
 cxl_core-$(CONFIG_TRACING) += trace.o
 cxl_core-$(CONFIG_CXL_REGION) += region.o
+cxl_core-$(CONFIG_CXL_FEATURES) += features.o
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
new file mode 100644
index 000000000000..f2898a56bd63
--- /dev/null
+++ b/drivers/cxl/core/features.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2024-2025 Intel Corporation. All rights reserved. */
+#include <linux/device.h>
+#include <cxl/mailbox.h>
+#include <cxl/features.h>
+#include "cxl.h"
+#include "cxlmem.h"
+
+inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds)
+{
+	return cxlds->cxlfs;
+}
+EXPORT_SYMBOL_NS_GPL(to_cxlfs, "CXL");
+
+static int cxl_get_supported_features_count(struct cxl_mailbox *cxl_mbox)
+{
+	struct cxl_mbox_get_sup_feats_out mbox_out;
+	struct cxl_mbox_get_sup_feats_in mbox_in;
+	struct cxl_mbox_cmd mbox_cmd;
+	int rc;
+
+	memset(&mbox_in, 0, sizeof(mbox_in));
+	mbox_in.count = cpu_to_le32(sizeof(mbox_out));
+	memset(&mbox_out, 0, sizeof(mbox_out));
+	mbox_cmd = (struct cxl_mbox_cmd) {
+		.opcode = CXL_MBOX_OP_GET_SUPPORTED_FEATURES,
+		.size_in = sizeof(mbox_in),
+		.payload_in = &mbox_in,
+		.size_out = sizeof(mbox_out),
+		.payload_out = &mbox_out,
+		.min_out = sizeof(mbox_out),
+	};
+	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
+	if (rc < 0)
+		return rc;
+
+	return le16_to_cpu(mbox_out.supported_feats);
+}
+
+static struct cxl_feat_entries *
+get_supported_features(struct cxl_features_state *cxlfs)
+{
+	int remain_feats, max_size, max_feats, start, rc, hdr_size;
+	struct cxl_mailbox *cxl_mbox = &cxlfs->cxlds->cxl_mbox;
+	int feat_size = sizeof(struct cxl_feat_entry);
+	struct cxl_mbox_get_sup_feats_in mbox_in;
+	struct cxl_feat_entry *entry;
+	struct cxl_mbox_cmd mbox_cmd;
+	int count;
+
+	count = cxl_get_supported_features_count(cxl_mbox);
+	if (count <= 0)
+		return NULL;
+
+	struct cxl_feat_entries *entries __free(kvfree) =
+		kvmalloc(struct_size(entries, ent, count), GFP_KERNEL);
+	if (!entries)
+		return NULL;
+
+	struct cxl_mbox_get_sup_feats_out *mbox_out __free(kvfree) =
+		kvmalloc(cxl_mbox->payload_size, GFP_KERNEL);
+	if (!mbox_out)
+		return NULL;
+
+	hdr_size = struct_size(mbox_out, ents, 0);
+	max_size = cxl_mbox->payload_size - hdr_size;
+	/* max feat entries that can fit in mailbox max payload size */
+	max_feats = max_size / feat_size;
+	entry = entries->ent;
+
+	start = 0;
+	remain_feats = count;
+	do {
+		int retrieved, alloc_size, copy_feats;
+		int num_entries;
+
+		if (remain_feats > max_feats) {
+			alloc_size = struct_size(mbox_out, ents, max_feats);
+			remain_feats = remain_feats - max_feats;
+			copy_feats = max_feats;
+		} else {
+			alloc_size = struct_size(mbox_out, ents, remain_feats);
+			copy_feats = remain_feats;
+			remain_feats = 0;
+		}
+
+		memset(&mbox_in, 0, sizeof(mbox_in));
+		mbox_in.count = cpu_to_le32(alloc_size);
+		mbox_in.start_idx = cpu_to_le16(start);
+		memset(mbox_out, 0, alloc_size);
+		mbox_cmd = (struct cxl_mbox_cmd) {
+			.opcode = CXL_MBOX_OP_GET_SUPPORTED_FEATURES,
+			.size_in = sizeof(mbox_in),
+			.payload_in = &mbox_in,
+			.size_out = alloc_size,
+			.payload_out = mbox_out,
+			.min_out = hdr_size,
+		};
+		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
+		if (rc < 0)
+			return NULL;
+
+		if (mbox_cmd.size_out <= hdr_size)
+			return NULL;
+
+		/*
+		 * Make sure retrieved out buffer is multiple of feature
+		 * entries.
+		 */
+		retrieved = mbox_cmd.size_out - hdr_size;
+		if (retrieved % feat_size)
+			return NULL;
+
+		num_entries = le16_to_cpu(mbox_out->num_entries);
+		/*
+		 * If the reported output entries * defined entry size !=
+		 * retrieved output bytes, then the output package is incorrect.
+		 */
+		if (num_entries * feat_size != retrieved)
+			return NULL;
+
+		memcpy(entry, mbox_out->ents, retrieved);
+		entry += num_entries;
+		/*
+		 * If the number of output entries is less than expected, add the
+		 * remaining entries to the next batch.
+		 */
+		remain_feats += copy_feats - num_entries;
+		start += num_entries;
+	} while (remain_feats);
+
+	entries->num_features = count;
+
+	return no_free_ptr(entries);
+}
+
+static void free_cxlfs(void *_cxlfs)
+{
+	struct cxl_features_state *cxlfs = _cxlfs;
+	struct cxl_dev_state *cxlds = cxlfs->cxlds;
+
+	cxlds->cxlfs = NULL;
+	kvfree(cxlfs->entries);
+	kfree(cxlfs);
+}
+
+/**
+ * devm_cxl_setup_features() - Allocate and initialize features context
+ * @cxlds: CXL device context
+ *
+ * Return 0 on success or -errno on failure.
+ */
+int devm_cxl_setup_features(struct cxl_dev_state *cxlds)
+{
+	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
+
+	if (cxl_mbox->feat_cap < CXL_FEATURES_RO)
+		return -ENODEV;
+
+	struct cxl_features_state *cxlfs __free(kfree) =
+		kzalloc(sizeof(*cxlfs), GFP_KERNEL);
+	if (!cxlfs)
+		return -ENOMEM;
+
+	cxlfs->cxlds = cxlds;
+
+	cxlfs->entries = get_supported_features(cxlfs);
+	if (!cxlfs->entries)
+		return -ENOMEM;
+
+	cxlds->cxlfs = cxlfs;
+
+	return devm_add_action_or_reset(cxlds->dev, free_cxlfs, no_free_ptr(cxlfs));
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_setup_features, "CXL");
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 55c55685cb39..0dc2682eb379 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -392,6 +392,7 @@ struct cxl_dpa_perf {
  * @serial: PCIe Device Serial Number
  * @type: Generic Memory Class device or Vendor Specific Memory device
  * @cxl_mbox: CXL mailbox context
+ * @cxl_features: CXL features context
  */
 struct cxl_dev_state {
 	struct device *dev;
@@ -407,6 +408,9 @@ struct cxl_dev_state {
 	u64 serial;
 	enum cxl_devtype type;
 	struct cxl_mailbox cxl_mbox;
+#ifdef CONFIG_CXL_FEATURES
+	struct cxl_features_state *cxlfs;
+#endif
 };
 
 static inline struct cxl_dev_state *mbox_to_cxlds(struct cxl_mailbox *cxl_mbox)
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index a96e54c6259e..3e666ec51580 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -997,6 +997,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
+	rc = devm_cxl_setup_features(cxlds);
+	if (rc)
+		dev_dbg(&pdev->dev, "No CXL Features discovered\n");
+
 	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
diff --git a/include/cxl/features.h b/include/cxl/features.h
index 357d3acf8429..5b3485eb0f77 100644
--- a/include/cxl/features.h
+++ b/include/cxl/features.h
@@ -3,6 +3,8 @@
 #ifndef __CXL_FEATURES_H__
 #define __CXL_FEATURES_H__
 
+#include <linux/uuid.h>
+
 /* Feature commands capability supported by a device */
 enum cxl_features_capability {
 	CXL_FEATURES_NONE = 0,
@@ -10,4 +12,89 @@ enum cxl_features_capability {
 	CXL_FEATURES_RW,
 };
 
+/* Get Supported Features (0x500h) CXL r3.2 8.2.9.6.1 */
+struct cxl_mbox_get_sup_feats_in {
+	__le32 count;
+	__le16 start_idx;
+	u8 reserved[2];
+} __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)
+
+/*
+ * CXL spec r3.2 Table 8-109
+ * Get Supported Features Supported Feature Entry
+ */
+struct cxl_feat_entry {
+	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];
+} __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)
+
+/*
+ * 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);
+} __packed;
+
+/**
+ * struct cxl_features_state - The Features state for the device
+ * @cxlds: Pointer to CXL device state
+ * @entries: CXl feature entry context
+ *	@num_features: total Features supported by the device
+ *	@ent: Flex array of Feature detail entries from the device
+ */
+struct cxl_features_state {
+	struct cxl_dev_state *cxlds;
+	struct cxl_feat_entries {
+		int num_features;
+		struct cxl_feat_entry ent[] __counted_by(num_features);
+	} *entries;
+};
+
+#ifdef CONFIG_CXL_FEATURES
+inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds);
+int devm_cxl_setup_features(struct cxl_dev_state *cxlds);
+#else
+static inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds)
+{
+	return NULL;
+}
+
+static inline int devm_cxl_setup_features(struct cxl_dev_state *cxlds)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 #endif
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index b1256fee3567..0a6572ab6f37 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -63,6 +63,7 @@ cxl_core-y += $(CXL_CORE_SRC)/pmu.o
 cxl_core-y += $(CXL_CORE_SRC)/cdat.o
 cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
 cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
+cxl_core-$(CONFIG_CXL_FEATURES) += $(CXL_CORE_SRC)/features.o
 cxl_core-y += config_check.o
 cxl_core-y += cxl_core_test.o
 cxl_core-y += cxl_core_exports.o
diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index 8d731bd63988..e9494cd446ef 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -1558,6 +1558,10 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
+	rc = devm_cxl_setup_features(cxlds);
+	if (rc)
+		dev_dbg(dev, "No CXL Features discovered\n");
+
 	cxl_mock_add_event_logs(&mdata->mes);
 
 	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
-- 
2.48.1


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

* [PATCH v6 03/14] cxl/test: Add Get Supported Features mailbox command support
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 01/14] cxl: Enumerate feature commands Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 02/14] cxl: Add Get Supported Features command for kernel usage Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 04/14] cxl/mbox: Add GET_FEATURE mailbox command Dave Jiang
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

Add cxl-test emulation of Get Supported Features mailbox command.
Currently only adding a test feature with feature identifier of
all f's for testing.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/mem.c | 70 ++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index e9494cd446ef..4809a90ff9b6 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -44,6 +44,10 @@ static struct cxl_cel_entry mock_cel[] = {
 		.opcode = cpu_to_le16(CXL_MBOX_OP_GET_SUPPORTED_LOGS),
 		.effect = CXL_CMD_EFFECT_NONE,
 	},
+	{
+		.opcode = cpu_to_le16(CXL_MBOX_OP_GET_SUPPORTED_FEATURES),
+		.effect = CXL_CMD_EFFECT_NONE,
+	},
 	{
 		.opcode = cpu_to_le16(CXL_MBOX_OP_IDENTIFY),
 		.effect = CXL_CMD_EFFECT_NONE,
@@ -1354,6 +1358,69 @@ static int mock_activate_fw(struct cxl_mockmem_data *mdata,
 	return -EINVAL;
 }
 
+#define CXL_VENDOR_FEATURE_TEST							\
+	UUID_INIT(0xffffffff, 0xffff, 0xffff, 0xff, 0xff, 0xff, 0xff, 0xff,	\
+		  0xff, 0xff, 0xff)
+
+static void fill_feature_vendor_test(struct cxl_feat_entry *feat)
+{
+	feat->uuid = CXL_VENDOR_FEATURE_TEST;
+	feat->id = 0;
+	feat->get_feat_size = cpu_to_le16(0x4);
+	feat->set_feat_size = cpu_to_le16(0x4);
+	feat->flags = cpu_to_le32(CXL_FEATURE_F_CHANGEABLE |
+				  CXL_FEATURE_F_DEFAULT_SEL |
+				  CXL_FEATURE_F_SAVED_SEL);
+	feat->get_feat_ver = 1;
+	feat->set_feat_ver = 1;
+	feat->effects = cpu_to_le16(CXL_CMD_CONFIG_CHANGE_COLD_RESET |
+				    CXL_CMD_EFFECTS_VALID);
+}
+
+#define MAX_CXL_TEST_FEATS	1
+
+static int mock_get_supported_features(struct cxl_mockmem_data *mdata,
+				       struct cxl_mbox_cmd *cmd)
+{
+	struct cxl_mbox_get_sup_feats_in *in = cmd->payload_in;
+	struct cxl_mbox_get_sup_feats_out *out = cmd->payload_out;
+	struct cxl_feat_entry *feat;
+	u16 start_idx, count;
+
+	if (cmd->size_out < sizeof(*out)) {
+		cmd->return_code = CXL_MBOX_CMD_RC_PAYLOADLEN;
+		return -EINVAL;
+	}
+
+	/*
+	 * Current emulation only supports 1 feature
+	 */
+	start_idx = le16_to_cpu(in->start_idx);
+	if (start_idx != 0) {
+		cmd->return_code = CXL_MBOX_CMD_RC_INPUT;
+		return -EINVAL;
+	}
+
+	count = le16_to_cpu(in->count);
+	if (count < struct_size(out, ents, 0)) {
+		cmd->return_code = CXL_MBOX_CMD_RC_PAYLOADLEN;
+		return -EINVAL;
+	}
+
+	out->supported_feats = cpu_to_le16(MAX_CXL_TEST_FEATS);
+	cmd->return_code = 0;
+	if (count < struct_size(out, ents, MAX_CXL_TEST_FEATS)) {
+		out->num_entries = 0;
+		return 0;
+	}
+
+	out->num_entries = cpu_to_le16(MAX_CXL_TEST_FEATS);
+	feat = out->ents;
+	fill_feature_vendor_test(feat);
+
+	return 0;
+}
+
 static int cxl_mock_mbox_send(struct cxl_mailbox *cxl_mbox,
 			      struct cxl_mbox_cmd *cmd)
 {
@@ -1439,6 +1506,9 @@ static int cxl_mock_mbox_send(struct cxl_mailbox *cxl_mbox,
 	case CXL_MBOX_OP_ACTIVATE_FW:
 		rc = mock_activate_fw(mdata, cmd);
 		break;
+	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
+		rc = mock_get_supported_features(mdata, cmd);
+		break;
 	default:
 		break;
 	}
-- 
2.48.1


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

* [PATCH v6 04/14] cxl/mbox: Add GET_FEATURE mailbox command
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (2 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 03/14] cxl/test: Add Get Supported Features mailbox command support Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 05/14] cxl/mbox: Add SET_FEATURE " Dave Jiang
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

From: Shiju Jose <shiju.jose@huawei.com>

Add support for GET_FEATURE mailbox command.

CXL spec r3.2 section 8.2.9.6 describes optional device specific features.
The settings of a feature can be retrieved using Get Feature command.
CXL spec r3.2 section 8.2.9.6.2 describes Get Feature command.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/core.h     |  7 ++++++
 drivers/cxl/core/features.c | 50 +++++++++++++++++++++++++++++++++++++
 include/cxl/features.h      | 24 ++++++++++++++++++
 3 files changed, 81 insertions(+)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index f77fcf71ade5..7cd3b98150e6 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -117,4 +117,11 @@ bool cxl_need_node_perf_attrs_update(int nid);
 int cxl_port_get_switch_dport_bandwidth(struct cxl_port *port,
 					struct access_coordinate *c);
 
+#ifdef CONFIG_CXL_FEATURES
+size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
+		       enum cxl_get_feat_selection selection,
+		       void *feat_out, size_t feat_out_size, u16 offset,
+		       u16 *return_code);
+#endif
+
 #endif /* __CXL_CORE_H__ */
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index f2898a56bd63..3fc6f8415f19 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -4,6 +4,7 @@
 #include <cxl/mailbox.h>
 #include <cxl/features.h>
 #include "cxl.h"
+#include "core.h"
 #include "cxlmem.h"
 
 inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds)
@@ -173,3 +174,52 @@ int devm_cxl_setup_features(struct cxl_dev_state *cxlds)
 	return devm_add_action_or_reset(cxlds->dev, free_cxlfs, no_free_ptr(cxlfs));
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_setup_features, "CXL");
+
+size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
+		       enum cxl_get_feat_selection selection,
+		       void *feat_out, size_t feat_out_size, u16 offset,
+		       u16 *return_code)
+{
+	size_t data_to_rd_size, size_out;
+	struct cxl_mbox_get_feat_in pi;
+	struct cxl_mbox_cmd mbox_cmd;
+	size_t data_rcvd_size = 0;
+	int rc;
+
+	if (return_code)
+		*return_code = CXL_MBOX_CMD_RC_INPUT;
+
+	if (!feat_out || !feat_out_size)
+		return 0;
+
+	size_out = min(feat_out_size, cxl_mbox->payload_size);
+	uuid_copy(&pi.uuid, feat_uuid);
+	pi.selection = selection;
+	do {
+		data_to_rd_size = min(feat_out_size - data_rcvd_size,
+				      cxl_mbox->payload_size);
+		pi.offset = cpu_to_le16(offset + data_rcvd_size);
+		pi.count = cpu_to_le16(data_to_rd_size);
+
+		mbox_cmd = (struct cxl_mbox_cmd) {
+			.opcode = CXL_MBOX_OP_GET_FEATURE,
+			.size_in = sizeof(pi),
+			.payload_in = &pi,
+			.size_out = size_out,
+			.payload_out = feat_out + data_rcvd_size,
+			.min_out = data_to_rd_size,
+		};
+		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
+		if (rc < 0 || !mbox_cmd.size_out) {
+			if (return_code)
+				*return_code = mbox_cmd.return_code;
+			return 0;
+		}
+		data_rcvd_size += mbox_cmd.size_out;
+	} while (data_rcvd_size < feat_out_size);
+
+	if (return_code)
+		*return_code = CXL_MBOX_CMD_RC_SUCCESS;
+
+	return data_rcvd_size;
+}
diff --git a/include/cxl/features.h b/include/cxl/features.h
index 5b3485eb0f77..766c09978e5e 100644
--- a/include/cxl/features.h
+++ b/include/cxl/features.h
@@ -67,6 +67,29 @@ struct cxl_mbox_get_sup_feats_out {
 	struct cxl_feat_entry ents[] __counted_by_le(num_entries);
 } __packed;
 
+/*
+ * Get Feature CXL spec r3.2 Spec 8.2.9.6.2
+ */
+
+/*
+ * Get Feature input payload
+ * CXL spec r3.2 section 8.2.9.6.2 Table 8-99
+ */
+struct cxl_mbox_get_feat_in {
+	uuid_t uuid;
+	__le16 offset;
+	__le16 count;
+	u8 selection;
+}  __packed;
+
+/* Selection field for 'struct cxl_mbox_get_feat_in' */
+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
+};
+
 /**
  * struct cxl_features_state - The Features state for the device
  * @cxlds: Pointer to CXL device state
@@ -82,6 +105,7 @@ struct cxl_features_state {
 	} *entries;
 };
 
+struct cxl_mailbox;
 #ifdef CONFIG_CXL_FEATURES
 inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds);
 int devm_cxl_setup_features(struct cxl_dev_state *cxlds);
-- 
2.48.1


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

* [PATCH v6 05/14] cxl/mbox: Add SET_FEATURE mailbox command
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (3 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 04/14] cxl/mbox: Add GET_FEATURE mailbox command Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 06/14] cxl: Setup exclusive CXL features that are reserved for the kernel Dave Jiang
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

From: Shiju Jose <shiju.jose@huawei.com>

Add support for SET_FEATURE mailbox command.

CXL spec r3.2 section 8.2.9.6 describes optional device specific features.
CXL devices supports features with changeable attributes.
The settings of a feature can be optionally modified using Set Feature
command.
CXL spec r3.2 section 8.2.9.6.3 describes Set Feature command.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v6:
- move 'rc' into scope. (Jonathan)
- label feat_data as const.
- use __struct_group() for cxl_mbox_get_sup_feats_out.
---
 drivers/cxl/core/core.h     |  4 ++
 drivers/cxl/core/features.c | 80 +++++++++++++++++++++++++++++++++++++
 include/cxl/features.h      | 33 +++++++++++++++
 3 files changed, 117 insertions(+)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 7cd3b98150e6..17e99a25c29a 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -122,6 +122,10 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 		       enum cxl_get_feat_selection selection,
 		       void *feat_out, size_t feat_out_size, u16 offset,
 		       u16 *return_code);
+int cxl_set_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
+		    u8 feat_version, const void *feat_data,
+		    size_t feat_data_size, u32 feat_flag, u16 offset,
+		    u16 *return_code);
 #endif
 
 #endif /* __CXL_CORE_H__ */
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 3fc6f8415f19..17a82734a340 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -223,3 +223,83 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 
 	return data_rcvd_size;
 }
+
+/*
+ * FEAT_DATA_MIN_PAYLOAD_SIZE - min extra number of bytes should be
+ * available in the mailbox for storing the actual feature data so that
+ * the feature data transfer would work as expected.
+ */
+#define FEAT_DATA_MIN_PAYLOAD_SIZE 10
+int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
+		    const uuid_t *feat_uuid, u8 feat_version,
+		    const void *feat_data, size_t feat_data_size,
+		    u32 feat_flag, u16 offset, u16 *return_code)
+{
+	size_t data_in_size, data_sent_size = 0;
+	struct cxl_mbox_cmd mbox_cmd;
+	size_t hdr_size;
+
+	if (return_code)
+		*return_code = CXL_MBOX_CMD_RC_INPUT;
+
+	struct cxl_mbox_set_feat_in *pi __free(kfree) =
+			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
+	if (!pi)
+		return -ENOMEM;
+
+	uuid_copy(&pi->uuid, feat_uuid);
+	pi->version = feat_version;
+	feat_flag &= ~CXL_SET_FEAT_FLAG_DATA_TRANSFER_MASK;
+	feat_flag |= CXL_SET_FEAT_FLAG_DATA_SAVED_ACROSS_RESET;
+	hdr_size = sizeof(pi->hdr);
+	/*
+	 * Check minimum mbox payload size is available for
+	 * the feature data transfer.
+	 */
+	if (hdr_size + FEAT_DATA_MIN_PAYLOAD_SIZE > cxl_mbox->payload_size)
+		return -ENOMEM;
+
+	if (hdr_size + feat_data_size <= cxl_mbox->payload_size) {
+		pi->flags = cpu_to_le32(feat_flag |
+					CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER);
+		data_in_size = feat_data_size;
+	} else {
+		pi->flags = cpu_to_le32(feat_flag |
+					CXL_SET_FEAT_FLAG_INITIATE_DATA_TRANSFER);
+		data_in_size = cxl_mbox->payload_size - hdr_size;
+	}
+
+	do {
+		int rc;
+
+		pi->offset = cpu_to_le16(offset + data_sent_size);
+		memcpy(pi->feat_data, feat_data + data_sent_size, data_in_size);
+		mbox_cmd = (struct cxl_mbox_cmd) {
+			.opcode = CXL_MBOX_OP_SET_FEATURE,
+			.size_in = hdr_size + data_in_size,
+			.payload_in = pi,
+		};
+		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
+		if (rc < 0) {
+			if (return_code)
+				*return_code = mbox_cmd.return_code;
+			return rc;
+		}
+
+		data_sent_size += data_in_size;
+		if (data_sent_size >= feat_data_size) {
+			if (return_code)
+				*return_code = CXL_MBOX_CMD_RC_SUCCESS;
+			return 0;
+		}
+
+		if ((feat_data_size - data_sent_size) <= (cxl_mbox->payload_size - hdr_size)) {
+			data_in_size = feat_data_size - data_sent_size;
+			pi->flags = cpu_to_le32(feat_flag |
+						CXL_SET_FEAT_FLAG_FINISH_DATA_TRANSFER);
+		} else {
+			pi->flags = cpu_to_le32(feat_flag |
+						CXL_SET_FEAT_FLAG_CONTINUE_DATA_TRANSFER);
+		}
+	} while (true);
+}
diff --git a/include/cxl/features.h b/include/cxl/features.h
index 766c09978e5e..ae56ef7ec9ab 100644
--- a/include/cxl/features.h
+++ b/include/cxl/features.h
@@ -90,6 +90,39 @@ enum cxl_get_feat_selection {
 	CXL_GET_FEAT_SEL_MAX
 };
 
+/*
+ * Set Feature CXL spec r3.2  8.2.9.6.3
+ */
+
+/*
+ * Set Feature input payload
+ * 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 */,
+		uuid_t uuid;
+		__le32 flags;
+		__le16 offset;
+		u8 version;
+		u8 rsvd[9];
+	);
+	__u8 feat_data[];
+}  __packed;
+
+/* 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)
+
 /**
  * struct cxl_features_state - The Features state for the device
  * @cxlds: Pointer to CXL device state
-- 
2.48.1


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

* [PATCH v6 06/14] cxl: Setup exclusive CXL features that are reserved for the kernel
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (4 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 05/14] cxl/mbox: Add SET_FEATURE " Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-20 15:37   ` Shiju Jose
  2025-02-18 22:54 ` [PATCH v6 07/14] cxl: Add FWCTL support to CXL Dave Jiang
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

Certain features will be exclusively used by components such as in
kernel RAS driver. Setup an exclusion list that can be used to detect
if a feature is exclusive to the kernel.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/features.c | 28 ++++++++++++++++++++++++++++
 include/cxl/features.h      | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 17a82734a340..048ba4fc3538 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -7,6 +7,28 @@
 #include "core.h"
 #include "cxlmem.h"
 
+/* All the features below are exclusive to the kernel */
+static const uuid_t cxl_exclusive_feats[] = {
+	CXL_FEAT_PATROL_SCRUB_UUID,
+	CXL_FEAT_ECS_UUID,
+	CXL_FEAT_SPPR_UUID,
+	CXL_FEAT_HPPR_UUID,
+	CXL_FEAT_CACHELINE_SPARING_UUID,
+	CXL_FEAT_ROW_SPARING_UUID,
+	CXL_FEAT_BANK_SPARING_UUID,
+	CXL_FEAT_RANK_SPARING_UUID,
+};
+
+static bool is_cxl_feature_exclusive(struct cxl_feat_entry *entry)
+{
+	for (int i = 0; i < ARRAY_SIZE(cxl_exclusive_feats); i++) {
+		if (uuid_equal(&entry->uuid, &cxl_exclusive_feats[i]))
+			return true;
+	}
+
+	return false;
+}
+
 inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds)
 {
 	return cxlds->cxlfs;
@@ -47,6 +69,7 @@ get_supported_features(struct cxl_features_state *cxlfs)
 	struct cxl_mbox_get_sup_feats_in mbox_in;
 	struct cxl_feat_entry *entry;
 	struct cxl_mbox_cmd mbox_cmd;
+	int user_feats = 0;
 	int count;
 
 	count = cxl_get_supported_features_count(cxl_mbox);
@@ -121,6 +144,10 @@ get_supported_features(struct cxl_features_state *cxlfs)
 			return NULL;
 
 		memcpy(entry, mbox_out->ents, retrieved);
+		for (int i = 0; i < num_entries; i++) {
+			if (!is_cxl_feature_exclusive(entry + i))
+				user_feats++;
+		}
 		entry += num_entries;
 		/*
 		 * If the number of output entries is less than expected, add the
@@ -131,6 +158,7 @@ get_supported_features(struct cxl_features_state *cxlfs)
 	} while (remain_feats);
 
 	entries->num_features = count;
+	entries->num_user_features = user_feats;
 
 	return no_free_ptr(entries);
 }
diff --git a/include/cxl/features.h b/include/cxl/features.h
index ae56ef7ec9ab..d2cde46b0fec 100644
--- a/include/cxl/features.h
+++ b/include/cxl/features.h
@@ -5,6 +5,39 @@
 
 #include <linux/uuid.h>
 
+/* Feature UUIDs used by the kernel */
+#define CXL_FEAT_PATROL_SCRUB_UUID						\
+	UUID_INIT(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33, 0x75, 0x77, 0x4e,	\
+		  0x06, 0xdb, 0x8a)
+
+#define CXL_FEAT_ECS_UUID							\
+	UUID_INIT(0xe5b13f22, 0x2328, 0x4a14, 0xb8, 0xba, 0xb9, 0x69, 0x1e,	\
+		  0x89, 0x33, 0x86)
+
+#define CXL_FEAT_SPPR_UUID							\
+	UUID_INIT(0x892ba475, 0xfad8, 0x474e, 0x9d, 0x3e, 0x69, 0x2c, 0x91,	\
+		  0x75, 0x68, 0xbb)
+
+#define CXL_FEAT_HPPR_UUID							\
+	UUID_INIT(0x80ea4521, 0x786f, 0x4127, 0xaf, 0xb1, 0xec, 0x74, 0x59,	\
+		  0xfb, 0x0e, 0x24)
+
+#define CXL_FEAT_CACHELINE_SPARING_UUID						\
+	UUID_INIT(0x96C33386, 0x91dd, 0x44c7, 0x9e, 0xcb, 0xfd, 0xaf, 0x65,	\
+		  0x03, 0xba, 0xc4)
+
+#define CXL_FEAT_ROW_SPARING_UUID						\
+	UUID_INIT(0x450ebf67, 0xb135, 0x4f97, 0xa4, 0x98, 0xc2, 0xd5, 0x7f,	\
+		  0x27, 0x9b, 0xed)
+
+#define CXL_FEAT_BANK_SPARING_UUID						\
+	UUID_INIT(0x78b79636, 0x90ac, 0x4b64, 0xa4, 0xef, 0xfa, 0xac, 0x5d,	\
+		  0x18, 0xa8, 0x63)
+
+#define CXL_FEAT_RANK_SPARING_UUID						\
+	UUID_INIT(0x34dbaff5, 0x0552, 0x4281, 0x8f, 0x76, 0xda, 0x0b, 0x5e,	\
+		  0x7a, 0x76, 0xa7)
+
 /* Feature commands capability supported by a device */
 enum cxl_features_capability {
 	CXL_FEATURES_NONE = 0,
@@ -134,6 +167,7 @@ struct cxl_features_state {
 	struct cxl_dev_state *cxlds;
 	struct cxl_feat_entries {
 		int num_features;
+		int num_user_features;
 		struct cxl_feat_entry ent[] __counted_by(num_features);
 	} *entries;
 };
-- 
2.48.1


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

* [PATCH v6 07/14] cxl: Add FWCTL support to CXL
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (5 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 06/14] cxl: Setup exclusive CXL features that are reserved for the kernel Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-19  7:25   ` Li Ming
  2025-02-19 17:09   ` Jonathan Cameron
  2025-02-18 22:54 ` [PATCH v6 08/14] cxl: Move cxl feature command structs to user header Dave Jiang
                   ` (6 subsequent siblings)
  13 siblings, 2 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed

Add fwctl support code to allow sending of CXL feature commands from
userspace through as ioctls via FWCTL. Provide initial setup bits. The
CXL PCI probe function will call cxl_setup_fwctl() after the cxl_memdev
has been enumerated in order to setup FWCTL char device under the
cxl_memdev like the existing memdev char device for issuing CXL raw
mailbox commands from userspace via ioctls.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v6:
- Drop info callback. (Dan)
---
 drivers/cxl/Kconfig          |  1 +
 drivers/cxl/core/features.c  | 74 ++++++++++++++++++++++++++++++++++++
 drivers/cxl/pci.c            |  4 ++
 include/cxl/features.h       | 10 +++++
 include/uapi/fwctl/fwctl.h   |  1 +
 tools/testing/cxl/test/mem.c |  4 ++
 6 files changed, 94 insertions(+)

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index ad2e796e4ac6..2b6e662c89bf 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -105,6 +105,7 @@ config CXL_MEM
 config CXL_FEATURES
 	bool "CXL: Features"
 	depends on CXL_PCI
+	select FWCTL
 	help
 	  Enable support for CXL Features. A CXL device that includes a mailbox
 	  supports commands that allows listing, getting, and setting of
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 048ba4fc3538..846de3294a5e 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2024-2025 Intel Corporation. All rights reserved. */
+#include <linux/fwctl.h>
 #include <linux/device.h>
 #include <cxl/mailbox.h>
 #include <cxl/features.h>
@@ -331,3 +332,76 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
 		}
 	} while (true);
 }
+
+/* FWCTL support */
+
+static inline struct cxl_memdev *fwctl_to_memdev(struct fwctl_device *fwctl_dev)
+{
+	return to_cxl_memdev(fwctl_dev->dev.parent);
+}
+
+static int cxlctl_open_uctx(struct fwctl_uctx *uctx)
+{
+	return 0;
+}
+
+static void cxlctl_close_uctx(struct fwctl_uctx *uctx)
+{
+}
+
+static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
+			   void *in, size_t in_len, size_t *out_len)
+{
+	/* Place holder */
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
+static const struct fwctl_ops cxlctl_ops = {
+	.device_type = FWCTL_DEVICE_TYPE_CXL,
+	.uctx_size = sizeof(struct fwctl_uctx),
+	.open_uctx = cxlctl_open_uctx,
+	.close_uctx = cxlctl_close_uctx,
+	.fw_rpc = cxlctl_fw_rpc,
+};
+
+DEFINE_FREE(free_fwctl_dev, struct fwctl_device *, if (_T) fwctl_put(_T))
+
+static void free_memdev_fwctl(void *_fwctl_dev)
+{
+	struct fwctl_device *fwctl_dev = _fwctl_dev;
+
+	fwctl_unregister(fwctl_dev);
+	fwctl_put(fwctl_dev);
+}
+
+int devm_cxl_setup_fwctl(struct cxl_memdev *cxlmd)
+{
+	struct cxl_dev_state *cxlds = cxlmd->cxlds;
+	struct cxl_features_state *cxlfs;
+	int rc;
+
+	cxlfs = to_cxlfs(cxlds);
+	if (!cxlfs)
+		return -ENODEV;
+
+	/* No need to setup FWCTL if there are no user allowed features found */
+	if (!cxlfs->entries->num_user_features)
+		return -ENODEV;
+
+	struct fwctl_device *fwctl_dev __free(free_fwctl_dev) =
+		_fwctl_alloc_device(&cxlmd->dev, &cxlctl_ops, sizeof(*fwctl_dev));
+	if (!fwctl_dev)
+		return -ENOMEM;
+
+	rc = fwctl_register(fwctl_dev);
+	if (rc)
+		return rc;
+
+	cxlfs->fwctl_dev = fwctl_dev;
+
+	return devm_add_action_or_reset(&cxlmd->dev, free_memdev_fwctl,
+					no_free_ptr(fwctl_dev));
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_setup_fwctl, "CXL");
+
+MODULE_IMPORT_NS("FWCTL");
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 3e666ec51580..993fa60fe453 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1013,6 +1013,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
+	rc = devm_cxl_setup_fwctl(cxlmd);
+	if (rc)
+		dev_dbg(&pdev->dev, "No CXL FWCTL setup\n");
+
 	pmu_count = cxl_count_regblock(pdev, CXL_REGLOC_RBI_PMU);
 	if (pmu_count < 0)
 		return pmu_count;
diff --git a/include/cxl/features.h b/include/cxl/features.h
index d2cde46b0fec..fce3c2dbde5a 100644
--- a/include/cxl/features.h
+++ b/include/cxl/features.h
@@ -4,6 +4,7 @@
 #define __CXL_FEATURES_H__
 
 #include <linux/uuid.h>
+#include <linux/fwctl.h>
 
 /* Feature UUIDs used by the kernel */
 #define CXL_FEAT_PATROL_SCRUB_UUID						\
@@ -162,6 +163,7 @@ enum cxl_set_feat_flag_data_transfer {
  * @entries: CXl feature entry context
  *	@num_features: total Features supported by the device
  *	@ent: Flex array of Feature detail entries from the device
+ * @fwctl_dev: Firmware Control device
  */
 struct cxl_features_state {
 	struct cxl_dev_state *cxlds;
@@ -170,12 +172,15 @@ struct cxl_features_state {
 		int num_user_features;
 		struct cxl_feat_entry ent[] __counted_by(num_features);
 	} *entries;
+	struct fwctl_device *fwctl_dev;
 };
 
 struct cxl_mailbox;
+struct cxl_memdev;
 #ifdef CONFIG_CXL_FEATURES
 inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds);
 int devm_cxl_setup_features(struct cxl_dev_state *cxlds);
+int devm_cxl_setup_fwctl(struct cxl_memdev *cxlmd);
 #else
 static inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds)
 {
@@ -186,6 +191,11 @@ static inline int devm_cxl_setup_features(struct cxl_dev_state *cxlds)
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int devm_cxl_setup_fwctl(struct cxl_memdev *cxlmd)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 #endif
diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
index 518f054f02d2..f57f6d86b12f 100644
--- a/include/uapi/fwctl/fwctl.h
+++ b/include/uapi/fwctl/fwctl.h
@@ -43,6 +43,7 @@ enum {
 enum fwctl_device_type {
 	FWCTL_DEVICE_TYPE_ERROR = 0,
 	FWCTL_DEVICE_TYPE_MLX5 = 1,
+	FWCTL_DEVICE_TYPE_CXL = 2,
 	FWCTL_DEVICE_TYPE_BNXT = 3,
 };
 
diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index 4809a90ff9b6..848db399102c 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -1646,6 +1646,10 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
+	rc = devm_cxl_setup_fwctl(cxlmd);
+	if (rc)
+		dev_dbg(dev, "No CXL FWCTL setup\n");
+
 	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
 
 	return 0;
-- 
2.48.1


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

* [PATCH v6 08/14] cxl: Move cxl feature command structs to user header
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (6 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 07/14] cxl: Add FWCTL support to CXL Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-19  7:26   ` Li Ming
  2025-02-18 22:54 ` [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands Dave Jiang
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed

In preparation for cxl fwctl enabling, move data structures related to
cxl feature commands to a user header file.

Reviewed-by; Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 include/cxl/features.h      | 112 +------------------------------
 include/uapi/cxl/features.h | 127 ++++++++++++++++++++++++++++++++++++
 2 files changed, 128 insertions(+), 111 deletions(-)
 create mode 100644 include/uapi/cxl/features.h

diff --git a/include/cxl/features.h b/include/cxl/features.h
index fce3c2dbde5a..ead63573b0b4 100644
--- a/include/cxl/features.h
+++ b/include/cxl/features.h
@@ -5,6 +5,7 @@
 
 #include <linux/uuid.h>
 #include <linux/fwctl.h>
+#include <uapi/cxl/features.h>
 
 /* Feature UUIDs used by the kernel */
 #define CXL_FEAT_PATROL_SCRUB_UUID						\
@@ -46,117 +47,6 @@ enum cxl_features_capability {
 	CXL_FEATURES_RW,
 };
 
-/* Get Supported Features (0x500h) CXL r3.2 8.2.9.6.1 */
-struct cxl_mbox_get_sup_feats_in {
-	__le32 count;
-	__le16 start_idx;
-	u8 reserved[2];
-} __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)
-
-/*
- * CXL spec r3.2 Table 8-109
- * Get Supported Features Supported Feature Entry
- */
-struct cxl_feat_entry {
-	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];
-} __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)
-
-/*
- * 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);
-} __packed;
-
-/*
- * Get Feature CXL spec r3.2 Spec 8.2.9.6.2
- */
-
-/*
- * Get Feature input payload
- * CXL spec r3.2 section 8.2.9.6.2 Table 8-99
- */
-struct cxl_mbox_get_feat_in {
-	uuid_t uuid;
-	__le16 offset;
-	__le16 count;
-	u8 selection;
-}  __packed;
-
-/* Selection field for 'struct cxl_mbox_get_feat_in' */
-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
- */
-
-/*
- * Set Feature input payload
- * 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 */,
-		uuid_t uuid;
-		__le32 flags;
-		__le16 offset;
-		u8 version;
-		u8 rsvd[9];
-	);
-	__u8 feat_data[];
-}  __packed;
-
-/* 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)
-
 /**
  * struct cxl_features_state - The Features state for the device
  * @cxlds: Pointer to CXL device state
diff --git a/include/uapi/cxl/features.h b/include/uapi/cxl/features.h
new file mode 100644
index 000000000000..17f9da63f982
--- /dev/null
+++ b/include/uapi/cxl/features.h
@@ -0,0 +1,127 @@
+/* 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>
+#ifndef __KERNEL__
+#include <uuid/uuid.h>
+#else
+#include <linux/uuid.h>
+#endif
+
+/* Get Supported Features (0x500h) CXL r3.2 8.2.9.6.1 */
+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)
+
+/*
+ * CXL spec r3.2 Table 8-109
+ * Get Supported Features Supported Feature Entry
+ */
+struct cxl_feat_entry {
+	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)
+
+/*
+ * 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
+ */
+
+/*
+ * Get Feature input payload
+ * CXL spec r3.2 section 8.2.9.6.2 Table 8-99
+ */
+struct cxl_mbox_get_feat_in {
+	uuid_t uuid;
+	__le16 offset;
+	__le16 count;
+	__u8 selection;
+} __attribute__ ((__packed__));
+
+/* Selection field for 'struct cxl_mbox_get_feat_in' */
+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
+ */
+
+/*
+ * Set Feature input payload
+ * 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 */,
+		uuid_t uuid;
+		__le32 flags;
+		__le16 offset;
+		__u8 version;
+		__u8 rsvd[9];
+	);
+	__u8 feat_data[];
+}  __packed;
+
+/* 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
-- 
2.48.1


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

* [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (7 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 08/14] cxl: Move cxl feature command structs to user header Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-19  7:27   ` Li Ming
  2025-02-19 17:53   ` Jonathan Cameron
  2025-02-18 22:54 ` [PATCH v6 10/14] cxl: Add support to handle user feature commands for get feature Dave Jiang
                   ` (4 subsequent siblings)
  13 siblings, 2 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed

fwctl provides a fwctl_ops->fw_rpc() callback in order to issue ioctls
to a device. The cxl fwctl driver will start by supporting the CXL
Feature commands: Get Supported Features, Get Feature, and Set Feature.

The fw_rpc() callback provides 'enum fwctl_rpc_scope' parameter where
it indicates the security scope of the call. The Get Supported Features
and Get Feature calls can be executed with the scope of
FWCTL_RPC_CONFIGRATION. The Set Feature call is gated by the effects
of the Feature reported by Get Supported Features call for the specific
Feature.

Only "Get Supported Features" is supported in this patch. Additional
commands will be added in follow on patches. "Get Supported Features"
will filter the Features that are exclusive to the kernel. The flag
field of the Feature details will be cleared of the "Changeable"
field and the "set feat size" will be set to 0 to indicate that
the feature is not changeable.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v6:
- Embed hw op in 'fwctl_rpc_cxl'. (Saeed, Jason)
- Move set_features bits to set_features enabling patch. (Saeed)
- Fix copyright years. (Jason)
---
 drivers/cxl/core/features.c | 121 +++++++++++++++++++++++++++++++++++-
 include/uapi/cxl/features.h |   1 +
 include/uapi/fwctl/cxl.h    |  57 +++++++++++++++++
 3 files changed, 177 insertions(+), 2 deletions(-)
 create mode 100644 include/uapi/fwctl/cxl.h

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 846de3294a5e..106ea9e25c82 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -4,6 +4,7 @@
 #include <linux/device.h>
 #include <cxl/mailbox.h>
 #include <cxl/features.h>
+#include <uapi/fwctl/cxl.h>
 #include "cxl.h"
 #include "core.h"
 #include "cxlmem.h"
@@ -349,11 +350,127 @@ static void cxlctl_close_uctx(struct fwctl_uctx *uctx)
 {
 }
 
+static void *cxlctl_get_supported_features(struct cxl_features_state *cxlfs,
+					   const struct fwctl_rpc_cxl *rpc_in,
+					   size_t *out_len)
+{
+	const struct cxl_mbox_get_sup_feats_in *feat_in;
+	struct cxl_mbox_get_sup_feats_out *feat_out;
+	struct cxl_feat_entry *pos;
+	size_t out_size;
+	int requested;
+	u32 count;
+	u16 start;
+	int i;
+
+	if (rpc_in->op_size != sizeof(*feat_in))
+		return ERR_PTR(-EINVAL);
+
+	feat_in = &rpc_in->get_sup_feats_in;
+	count = le32_to_cpu(feat_in->count);
+	start = le16_to_cpu(feat_in->start_idx);
+	requested = count / sizeof(*pos);
+
+	/*
+	 * Make sure that the total requested number of entries is not greater
+	 * than the total number of supported features allowed for userspace.
+	 */
+	if (start >= cxlfs->entries->num_features)
+		return ERR_PTR(-EINVAL);
+
+	requested = min_t(int, requested, cxlfs->entries->num_features - start);
+
+	out_size = sizeof(struct fwctl_rpc_cxl_out) +
+		struct_size(feat_out, ents, requested);
+
+	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
+		kvzalloc(out_size, GFP_KERNEL);
+	if (!rpc_out)
+		return ERR_PTR(-ENOMEM);
+
+	rpc_out->size = struct_size(feat_out, ents, requested);
+	feat_out = (struct cxl_mbox_get_sup_feats_out *)rpc_out->payload;
+	if (requested == 0) {
+		feat_out->num_entries = cpu_to_le16(requested);
+		feat_out->supported_feats =
+			cpu_to_le16(cxlfs->entries->num_features);
+		rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
+		*out_len = out_size;
+		return no_free_ptr(rpc_out);
+	}
+
+	for (i = start, pos = &feat_out->ents[0];
+	     i < cxlfs->entries->num_features; i++, pos++) {
+		if (i - start == requested)
+			break;
+
+		memcpy(pos, &cxlfs->entries->ent[i], sizeof(*pos));
+		/*
+		 * If the feature is exclusive, set the set_feat_size to 0 to
+		 * indicate that the feature is not changeable.
+		 */
+		if (is_cxl_feature_exclusive(pos)) {
+			u32 flags;
+
+			pos->set_feat_size = 0;
+			flags = le32_to_cpu(pos->flags);
+			flags &= ~CXL_FEATURE_F_CHANGEABLE;
+			pos->flags = cpu_to_le32(flags);
+		}
+	}
+
+	feat_out->num_entries = cpu_to_le16(requested);
+	feat_out->supported_feats = cpu_to_le16(cxlfs->entries->num_features);
+	rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
+	*out_len = out_size;
+
+	return no_free_ptr(rpc_out);
+}
+
+static bool cxlctl_validate_hw_command(struct cxl_features_state *cxlfs,
+				       const struct fwctl_rpc_cxl *rpc_in,
+				       enum fwctl_rpc_scope scope,
+				       u16 opcode)
+{
+	struct cxl_mailbox *cxl_mbox = &cxlfs->cxlds->cxl_mbox;
+
+	switch (opcode) {
+	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
+		if (cxl_mbox->feat_cap < CXL_FEATURES_RO)
+			return false;
+		if (scope >= FWCTL_RPC_CONFIGURATION)
+			return true;
+		return false;
+	default:
+		return false;
+	}
+}
+
+static void *cxlctl_handle_commands(struct cxl_features_state *cxlfs,
+				    const struct fwctl_rpc_cxl *rpc_in,
+				    size_t *out_len, u16 opcode)
+{
+	switch (opcode) {
+	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
+		return cxlctl_get_supported_features(cxlfs, rpc_in, out_len);
+	default:
+		return ERR_PTR(-EOPNOTSUPP);
+	}
+}
+
 static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
 			   void *in, size_t in_len, size_t *out_len)
 {
-	/* Place holder */
-	return ERR_PTR(-EOPNOTSUPP);
+	struct fwctl_device *fwctl_dev = uctx->fwctl;
+	struct cxl_memdev *cxlmd = fwctl_to_memdev(fwctl_dev);
+	struct cxl_features_state *cxlfs = to_cxlfs(cxlmd->cxlds);
+	const struct fwctl_rpc_cxl *rpc_in = in;
+	u16 opcode = rpc_in->opcode;
+
+	if (!cxlctl_validate_hw_command(cxlfs, rpc_in, scope, opcode))
+		return ERR_PTR(-EINVAL);
+
+	return cxlctl_handle_commands(cxlfs, rpc_in, out_len, opcode);
 }
 
 static const struct fwctl_ops cxlctl_ops = {
diff --git a/include/uapi/cxl/features.h b/include/uapi/cxl/features.h
index 17f9da63f982..eae0ce008c3a 100644
--- a/include/uapi/cxl/features.h
+++ b/include/uapi/cxl/features.h
@@ -33,6 +33,7 @@ struct cxl_mbox_get_sup_feats_in {
 #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)
 
 /*
  * CXL spec r3.2 Table 8-109
diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
new file mode 100644
index 000000000000..39996ec56816
--- /dev/null
+++ b/include/uapi/fwctl/cxl.h
@@ -0,0 +1,57 @@
+/* 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 cxl_mbox_get_sup_feats_in;
+struct cxl_mbox_get_sup_feats_out;
+
+/**
+ * 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
+ * @op: hardware operation input byte array
+ */
+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;
+		__DECLARE_FLEX_ARRAY(__u8, op);
+	};
+};
+
+/**
+ * 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: Return data from device
+ */
+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
-- 
2.48.1


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

* [PATCH v6 10/14] cxl: Add support to handle user feature commands for get feature
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (8 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-19 17:57   ` Jonathan Cameron
  2025-02-18 22:54 ` [PATCH v6 11/14] cxl: Add support to handle user feature commands for set feature Dave Jiang
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

Add helper function to parse the user data from fwctl RPC ioctl and
send the parsed input parameters to cxl_get_feature() call.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v6:
- adjust for embedded input payload
- move get_feature bits here from previous patch
---
 drivers/cxl/core/features.c | 44 +++++++++++++++++++++++++++++++++++++
 include/uapi/fwctl/cxl.h    |  3 +++
 2 files changed, 47 insertions(+)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 106ea9e25c82..ec0e3ace1803 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -427,6 +427,47 @@ static void *cxlctl_get_supported_features(struct cxl_features_state *cxlfs,
 	return no_free_ptr(rpc_out);
 }
 
+static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
+				const struct fwctl_rpc_cxl *rpc_in,
+				size_t *out_len)
+{
+	struct cxl_mailbox *cxl_mbox = &cxlfs->cxlds->cxl_mbox;
+	const struct cxl_mbox_get_feat_in *feat_in;
+	u16 offset, count, return_code;
+	size_t out_size = *out_len;
+
+	if (rpc_in->op_size != sizeof(*feat_in))
+		return ERR_PTR(-EINVAL);
+
+	feat_in = &rpc_in->get_feat_in;
+	offset = le16_to_cpu(feat_in->offset);
+	count = le16_to_cpu(feat_in->count);
+
+	if (!count)
+		return ERR_PTR(-EINVAL);
+
+	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
+		kvzalloc(out_size, GFP_KERNEL);
+	if (!rpc_out)
+		return ERR_PTR(-ENOMEM);
+
+	out_size = cxl_get_feature(cxl_mbox, &feat_in->uuid,
+				   feat_in->selection, rpc_out->payload,
+				   count, offset, &return_code);
+	*out_len = sizeof(struct fwctl_rpc_cxl_out);
+	if (!out_size) {
+		rpc_out->size = 0;
+		rpc_out->retval = return_code;
+		return no_free_ptr(rpc_out);
+	}
+
+	rpc_out->size = out_size;
+	rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
+	*out_len += out_size;
+
+	return no_free_ptr(rpc_out);
+}
+
 static bool cxlctl_validate_hw_command(struct cxl_features_state *cxlfs,
 				       const struct fwctl_rpc_cxl *rpc_in,
 				       enum fwctl_rpc_scope scope,
@@ -436,6 +477,7 @@ static bool cxlctl_validate_hw_command(struct cxl_features_state *cxlfs,
 
 	switch (opcode) {
 	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
+	case CXL_MBOX_OP_GET_FEATURE:
 		if (cxl_mbox->feat_cap < CXL_FEATURES_RO)
 			return false;
 		if (scope >= FWCTL_RPC_CONFIGURATION)
@@ -453,6 +495,8 @@ static void *cxlctl_handle_commands(struct cxl_features_state *cxlfs,
 	switch (opcode) {
 	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
 		return cxlctl_get_supported_features(cxlfs, rpc_in, out_len);
+	case CXL_MBOX_OP_GET_FEATURE:
+		return cxlctl_get_feature(cxlfs, rpc_in, out_len);
 	default:
 		return ERR_PTR(-EOPNOTSUPP);
 	}
diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
index 39996ec56816..07b7e5bab461 100644
--- a/include/uapi/fwctl/cxl.h
+++ b/include/uapi/fwctl/cxl.h
@@ -13,6 +13,7 @@
 
 struct cxl_mbox_get_sup_feats_in;
 struct cxl_mbox_get_sup_feats_out;
+struct cxl_mbox_get_feat_in;
 
 /**
  * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input for CXL
@@ -21,6 +22,7 @@ struct cxl_mbox_get_sup_feats_out;
  * @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
  * @op: hardware operation input byte array
  */
 struct fwctl_rpc_cxl {
@@ -32,6 +34,7 @@ struct fwctl_rpc_cxl {
 	);
 	union {
 		struct cxl_mbox_get_sup_feats_in get_sup_feats_in;
+		struct cxl_mbox_get_feat_in get_feat_in;
 		__DECLARE_FLEX_ARRAY(__u8, op);
 	};
 };
-- 
2.48.1


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

* [PATCH v6 11/14] cxl: Add support to handle user feature commands for set feature
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (9 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 10/14] cxl: Add support to handle user feature commands for get feature Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-19 18:12   ` Jonathan Cameron
  2025-02-18 22:54 ` [PATCH v6 12/14] cxl/test: Add Get Feature support to cxl_test Dave Jiang
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

Add helper function to parse the user data from fwctl RPC ioctl and
send the parsed input parameters to cxl_set_feature() call.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v6:
- Add set features bits checking bits from two patches ago. (Saeed)
- Adjust for embedded input payload
---
 drivers/cxl/core/features.c | 146 +++++++++++++++++++++++++++++++++++-
 include/uapi/fwctl/cxl.h    |   3 +
 2 files changed, 147 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index ec0e3ace1803..f6ca11f14549 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -21,16 +21,21 @@ static const uuid_t cxl_exclusive_feats[] = {
 	CXL_FEAT_RANK_SPARING_UUID,
 };
 
-static bool is_cxl_feature_exclusive(struct cxl_feat_entry *entry)
+static bool is_cxl_feature_exclusive_by_uuid(const uuid_t *uuid)
 {
 	for (int i = 0; i < ARRAY_SIZE(cxl_exclusive_feats); i++) {
-		if (uuid_equal(&entry->uuid, &cxl_exclusive_feats[i]))
+		if (uuid_equal(uuid, &cxl_exclusive_feats[i]))
 			return true;
 	}
 
 	return false;
 }
 
+static bool is_cxl_feature_exclusive(struct cxl_feat_entry *entry)
+{
+	return is_cxl_feature_exclusive_by_uuid(&entry->uuid);
+}
+
 inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds)
 {
 	return cxlds->cxlfs;
@@ -350,6 +355,27 @@ static void cxlctl_close_uctx(struct fwctl_uctx *uctx)
 {
 }
 
+static struct cxl_feat_entry *
+get_support_feature_info(struct cxl_features_state *cxlfs,
+			 const struct fwctl_rpc_cxl *rpc_in)
+{
+	struct cxl_feat_entry *feat;
+	uuid_t *uuid;
+
+	if (rpc_in->op_size < sizeof(uuid))
+		return ERR_PTR(-EINVAL);
+
+	uuid = (uuid_t *)rpc_in->op;
+
+	for (int i = 0; i < cxlfs->entries->num_features; i++) {
+		feat = &cxlfs->entries->ent[i];
+		if (uuid_equal(uuid, &feat->uuid))
+			return feat;
+	}
+
+	return ERR_PTR(-EINVAL);
+}
+
 static void *cxlctl_get_supported_features(struct cxl_features_state *cxlfs,
 					   const struct fwctl_rpc_cxl *rpc_in,
 					   size_t *out_len)
@@ -468,6 +494,116 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
 	return no_free_ptr(rpc_out);
 }
 
+static void *cxlctl_set_feature(struct cxl_features_state *cxlfs,
+				const struct fwctl_rpc_cxl *rpc_in,
+				size_t *out_len)
+{
+	struct cxl_mailbox *cxl_mbox = &cxlfs->cxlds->cxl_mbox;
+	const struct cxl_mbox_set_feat_in *feat_in;
+	size_t out_size, data_size;
+	u16 offset, return_code;
+	u32 flags;
+	int rc;
+
+	if (rpc_in->op_size <= sizeof(struct cxl_mbox_set_feat_hdr))
+		return ERR_PTR(-EINVAL);
+
+	feat_in = &rpc_in->set_feat_in;
+
+	if (is_cxl_feature_exclusive_by_uuid(&feat_in->hdr.uuid))
+		return ERR_PTR(-EPERM);
+
+	offset = le16_to_cpu(feat_in->hdr.offset);
+	flags = le32_to_cpu(feat_in->hdr.flags);
+	out_size = *out_len;
+
+	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
+		kvzalloc(out_size, GFP_KERNEL);
+	if (!rpc_out)
+		return ERR_PTR(-ENOMEM);
+
+	rpc_out->size = 0;
+
+	data_size = rpc_in->op_size - sizeof(feat_in->hdr);
+	rc = cxl_set_feature(cxl_mbox, &feat_in->hdr.uuid,
+			     feat_in->hdr.version, feat_in->feat_data,
+			     data_size, flags, offset, &return_code);
+	if (rc) {
+		rpc_out->retval = return_code;
+		return no_free_ptr(rpc_out);
+	}
+
+	rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
+	*out_len = sizeof(*rpc_out);
+
+	return no_free_ptr(rpc_out);
+}
+
+static bool cxlctl_validate_set_features(struct cxl_features_state *cxlfs,
+					 const struct fwctl_rpc_cxl *rpc_in,
+					 enum fwctl_rpc_scope scope)
+{
+	u16 effects, imm_mask, reset_mask;
+	struct cxl_feat_entry *feat;
+	u32 flags;
+
+	feat = get_support_feature_info(cxlfs, rpc_in);
+	if (IS_ERR(feat))
+		return false;
+
+	/* Ensure that the attribute is changeable */
+	flags = le32_to_cpu(feat->flags);
+	if (!(flags & CXL_FEATURE_F_CHANGEABLE))
+		return false;
+
+	effects = le16_to_cpu(feat->effects);
+
+	/*
+	 * Reserved bits are set, rejecting since the effects is not
+	 * comprehended by the driver.
+	 */
+	if (effects & CXL_CMD_EFFECTS_RESERVED) {
+		dev_warn_once(cxlfs->cxlds->dev,
+			      "Reserved bits set in the Feature effects field!\n");
+		return false;
+	}
+
+	/* Currently no user background command support */
+	if (effects & CXL_CMD_BACKGROUND)
+		return false;
+
+	/* Effects cause immediate change, highest security scope is needed */
+	imm_mask = CXL_CMD_CONFIG_CHANGE_IMMEDIATE |
+		   CXL_CMD_DATA_CHANGE_IMMEDIATE |
+		   CXL_CMD_POLICY_CHANGE_IMMEDIATE |
+		   CXL_CMD_LOG_CHANGE_IMMEDIATE;
+
+	reset_mask = CXL_CMD_CONFIG_CHANGE_COLD_RESET |
+		     CXL_CMD_CONFIG_CHANGE_CONV_RESET |
+		     CXL_CMD_CONFIG_CHANGE_CXL_RESET;
+
+	/* If no immediate or reset effect set, The hardware has a bug */
+	if (!(effects & imm_mask) && !(effects & reset_mask))
+		return false;
+
+	/*
+	 * If the Feature setting causes immediate configuration change
+	 * then we need the full write permission policy.
+	 */
+	if (effects & imm_mask && scope >= FWCTL_RPC_DEBUG_WRITE_FULL)
+		return true;
+
+	/*
+	 * If the Feature setting only causes configuration change
+	 * after a reset, then the lesser level of write permission
+	 * policy is ok.
+	 */
+	if (!(effects & imm_mask) && scope >= FWCTL_RPC_DEBUG_WRITE)
+		return true;
+
+	return false;
+}
+
 static bool cxlctl_validate_hw_command(struct cxl_features_state *cxlfs,
 				       const struct fwctl_rpc_cxl *rpc_in,
 				       enum fwctl_rpc_scope scope,
@@ -483,6 +619,10 @@ static bool cxlctl_validate_hw_command(struct cxl_features_state *cxlfs,
 		if (scope >= FWCTL_RPC_CONFIGURATION)
 			return true;
 		return false;
+	case CXL_MBOX_OP_SET_FEATURE:
+		if (cxl_mbox->feat_cap < CXL_FEATURES_RW)
+			return false;
+		return cxlctl_validate_set_features(cxlfs, rpc_in, scope);
 	default:
 		return false;
 	}
@@ -497,6 +637,8 @@ static void *cxlctl_handle_commands(struct cxl_features_state *cxlfs,
 		return cxlctl_get_supported_features(cxlfs, rpc_in, out_len);
 	case CXL_MBOX_OP_GET_FEATURE:
 		return cxlctl_get_feature(cxlfs, rpc_in, out_len);
+	case CXL_MBOX_OP_SET_FEATURE:
+		return cxlctl_set_feature(cxlfs, rpc_in, out_len);
 	default:
 		return ERR_PTR(-EOPNOTSUPP);
 	}
diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
index 07b7e5bab461..23e8a4a3f533 100644
--- a/include/uapi/fwctl/cxl.h
+++ b/include/uapi/fwctl/cxl.h
@@ -14,6 +14,7 @@
 struct cxl_mbox_get_sup_feats_in;
 struct cxl_mbox_get_sup_feats_out;
 struct cxl_mbox_get_feat_in;
+struct cxl_mbox_set_feat_in;
 
 /**
  * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input for CXL
@@ -23,6 +24,7 @@ struct cxl_mbox_get_feat_in;
  * @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
  * @op: hardware operation input byte array
  */
 struct fwctl_rpc_cxl {
@@ -35,6 +37,7 @@ struct fwctl_rpc_cxl {
 	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;
 		__DECLARE_FLEX_ARRAY(__u8, op);
 	};
 };
-- 
2.48.1


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

* [PATCH v6 12/14] cxl/test: Add Get Feature support to cxl_test
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (10 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 11/14] cxl: Add support to handle user feature commands for set feature Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 13/14] cxl/test: Add Set " Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 14/14] fwctl/cxl: Add documentation to FWCTL CXL Dave Jiang
  13 siblings, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

Add emulation of Get Feature command to cxl_test. The feature for
device patrol scrub is returned by the emulation code. This is
the only feature currently supported by cxl_test. It returns
the information for the device patrol scrub feature.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/mem.c | 56 ++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index 848db399102c..af70932c9bb0 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -48,6 +48,10 @@ static struct cxl_cel_entry mock_cel[] = {
 		.opcode = cpu_to_le16(CXL_MBOX_OP_GET_SUPPORTED_FEATURES),
 		.effect = CXL_CMD_EFFECT_NONE,
 	},
+	{
+		.opcode = cpu_to_le16(CXL_MBOX_OP_GET_FEATURE),
+		.effect = CXL_CMD_EFFECT_NONE,
+	},
 	{
 		.opcode = cpu_to_le16(CXL_MBOX_OP_IDENTIFY),
 		.effect = CXL_CMD_EFFECT_NONE,
@@ -149,6 +153,10 @@ struct mock_event_store {
 	u32 ev_status;
 };
 
+struct vendor_test_feat {
+	__le32 data;
+} __packed;
+
 struct cxl_mockmem_data {
 	void *lsa;
 	void *fw;
@@ -165,6 +173,7 @@ struct cxl_mockmem_data {
 	u8 event_buf[SZ_4K];
 	u64 timestamp;
 	unsigned long sanitize_timeout;
+	struct vendor_test_feat test_feat;
 };
 
 static struct mock_event_log *event_find_log(struct device *dev, int log_type)
@@ -1379,6 +1388,44 @@ static void fill_feature_vendor_test(struct cxl_feat_entry *feat)
 
 #define MAX_CXL_TEST_FEATS	1
 
+static int mock_get_test_feature(struct cxl_mockmem_data *mdata,
+				 struct cxl_mbox_cmd *cmd)
+{
+	struct vendor_test_feat *output = cmd->payload_out;
+	struct cxl_mbox_get_feat_in *input = cmd->payload_in;
+	u16 offset = le16_to_cpu(input->offset);
+	u16 count = le16_to_cpu(input->count);
+	u8 *ptr;
+
+	if (offset > sizeof(*output)) {
+		cmd->return_code = CXL_MBOX_CMD_RC_INPUT;
+		return -EINVAL;
+	}
+
+	if (offset + count > sizeof(*output)) {
+		cmd->return_code = CXL_MBOX_CMD_RC_INPUT;
+		return -EINVAL;
+	}
+
+	ptr = (u8 *)&mdata->test_feat + offset;
+	memcpy((u8 *)output + offset, ptr, count);
+
+	return 0;
+}
+
+static int mock_get_feature(struct cxl_mockmem_data *mdata,
+			    struct cxl_mbox_cmd *cmd)
+{
+	struct cxl_mbox_get_feat_in *input = cmd->payload_in;
+
+	if (uuid_equal(&input->uuid, &CXL_VENDOR_FEATURE_TEST))
+		return mock_get_test_feature(mdata, cmd);
+
+	cmd->return_code = CXL_MBOX_CMD_RC_UNSUPPORTED;
+
+	return -EOPNOTSUPP;
+}
+
 static int mock_get_supported_features(struct cxl_mockmem_data *mdata,
 				       struct cxl_mbox_cmd *cmd)
 {
@@ -1509,6 +1556,9 @@ static int cxl_mock_mbox_send(struct cxl_mailbox *cxl_mbox,
 	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
 		rc = mock_get_supported_features(mdata, cmd);
 		break;
+	case CXL_MBOX_OP_GET_FEATURE:
+		rc = mock_get_feature(mdata, cmd);
+		break;
 	default:
 		break;
 	}
@@ -1556,6 +1606,11 @@ static int cxl_mock_mailbox_create(struct cxl_dev_state *cxlds)
 	return 0;
 }
 
+static void cxl_mock_test_feat_init(struct cxl_mockmem_data *mdata)
+{
+	mdata->test_feat.data = cpu_to_le32(0xdeadbeef);
+}
+
 static int cxl_mock_mem_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1651,6 +1706,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
 		dev_dbg(dev, "No CXL FWCTL setup\n");
 
 	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
+	cxl_mock_test_feat_init(mdata);
 
 	return 0;
 }
-- 
2.48.1


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

* [PATCH v6 13/14] cxl/test: Add Set Feature support to cxl_test
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (11 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 12/14] cxl/test: Add Get Feature support to cxl_test Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  2025-02-18 22:54 ` [PATCH v6 14/14] fwctl/cxl: Add documentation to FWCTL CXL Dave Jiang
  13 siblings, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, Li Ming

Add emulation to support Set Feature mailbox command to cxl_test.
The only feature supported is the device patrol scrub feature. The
set feature allows activation of patrol scrub for the cxl_test
emulated device. The command does not support partial data transfer
even though the spec allows it. This restriction is to reduce complexity
of the emulation given the patrol scrub feature is very minimal.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/mem.c | 51 ++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index af70932c9bb0..9495dbcc03a7 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -52,6 +52,10 @@ static struct cxl_cel_entry mock_cel[] = {
 		.opcode = cpu_to_le16(CXL_MBOX_OP_GET_FEATURE),
 		.effect = CXL_CMD_EFFECT_NONE,
 	},
+	{
+		.opcode = cpu_to_le16(CXL_MBOX_OP_SET_FEATURE),
+		.effect = cpu_to_le16(EFFECT(CONF_CHANGE_IMMEDIATE)),
+	},
 	{
 		.opcode = cpu_to_le16(CXL_MBOX_OP_IDENTIFY),
 		.effect = CXL_CMD_EFFECT_NONE,
@@ -1426,6 +1430,50 @@ static int mock_get_feature(struct cxl_mockmem_data *mdata,
 	return -EOPNOTSUPP;
 }
 
+static int mock_set_test_feature(struct cxl_mockmem_data *mdata,
+				 struct cxl_mbox_cmd *cmd)
+{
+	struct cxl_mbox_set_feat_in *input = cmd->payload_in;
+	struct vendor_test_feat *test =
+		(struct vendor_test_feat *)input->feat_data;
+	u32 action;
+
+	action = FIELD_GET(CXL_SET_FEAT_FLAG_DATA_TRANSFER_MASK,
+			   le32_to_cpu(input->hdr.flags));
+	/*
+	 * While it is spec compliant to support other set actions, it is not
+	 * necessary to add the complication in the emulation currently. Reject
+	 * anything besides full xfer.
+	 */
+	if (action != CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER) {
+		cmd->return_code = CXL_MBOX_CMD_RC_INPUT;
+		return -EINVAL;
+	}
+
+	/* Offset should be reserved when doing full transfer */
+	if (input->hdr.offset) {
+		cmd->return_code = CXL_MBOX_CMD_RC_INPUT;
+		return -EINVAL;
+	}
+
+	memcpy(&mdata->test_feat.data, &test->data, sizeof(u32));
+
+	return 0;
+}
+
+static int mock_set_feature(struct cxl_mockmem_data *mdata,
+			    struct cxl_mbox_cmd *cmd)
+{
+	struct cxl_mbox_set_feat_in *input = cmd->payload_in;
+
+	if (uuid_equal(&input->hdr.uuid, &CXL_VENDOR_FEATURE_TEST))
+		return mock_set_test_feature(mdata, cmd);
+
+	cmd->return_code = CXL_MBOX_CMD_RC_UNSUPPORTED;
+
+	return -EOPNOTSUPP;
+}
+
 static int mock_get_supported_features(struct cxl_mockmem_data *mdata,
 				       struct cxl_mbox_cmd *cmd)
 {
@@ -1559,6 +1607,9 @@ static int cxl_mock_mbox_send(struct cxl_mailbox *cxl_mbox,
 	case CXL_MBOX_OP_GET_FEATURE:
 		rc = mock_get_feature(mdata, cmd);
 		break;
+	case CXL_MBOX_OP_SET_FEATURE:
+		rc = mock_set_feature(mdata, cmd);
+		break;
 	default:
 		break;
 	}
-- 
2.48.1


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

* [PATCH v6 14/14] fwctl/cxl: Add documentation to FWCTL CXL
  2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
                   ` (12 preceding siblings ...)
  2025-02-18 22:54 ` [PATCH v6 13/14] cxl/test: Add Set " Dave Jiang
@ 2025-02-18 22:54 ` Dave Jiang
  13 siblings, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-18 22:54 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed

Add policy and operational documentation for FWCTL CXL.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v6:
- Updated user example due to kernel changes
---
 .../userspace-api/fwctl/fwctl-cxl.rst         | 144 ++++++++++++++++++
 Documentation/userspace-api/fwctl/index.rst   |   1 +
 MAINTAINERS                                   |   1 +
 3 files changed, 146 insertions(+)
 create mode 100644 Documentation/userspace-api/fwctl/fwctl-cxl.rst

diff --git a/Documentation/userspace-api/fwctl/fwctl-cxl.rst b/Documentation/userspace-api/fwctl/fwctl-cxl.rst
new file mode 100644
index 000000000000..4e7b4bae0057
--- /dev/null
+++ b/Documentation/userspace-api/fwctl/fwctl-cxl.rst
@@ -0,0 +1,144 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+================
+fwctl cxl driver
+================
+
+:Author: Dave Jiang
+
+Overview
+========
+
+The CXL spec defines a set of commands that can be issued to the mailbox of a
+CXL device or switch. It also left room for vendor specific commands to be
+issued to the mailbox as well. fwctl provides a path to issue a set of allowed
+mailbox commands from user space to the device moderated by the kernel driver.
+
+The following 3 commands will be used to support CXL Features:
+CXL spec r3.1 8.2.9.6.1 Get Supported Features (Opcode 0500h)
+CXL spec r3.1 8.2.9.6.2 Get Feature (Opcode 0501h)
+CXL spec r3.1 8.2.9.6.3 Set Feature (Opcode 0502h)
+
+The "Get Supported Features" return data may be filtered by the kernel driver to
+drop any features that are forbidden by the kernel or being exclusively used by
+the kernel. The driver will set the "Set Feature Size" of the "Get Supported
+Features Supported Feature Entry" to 0 to indicate that the Feature cannot be
+modified. The "Get Supported Features" command and the "Get Features" falls
+under the fwctl policy of FWCTL_RPC_CONFIGURATION.
+
+For "Set Feature" command, the access policy currently is broken down into two
+categories depending on the Set Feature effects reported by the device. If the
+Set Feature will cause immediate change to the device, the fwctl access policy
+must be FWCTL_RPC_DEBUG_WRITE_FULL. The effects for this level are
+"immediate config change", "immediate data change", "immediate policy change",
+or "immediate log change" for the set effects mask. If the effects are "config
+change with cold reset" or "config change with conventional reset", then the
+fwctl access policy must be FWCTL_RPC_DEBUG_WRITE or higher.
+
+fwctl cxl User API
+==================
+
+.. kernel-doc:: include/uapi/fwctl/cxl.h
+.. kernel-doc:: include/uapi/cxl/features.h
+
+1. Driver info query
+--------------------
+
+First step for the app is to issue the ioctl(FWCTL_CMD_INFO). Successful
+invocation of the ioctl implies the Features capability is operational and
+returns an all zeros 32bit payload. A ``struct fwctl_info`` needs to be filled
+out with the ``fwctl_info.out_device_type`` set to ``FWCTL_DEVICE_TYPE_CXL``.
+The return data should be ``struct fwctl_info_cxl`` that contains a reserved
+32bit field that should be all zeros.
+
+2. Send hardware commands
+-------------------------
+
+Next step is to send the 'Get Supported Features' command to the driver from
+user space via ioctl(FWCTL_RPC). A ``struct fwctl_rpc_cxl`` is pointed to
+by ``fwctl_rpc.in``. ``struct fwctl_rpc_cxl.in_payload`` points to
+the hardware input structure that is defined by the CXL spec. ``fwctl_rpc.out``
+points to the buffer that contains a ``struct fwctl_rpc_cxl_out`` that includes
+the hardware output data inlined as ``fwctl_rpc_cxl_out.payload``. This command
+is called twice. First time to retrieve the number of features supported.
+A second time to retrieve the specific feature details as the output data.
+
+After getting the specific feature details, a Get/Set Feature command can be
+appropriately programmed and sent. For a "Set Feature" command, the retrieved
+feature info contains an effects field that details the resulting
+"Set Feature" command will trigger. That will inform the user whether
+the system is configured to allowed the "Set Feature" command or not.
+
+Code example of a Get Feature
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+        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;
+                struct fwctl_rpc rpc = {0};
+                struct fwctl_rpc_cxl *in;
+                size_t out_size, in_size;
+                uint32_t val;
+                void *data;
+                int rc;
+
+                in_size = sizeof(*in) + sizeof(*feat_in);
+                rc = posix_memalign((void **)&in, 16, in_size);
+                if (rc)
+                        return -ENOMEM;
+                memset(in, 0, in_size);
+                feat_in = &in->get_feat_in;
+
+                uuid_copy(feat_in->uuid, feat_ctx->uuid);
+                feat_in->count = feat_ctx->get_size;
+
+                out_size = sizeof(*out) + feat_ctx->get_size;
+                rc = posix_memalign((void **)&out, 16, out_size);
+                if (rc)
+                        goto free_in;
+                memset(out, 0, out_size);
+
+                in->opcode = CXL_MBOX_OPCODE_GET_FEATURE;
+                in->op_size = sizeof(*feat_in);
+
+                rpc.size = sizeof(rpc);
+                rpc.scope = FWCTL_RPC_CONFIGURATION;
+                rpc.in_len = in_size;
+                rpc.out_len = out_size;
+                rpc.in = (uint64_t)(uint64_t *)in;
+                rpc.out = (uint64_t)(uint64_t *)out;
+
+                rc = send_command(fd, &rpc, out);
+                if (rc)
+                        goto free_all;
+
+                data = out->payload;
+                val = le32toh(*(__le32 *)data);
+                if (memcmp(&val, &expected_data, sizeof(val)) != 0) {
+                        rc = -ENXIO;
+                        goto free_all;
+                }
+
+        free_all:
+                free(out);
+        free_in:
+                free(in);
+                return rc;
+        }
+
+Take a look at CXL CLI test directory
+<https://github.com/pmem/ndctl/tree/main/test/fwctl.c> for a detailed user code
+for examples on how to exercise this path.
+
+
+fwctl cxl Kernel API
+====================
+
+.. kernel-doc:: drivers/cxl/core/features.c
+.. kernel-doc:: drivers/cxl/features.c
+   :export:
+.. kernel-doc:: include/cxl/features.h
diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
index 06959fbf1547..d9d40a468a31 100644
--- a/Documentation/userspace-api/fwctl/index.rst
+++ b/Documentation/userspace-api/fwctl/index.rst
@@ -10,3 +10,4 @@ to securely construct and execute RPCs inside device firmware.
    :maxdepth: 1
 
    fwctl
+   fwctl-cxl
diff --git a/MAINTAINERS b/MAINTAINERS
index 413ab79bf2f4..b450c960687c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5837,6 +5837,7 @@ M:	Dan Williams <dan.j.williams@intel.com>
 L:	linux-cxl@vger.kernel.org
 S:	Maintained
 F:	Documentation/driver-api/cxl
+F:	Documentation/userspace-api/fwctl/fwctl-cxl.rst
 F:	drivers/cxl/
 F:	include/cxl/
 F:	include/uapi/linux/cxl_mem.h
-- 
2.48.1


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

* Re: [PATCH v6 07/14] cxl: Add FWCTL support to CXL
  2025-02-18 22:54 ` [PATCH v6 07/14] cxl: Add FWCTL support to CXL Dave Jiang
@ 2025-02-19  7:25   ` Li Ming
  2025-02-19 17:09   ` Jonathan Cameron
  1 sibling, 0 replies; 29+ messages in thread
From: Li Ming @ 2025-02-19  7:25 UTC (permalink / raw)
  To: Dave Jiang
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, linux-cxl

On 2/19/2025 6:54 AM, Dave Jiang wrote:
> Add fwctl support code to allow sending of CXL feature commands from
> userspace through as ioctls via FWCTL. Provide initial setup bits. The
> CXL PCI probe function will call cxl_setup_fwctl() after the cxl_memdev
> has been enumerated in order to setup FWCTL char device under the
> cxl_memdev like the existing memdev char device for issuing CXL raw
> mailbox commands from userspace via ioctls.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>

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

* Re: [PATCH v6 08/14] cxl: Move cxl feature command structs to user header
  2025-02-18 22:54 ` [PATCH v6 08/14] cxl: Move cxl feature command structs to user header Dave Jiang
@ 2025-02-19  7:26   ` Li Ming
  0 siblings, 0 replies; 29+ messages in thread
From: Li Ming @ 2025-02-19  7:26 UTC (permalink / raw)
  To: Dave Jiang
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, linux-cxl

On 2/19/2025 6:54 AM, Dave Jiang wrote:
> In preparation for cxl fwctl enabling, move data structures related to
> cxl feature commands to a user header file.
>
> Reviewed-by; Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>

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

* Re: [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands
  2025-02-18 22:54 ` [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands Dave Jiang
@ 2025-02-19  7:27   ` Li Ming
  2025-02-19 17:53   ` Jonathan Cameron
  1 sibling, 0 replies; 29+ messages in thread
From: Li Ming @ 2025-02-19  7:27 UTC (permalink / raw)
  To: Dave Jiang
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	Jonathan.Cameron, dave, jgg, shiju.jose, saeed, linux-cxl

On 2/19/2025 6:54 AM, Dave Jiang wrote:
> fwctl provides a fwctl_ops->fw_rpc() callback in order to issue ioctls
> to a device. The cxl fwctl driver will start by supporting the CXL
> Feature commands: Get Supported Features, Get Feature, and Set Feature.
>
> The fw_rpc() callback provides 'enum fwctl_rpc_scope' parameter where
> it indicates the security scope of the call. The Get Supported Features
> and Get Feature calls can be executed with the scope of
> FWCTL_RPC_CONFIGRATION. The Set Feature call is gated by the effects
> of the Feature reported by Get Supported Features call for the specific
> Feature.
>
> Only "Get Supported Features" is supported in this patch. Additional
> commands will be added in follow on patches. "Get Supported Features"
> will filter the Features that are exclusive to the kernel. The flag
> field of the Feature details will be cleared of the "Changeable"
> field and the "set feat size" will be set to 0 to indicate that
> the feature is not changeable.
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>

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

* Re: [PATCH v6 07/14] cxl: Add FWCTL support to CXL
  2025-02-18 22:54 ` [PATCH v6 07/14] cxl: Add FWCTL support to CXL Dave Jiang
  2025-02-19  7:25   ` Li Ming
@ 2025-02-19 17:09   ` Jonathan Cameron
  1 sibling, 0 replies; 29+ messages in thread
From: Jonathan Cameron @ 2025-02-19 17:09 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, dave, jgg, shiju.jose, saeed

On Tue, 18 Feb 2025 15:54:36 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add fwctl support code to allow sending of CXL feature commands from
> userspace through as ioctls via FWCTL. Provide initial setup bits. The
> CXL PCI probe function will call cxl_setup_fwctl() after the cxl_memdev
> has been enumerated in order to setup FWCTL char device under the
> cxl_memdev like the existing memdev char device for issuing CXL raw
> mailbox commands from userspace via ioctls.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


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

* Re: [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands
  2025-02-18 22:54 ` [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands Dave Jiang
  2025-02-19  7:27   ` Li Ming
@ 2025-02-19 17:53   ` Jonathan Cameron
  2025-02-19 17:56     ` Jason Gunthorpe
  2025-02-19 18:29     ` Dave Jiang
  1 sibling, 2 replies; 29+ messages in thread
From: Jonathan Cameron @ 2025-02-19 17:53 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, dave, jgg, shiju.jose, saeed

On Tue, 18 Feb 2025 15:54:38 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> fwctl provides a fwctl_ops->fw_rpc() callback in order to issue ioctls
> to a device. The cxl fwctl driver will start by supporting the CXL
> Feature commands: Get Supported Features, Get Feature, and Set Feature.
> 
> The fw_rpc() callback provides 'enum fwctl_rpc_scope' parameter where
> it indicates the security scope of the call. The Get Supported Features
> and Get Feature calls can be executed with the scope of
> FWCTL_RPC_CONFIGRATION. The Set Feature call is gated by the effects
> of the Feature reported by Get Supported Features call for the specific
> Feature.
> 
> Only "Get Supported Features" is supported in this patch. Additional
> commands will be added in follow on patches. "Get Supported Features"
> will filter the Features that are exclusive to the kernel. The flag
> field of the Feature details will be cleared of the "Changeable"
> field and the "set feat size" will be set to 0 to indicate that
> the feature is not changeable.
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Hi Dave,

I'm probably missing something but I don't understand the need for
__DECLARE_FLEXIBLE_ARRAY() in the unions.  We always seem to use one of
the structs (some of which have trailing flexible arrays).


> ---
> v6:
> - Embed hw op in 'fwctl_rpc_cxl'. (Saeed, Jason)
> - Move set_features bits to set_features enabling patch. (Saeed)
> - Fix copyright years. (Jason)
> ---
>  drivers/cxl/core/features.c | 121 +++++++++++++++++++++++++++++++++++-
>  include/uapi/cxl/features.h |   1 +
>  include/uapi/fwctl/cxl.h    |  57 +++++++++++++++++
>  3 files changed, 177 insertions(+), 2 deletions(-)
>  create mode 100644 include/uapi/fwctl/cxl.h
> 
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 846de3294a5e..106ea9e25c82 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -4,6 +4,7 @@
>  #include <linux/device.h>
>  #include <cxl/mailbox.h>
>  #include <cxl/features.h>
> +#include <uapi/fwctl/cxl.h>
>  #include "cxl.h"
>  #include "core.h"
>  #include "cxlmem.h"
> @@ -349,11 +350,127 @@ static void cxlctl_close_uctx(struct fwctl_uctx *uctx)
>  {
>  }
>  
> +static void *cxlctl_get_supported_features(struct cxl_features_state *cxlfs,
> +					   const struct fwctl_rpc_cxl *rpc_in,
> +					   size_t *out_len)
> +{
> +	const struct cxl_mbox_get_sup_feats_in *feat_in;
> +	struct cxl_mbox_get_sup_feats_out *feat_out;
> +	struct cxl_feat_entry *pos;
> +	size_t out_size;
> +	int requested;
> +	u32 count;
> +	u16 start;
> +	int i;
> +
> +	if (rpc_in->op_size != sizeof(*feat_in))
> +		return ERR_PTR(-EINVAL);
> +
> +	feat_in = &rpc_in->get_sup_feats_in;
> +	count = le32_to_cpu(feat_in->count);
> +	start = le16_to_cpu(feat_in->start_idx);
> +	requested = count / sizeof(*pos);
> +
> +	/*
> +	 * Make sure that the total requested number of entries is not greater
> +	 * than the total number of supported features allowed for userspace.
> +	 */
> +	if (start >= cxlfs->entries->num_features)
> +		return ERR_PTR(-EINVAL);
> +
> +	requested = min_t(int, requested, cxlfs->entries->num_features - start);
> +
> +	out_size = sizeof(struct fwctl_rpc_cxl_out) +
> +		struct_size(feat_out, ents, requested);
> +
> +	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> +		kvzalloc(out_size, GFP_KERNEL);
> +	if (!rpc_out)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rpc_out->size = struct_size(feat_out, ents, requested);
> +	feat_out = (struct cxl_mbox_get_sup_feats_out *)rpc_out->payload;

Why not, 

	feat_out = rpc_out->get_sup_feats_out;
?


> +	if (requested == 0) {
> +		feat_out->num_entries = cpu_to_le16(requested);
> +		feat_out->supported_feats =
> +			cpu_to_le16(cxlfs->entries->num_features);
> +		rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
> +		*out_len = out_size;
> +		return no_free_ptr(rpc_out);
> +	}
> +
> +	for (i = start, pos = &feat_out->ents[0];
> +	     i < cxlfs->entries->num_features; i++, pos++) {
> +		if (i - start == requested)
> +			break;
> +
> +		memcpy(pos, &cxlfs->entries->ent[i], sizeof(*pos));
> +		/*
> +		 * If the feature is exclusive, set the set_feat_size to 0 to
> +		 * indicate that the feature is not changeable.
> +		 */
> +		if (is_cxl_feature_exclusive(pos)) {
> +			u32 flags;
> +
> +			pos->set_feat_size = 0;
> +			flags = le32_to_cpu(pos->flags);
> +			flags &= ~CXL_FEATURE_F_CHANGEABLE;
> +			pos->flags = cpu_to_le32(flags);
> +		}
> +	}
> +
> +	feat_out->num_entries = cpu_to_le16(requested);
> +	feat_out->supported_feats = cpu_to_le16(cxlfs->entries->num_features);
> +	rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
> +	*out_len = out_size;
> +
> +	return no_free_ptr(rpc_out);
> +}

> diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
> new file mode 100644
> index 000000000000..39996ec56816
> --- /dev/null
> +++ b/include/uapi/fwctl/cxl.h
> @@ -0,0 +1,57 @@
> +/* 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 cxl_mbox_get_sup_feats_in;
> +struct cxl_mbox_get_sup_feats_out;

These are defined now in uapi/cxl/features.h anyway so why is forwards
def needed?

> +
> +/**
> + * 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
> + * @op: hardware operation input byte array
> + */
> +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;
> +		__DECLARE_FLEX_ARRAY(__u8, op);

Similar to below. op isn't used in this patch that I can see.
Is it just here so it's obvious what op_size refers to?

> +	};
> +};
> +
> +/**
> + * 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: Return data from device
> + */
> +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);

I this patch I don't see a particular use for payload other
than where I've noted I think you can use get_sup_feats_out
directly.

> +	};
> +};
> +
> +#endif


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

* Re: [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands
  2025-02-19 17:53   ` Jonathan Cameron
@ 2025-02-19 17:56     ` Jason Gunthorpe
  2025-02-19 18:00       ` Dave Jiang
  2025-02-19 18:29     ` Dave Jiang
  1 sibling, 1 reply; 29+ messages in thread
From: Jason Gunthorpe @ 2025-02-19 17:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Dave Jiang, linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, dave, shiju.jose, saeed

On Wed, Feb 19, 2025 at 05:53:12PM +0000, Jonathan Cameron wrote:

> I'm probably missing something but I don't understand the need for
> __DECLARE_FLEXIBLE_ARRAY() in the unions.  We always seem to use one of
> the structs (some of which have trailing flexible arrays).

Can you even use those defines in a uapi header???

Jason

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

* Re: [PATCH v6 10/14] cxl: Add support to handle user feature commands for get feature
  2025-02-18 22:54 ` [PATCH v6 10/14] cxl: Add support to handle user feature commands for get feature Dave Jiang
@ 2025-02-19 17:57   ` Jonathan Cameron
  0 siblings, 0 replies; 29+ messages in thread
From: Jonathan Cameron @ 2025-02-19 17:57 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, dave, jgg, shiju.jose, saeed, Li Ming

On Tue, 18 Feb 2025 15:54:39 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add helper function to parse the user data from fwctl RPC ioctl and
> send the parsed input parameters to cxl_get_feature() call.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Li Ming <ming.li@zohomail.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v6:
> - adjust for embedded input payload
> - move get_feature bits here from previous patch

One of those flexible arrays gets used here I see..  If
this is first place it is needed maybe drag the definition into
here?


> ---
>  drivers/cxl/core/features.c | 44 +++++++++++++++++++++++++++++++++++++
>  include/uapi/fwctl/cxl.h    |  3 +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 106ea9e25c82..ec0e3ace1803 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -427,6 +427,47 @@ static void *cxlctl_get_supported_features(struct cxl_features_state *cxlfs,
>  	return no_free_ptr(rpc_out);
>  }
>  
> +static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
> +				const struct fwctl_rpc_cxl *rpc_in,
> +				size_t *out_len)
> +{
> +	struct cxl_mailbox *cxl_mbox = &cxlfs->cxlds->cxl_mbox;
> +	const struct cxl_mbox_get_feat_in *feat_in;
> +	u16 offset, count, return_code;
> +	size_t out_size = *out_len;
> +
> +	if (rpc_in->op_size != sizeof(*feat_in))
> +		return ERR_PTR(-EINVAL);
> +
> +	feat_in = &rpc_in->get_feat_in;
> +	offset = le16_to_cpu(feat_in->offset);
> +	count = le16_to_cpu(feat_in->count);
> +
> +	if (!count)
> +		return ERR_PTR(-EINVAL);
> +
> +	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> +		kvzalloc(out_size, GFP_KERNEL);
> +	if (!rpc_out)
> +		return ERR_PTR(-ENOMEM);
> +
> +	out_size = cxl_get_feature(cxl_mbox, &feat_in->uuid,
> +				   feat_in->selection, rpc_out->payload,

Ah. You are using the flexible array here to provide a void *
That's fine but maybe drag defining it into this patch.

> +				   count, offset, &return_code);
> +	*out_len = sizeof(struct fwctl_rpc_cxl_out);
> +	if (!out_size) {
> +		rpc_out->size = 0;
> +		rpc_out->retval = return_code;
> +		return no_free_ptr(rpc_out);
> +	}
> +
> +	rpc_out->size = out_size;
> +	rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
> +	*out_len += out_size;
> +
> +	return no_free_ptr(rpc_out);
> +}
> +
>  static bool cxlctl_validate_hw_command(struct cxl_features_state *cxlfs,
>  				       const struct fwctl_rpc_cxl *rpc_in,
>  				       enum fwctl_rpc_scope scope,
> @@ -436,6 +477,7 @@ static bool cxlctl_validate_hw_command(struct cxl_features_state *cxlfs,
>  
>  	switch (opcode) {
>  	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
> +	case CXL_MBOX_OP_GET_FEATURE:
>  		if (cxl_mbox->feat_cap < CXL_FEATURES_RO)
>  			return false;
>  		if (scope >= FWCTL_RPC_CONFIGURATION)
> @@ -453,6 +495,8 @@ static void *cxlctl_handle_commands(struct cxl_features_state *cxlfs,
>  	switch (opcode) {
>  	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
>  		return cxlctl_get_supported_features(cxlfs, rpc_in, out_len);
> +	case CXL_MBOX_OP_GET_FEATURE:
> +		return cxlctl_get_feature(cxlfs, rpc_in, out_len);
>  	default:
>  		return ERR_PTR(-EOPNOTSUPP);
>  	}
> diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
> index 39996ec56816..07b7e5bab461 100644
> --- a/include/uapi/fwctl/cxl.h
> +++ b/include/uapi/fwctl/cxl.h
> @@ -13,6 +13,7 @@
>  
>  struct cxl_mbox_get_sup_feats_in;
>  struct cxl_mbox_get_sup_feats_out;
> +struct cxl_mbox_get_feat_in;
>  
>  /**
>   * struct fwctl_rpc_cxl - ioctl(FWCTL_RPC) input for CXL
> @@ -21,6 +22,7 @@ struct cxl_mbox_get_sup_feats_out;
>   * @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
>   * @op: hardware operation input byte array
>   */
>  struct fwctl_rpc_cxl {
> @@ -32,6 +34,7 @@ struct fwctl_rpc_cxl {
>  	);
>  	union {
>  		struct cxl_mbox_get_sup_feats_in get_sup_feats_in;
> +		struct cxl_mbox_get_feat_in get_feat_in;
>  		__DECLARE_FLEX_ARRAY(__u8, op);
>  	};
>  };


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

* Re: [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands
  2025-02-19 17:56     ` Jason Gunthorpe
@ 2025-02-19 18:00       ` Dave Jiang
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-19 18:00 UTC (permalink / raw)
  To: Jason Gunthorpe, Jonathan Cameron
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, dave, shiju.jose, saeed



On 2/19/25 10:56 AM, Jason Gunthorpe wrote:
> On Wed, Feb 19, 2025 at 05:53:12PM +0000, Jonathan Cameron wrote:
> 
>> I'm probably missing something but I don't understand the need for
>> __DECLARE_FLEXIBLE_ARRAY() in the unions.  We always seem to use one of
>> the structs (some of which have trailing flexible arrays).
> 
> Can you even use those defines in a uapi header???

Yes. It's defined in uapi/linux/stddef.h. I compiled and tested against the unit test in userspace. No issues.

> 
> Jason
> 


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

* Re: [PATCH v6 11/14] cxl: Add support to handle user feature commands for set feature
  2025-02-18 22:54 ` [PATCH v6 11/14] cxl: Add support to handle user feature commands for set feature Dave Jiang
@ 2025-02-19 18:12   ` Jonathan Cameron
  2025-02-19 18:31     ` Dave Jiang
  0 siblings, 1 reply; 29+ messages in thread
From: Jonathan Cameron @ 2025-02-19 18:12 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, dave, jgg, shiju.jose, saeed, Li Ming

On Tue, 18 Feb 2025 15:54:40 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add helper function to parse the user data from fwctl RPC ioctl and
> send the parsed input parameters to cxl_set_feature() call.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Li Ming <ming.li@zohomail.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v6:
> - Add set features bits checking bits from two patches ago. (Saeed)
> - Adjust for embedded input payload

Follow on from earlier.  Now I see where op is used.
I think we can do better however.

>  
> +static struct cxl_feat_entry *
> +get_support_feature_info(struct cxl_features_state *cxlfs,
> +			 const struct fwctl_rpc_cxl *rpc_in)
> +{
> +	struct cxl_feat_entry *feat;
> +	uuid_t *uuid;
> +
> +	if (rpc_in->op_size < sizeof(uuid))
> +		return ERR_PTR(-EINVAL);
> +
> +	uuid = (uuid_t *)rpc_in->op;

hmm. So here you use the op, but we know this a set features, so
much like in previous patch we could use

rpc_in->set_feat_in->uuid

That is relying on the fact we know this is being called on a
set feature which the name doesn't imply. So maybe rename?

Or.. check rpc_in->opcode and grab it from rpc_in->get_feat_in->uuid or
rpc_in->set_feat_in->uuid as appropriate.
and error out on anything else as we don't 'know' for sure there
is a uuid at that location.

> +
> +	for (int i = 0; i < cxlfs->entries->num_features; i++) {
> +		feat = &cxlfs->entries->ent[i];
> +		if (uuid_equal(uuid, &feat->uuid))
> +			return feat;
> +	}
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +



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

* Re: [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands
  2025-02-19 17:53   ` Jonathan Cameron
  2025-02-19 17:56     ` Jason Gunthorpe
@ 2025-02-19 18:29     ` Dave Jiang
  1 sibling, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-19 18:29 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, dave, jgg, shiju.jose, saeed



On 2/19/25 10:53 AM, Jonathan Cameron wrote:
> On Tue, 18 Feb 2025 15:54:38 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> fwctl provides a fwctl_ops->fw_rpc() callback in order to issue ioctls
>> to a device. The cxl fwctl driver will start by supporting the CXL
>> Feature commands: Get Supported Features, Get Feature, and Set Feature.
>>
>> The fw_rpc() callback provides 'enum fwctl_rpc_scope' parameter where
>> it indicates the security scope of the call. The Get Supported Features
>> and Get Feature calls can be executed with the scope of
>> FWCTL_RPC_CONFIGRATION. The Set Feature call is gated by the effects
>> of the Feature reported by Get Supported Features call for the specific
>> Feature.
>>
>> Only "Get Supported Features" is supported in this patch. Additional
>> commands will be added in follow on patches. "Get Supported Features"
>> will filter the Features that are exclusive to the kernel. The flag
>> field of the Feature details will be cleared of the "Changeable"
>> field and the "set feat size" will be set to 0 to indicate that
>> the feature is not changeable.
>>
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Hi Dave,
> 
> I'm probably missing something but I don't understand the need for
> __DECLARE_FLEXIBLE_ARRAY() in the unions.  We always seem to use one of
> the structs (some of which have trailing flexible arrays).

There are some usages. I probably should move the addition of that to the said patch. For example, feature data output is a byte stream. 

> 
> 
>> ---
>> v6:
>> - Embed hw op in 'fwctl_rpc_cxl'. (Saeed, Jason)
>> - Move set_features bits to set_features enabling patch. (Saeed)
>> - Fix copyright years. (Jason)
>> ---
>>  drivers/cxl/core/features.c | 121 +++++++++++++++++++++++++++++++++++-
>>  include/uapi/cxl/features.h |   1 +
>>  include/uapi/fwctl/cxl.h    |  57 +++++++++++++++++
>>  3 files changed, 177 insertions(+), 2 deletions(-)
>>  create mode 100644 include/uapi/fwctl/cxl.h
>>
>> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
>> index 846de3294a5e..106ea9e25c82 100644
>> --- a/drivers/cxl/core/features.c
>> +++ b/drivers/cxl/core/features.c
>> @@ -4,6 +4,7 @@
>>  #include <linux/device.h>
>>  #include <cxl/mailbox.h>
>>  #include <cxl/features.h>
>> +#include <uapi/fwctl/cxl.h>
>>  #include "cxl.h"
>>  #include "core.h"
>>  #include "cxlmem.h"
>> @@ -349,11 +350,127 @@ static void cxlctl_close_uctx(struct fwctl_uctx *uctx)
>>  {
>>  }
>>  
>> +static void *cxlctl_get_supported_features(struct cxl_features_state *cxlfs,
>> +					   const struct fwctl_rpc_cxl *rpc_in,
>> +					   size_t *out_len)
>> +{
>> +	const struct cxl_mbox_get_sup_feats_in *feat_in;
>> +	struct cxl_mbox_get_sup_feats_out *feat_out;
>> +	struct cxl_feat_entry *pos;
>> +	size_t out_size;
>> +	int requested;
>> +	u32 count;
>> +	u16 start;
>> +	int i;
>> +
>> +	if (rpc_in->op_size != sizeof(*feat_in))
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	feat_in = &rpc_in->get_sup_feats_in;
>> +	count = le32_to_cpu(feat_in->count);
>> +	start = le16_to_cpu(feat_in->start_idx);
>> +	requested = count / sizeof(*pos);
>> +
>> +	/*
>> +	 * Make sure that the total requested number of entries is not greater
>> +	 * than the total number of supported features allowed for userspace.
>> +	 */
>> +	if (start >= cxlfs->entries->num_features)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	requested = min_t(int, requested, cxlfs->entries->num_features - start);
>> +
>> +	out_size = sizeof(struct fwctl_rpc_cxl_out) +
>> +		struct_size(feat_out, ents, requested);
>> +
>> +	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
>> +		kvzalloc(out_size, GFP_KERNEL);
>> +	if (!rpc_out)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	rpc_out->size = struct_size(feat_out, ents, requested);
>> +	feat_out = (struct cxl_mbox_get_sup_feats_out *)rpc_out->payload;
> 
> Why not, 
> 
> 	feat_out = rpc_out->get_sup_feats_out;
> ?

I missed that one during refactoring.

> 
> 
>> +	if (requested == 0) {
>> +		feat_out->num_entries = cpu_to_le16(requested);
>> +		feat_out->supported_feats =
>> +			cpu_to_le16(cxlfs->entries->num_features);
>> +		rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
>> +		*out_len = out_size;
>> +		return no_free_ptr(rpc_out);
>> +	}
>> +
>> +	for (i = start, pos = &feat_out->ents[0];
>> +	     i < cxlfs->entries->num_features; i++, pos++) {
>> +		if (i - start == requested)
>> +			break;
>> +
>> +		memcpy(pos, &cxlfs->entries->ent[i], sizeof(*pos));
>> +		/*
>> +		 * If the feature is exclusive, set the set_feat_size to 0 to
>> +		 * indicate that the feature is not changeable.
>> +		 */
>> +		if (is_cxl_feature_exclusive(pos)) {
>> +			u32 flags;
>> +
>> +			pos->set_feat_size = 0;
>> +			flags = le32_to_cpu(pos->flags);
>> +			flags &= ~CXL_FEATURE_F_CHANGEABLE;
>> +			pos->flags = cpu_to_le32(flags);
>> +		}
>> +	}
>> +
>> +	feat_out->num_entries = cpu_to_le16(requested);
>> +	feat_out->supported_feats = cpu_to_le16(cxlfs->entries->num_features);
>> +	rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
>> +	*out_len = out_size;
>> +
>> +	return no_free_ptr(rpc_out);
>> +}
> 
>> diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
>> new file mode 100644
>> index 000000000000..39996ec56816
>> --- /dev/null
>> +++ b/include/uapi/fwctl/cxl.h
>> @@ -0,0 +1,57 @@
>> +/* 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 cxl_mbox_get_sup_feats_in;
>> +struct cxl_mbox_get_sup_feats_out;
> 
> These are defined now in uapi/cxl/features.h anyway so why is forwards
> def needed?

I'll drop those.

> 
>> +
>> +/**
>> + * 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
>> + * @op: hardware operation input byte array
>> + */
>> +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;
>> +		__DECLARE_FLEX_ARRAY(__u8, op);
> 
> Similar to below. op isn't used in this patch that I can see.
> Is it just here so it's obvious what op_size refers to?

Yes. and also I cast it to the feature uuid field as the common header. But I can move this addition to the get_feature enabling.

> 
>> +	};
>> +};
>> +
>> +/**
>> + * 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: Return data from device
>> + */
>> +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);
> 
> I this patch I don't see a particular use for payload other
> than where I've noted I think you can use get_sup_feats_out
> directly.

The get_feature has a byte stream for feature data as output. It's also convenient for the user app to access the feature data.  

> 
>> +	};
>> +};
>> +
>> +#endif
> 


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

* Re: [PATCH v6 11/14] cxl: Add support to handle user feature commands for set feature
  2025-02-19 18:12   ` Jonathan Cameron
@ 2025-02-19 18:31     ` Dave Jiang
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Jiang @ 2025-02-19 18:31 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, dave, jgg, shiju.jose, saeed, Li Ming



On 2/19/25 11:12 AM, Jonathan Cameron wrote:
> On Tue, 18 Feb 2025 15:54:40 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> Add helper function to parse the user data from fwctl RPC ioctl and
>> send the parsed input parameters to cxl_set_feature() call.
>>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> Reviewed-by: Li Ming <ming.li@zohomail.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v6:
>> - Add set features bits checking bits from two patches ago. (Saeed)
>> - Adjust for embedded input payload
> 
> Follow on from earlier.  Now I see where op is used.
> I think we can do better however.
> 
>>  
>> +static struct cxl_feat_entry *
>> +get_support_feature_info(struct cxl_features_state *cxlfs,
>> +			 const struct fwctl_rpc_cxl *rpc_in)
>> +{
>> +	struct cxl_feat_entry *feat;
>> +	uuid_t *uuid;
>> +
>> +	if (rpc_in->op_size < sizeof(uuid))
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	uuid = (uuid_t *)rpc_in->op;
> 
> hmm. So here you use the op, but we know this a set features, so
> much like in previous patch we could use
> 
> rpc_in->set_feat_in->uuid
> 
> That is relying on the fact we know this is being called on a
> set feature which the name doesn't imply. So maybe rename?
> 
> Or.. check rpc_in->opcode and grab it from rpc_in->get_feat_in->uuid or
> rpc_in->set_feat_in->uuid as appropriate.
> and error out on anything else as we don't 'know' for sure there
> is a uuid at that location.

Yes that makes sense. I'll drop the flex array in the input data struct. 

> 
>> +
>> +	for (int i = 0; i < cxlfs->entries->num_features; i++) {
>> +		feat = &cxlfs->entries->ent[i];
>> +		if (uuid_equal(uuid, &feat->uuid))
>> +			return feat;
>> +	}
>> +
>> +	return ERR_PTR(-EINVAL);
>> +}
>> +
> 
> 
> 


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

* RE: [PATCH v6 01/14] cxl: Enumerate feature commands
  2025-02-18 22:54 ` [PATCH v6 01/14] cxl: Enumerate feature commands Dave Jiang
@ 2025-02-20 15:37   ` Shiju Jose
  0 siblings, 0 replies; 29+ messages in thread
From: Shiju Jose @ 2025-02-20 15:37 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl@vger.kernel.org
  Cc: dan.j.williams@intel.com, ira.weiny@intel.com,
	vishal.l.verma@intel.com, alison.schofield@intel.com,
	Jonathan Cameron, dave@stgolabs.net, jgg@nvidia.com,
	saeed@kernel.org, Li Ming

>-----Original Message-----
>From: Dave Jiang <dave.jiang@intel.com>
>Sent: 18 February 2025 22:55
>To: linux-cxl@vger.kernel.org
>Cc: dan.j.williams@intel.com; ira.weiny@intel.com; vishal.l.verma@intel.com;
>alison.schofield@intel.com; Jonathan Cameron
><jonathan.cameron@huawei.com>; dave@stgolabs.net; jgg@nvidia.com; Shiju
>Jose <shiju.jose@huawei.com>; saeed@kernel.org; Li Ming
><ming.li@zohomail.com>
>Subject: [PATCH v6 01/14] cxl: Enumerate feature commands
>
>Add feature commands enumeration code in order to detect and enumerate the
>3 feature related commands "get supported features", "get feature", and "set
>feature". The enumeration will help determine whether the driver can issue any
>of the 3 commands to the device.
>
>Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>Reviewed-by: Li Ming <ming.li@zohomail.com>
>Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Tested-by: Shiju Jose <shiju.jose@huawei.com>

>---
> drivers/cxl/core/mbox.c | 36 +++++++++++++++++++++++++++++++++++-
> drivers/cxl/cxlmem.h    |  3 +++
> include/cxl/features.h  | 13 +++++++++++++
> include/cxl/mailbox.h   |  3 +++
> 4 files changed, 54 insertions(+), 1 deletion(-)  create mode 100644
>include/cxl/features.h
>
>diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index
>9c1b9e353e3e..78c5346e3e89 100644
>--- a/drivers/cxl/core/mbox.c
>+++ b/drivers/cxl/core/mbox.c
>@@ -706,6 +706,35 @@ static int cxl_xfer_log(struct cxl_memdev_state *mds,
>uuid_t *uuid,
> 	return 0;
> }
>
>+static int check_features_opcodes(u16 opcode, int *ro_cmds, int
>+*wr_cmds) {
>+	switch (opcode) {
>+	case CXL_MBOX_OP_GET_SUPPORTED_FEATURES:
>+	case CXL_MBOX_OP_GET_FEATURE:
>+		(*ro_cmds)++;
>+		return 1;
>+	case CXL_MBOX_OP_SET_FEATURE:
>+		(*wr_cmds)++;
>+		return 1;
>+	default:
>+		return 0;
>+	}
>+}
>+
>+/* 'Get Supported Features' and 'Get Feature' */
>+#define MAX_FEATURES_READ_CMDS	2
>+static void set_features_cap(struct cxl_mailbox *cxl_mbox,
>+			     int ro_cmds, int wr_cmds)
>+{
>+	/* Setting up Features capability while walking the CEL */
>+	if (ro_cmds == MAX_FEATURES_READ_CMDS) {
>+		if (wr_cmds)
>+			cxl_mbox->feat_cap = CXL_FEATURES_RW;
>+		else
>+			cxl_mbox->feat_cap = CXL_FEATURES_RO;
>+	}
>+}
>+
> /**
>  * cxl_walk_cel() - Walk through the Command Effects Log.
>  * @mds: The driver data for the operation @@ -721,7 +750,7 @@ static void
>cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
> 	struct cxl_cel_entry *cel_entry;
> 	const int cel_entries = size / sizeof(*cel_entry);
> 	struct device *dev = mds->cxlds.dev;
>-	int i;
>+	int i, ro_cmds = 0, wr_cmds = 0;
>
> 	cel_entry = (struct cxl_cel_entry *) cel;
>
>@@ -735,6 +764,9 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds,
>size_t size, u8 *cel)
> 			enabled++;
> 		}
>
>+		enabled += check_features_opcodes(opcode, &ro_cmds,
>+						  &wr_cmds);
>+
> 		if (cxl_is_poison_command(opcode)) {
> 			cxl_set_poison_cmd_enabled(&mds->poison, opcode);
> 			enabled++;
>@@ -748,6 +780,8 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds,
>size_t size, u8 *cel)
> 		dev_dbg(dev, "Opcode 0x%04x %s\n", opcode,
> 			enabled ? "enabled" : "unsupported by driver");
> 	}
>+
>+	set_features_cap(cxl_mbox, ro_cmds, wr_cmds);
> }
>
> static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct
>cxl_memdev_state *mds) diff --git a/drivers/cxl/cxlmem.h
>b/drivers/cxl/cxlmem.h index a0a49809cd76..55c55685cb39 100644
>--- a/drivers/cxl/cxlmem.h
>+++ b/drivers/cxl/cxlmem.h
>@@ -490,6 +490,9 @@ enum cxl_opcode {
> 	CXL_MBOX_OP_GET_LOG_CAPS	= 0x0402,
> 	CXL_MBOX_OP_CLEAR_LOG           = 0x0403,
> 	CXL_MBOX_OP_GET_SUP_LOG_SUBLIST = 0x0405,
>+	CXL_MBOX_OP_GET_SUPPORTED_FEATURES	= 0x0500,
>+	CXL_MBOX_OP_GET_FEATURE		= 0x0501,
>+	CXL_MBOX_OP_SET_FEATURE		= 0x0502,
> 	CXL_MBOX_OP_IDENTIFY		= 0x4000,
> 	CXL_MBOX_OP_GET_PARTITION_INFO	= 0x4100,
> 	CXL_MBOX_OP_SET_PARTITION_INFO	= 0x4101,
>diff --git a/include/cxl/features.h b/include/cxl/features.h new file mode 100644
>index 000000000000..357d3acf8429
>--- /dev/null
>+++ b/include/cxl/features.h
>@@ -0,0 +1,13 @@
>+/* SPDX-License-Identifier: GPL-2.0-only */
>+/* Copyright(c) 2024-2025 Intel Corporation. */ #ifndef
>+__CXL_FEATURES_H__ #define __CXL_FEATURES_H__
>+
>+/* Feature commands capability supported by a device */ enum
>+cxl_features_capability {
>+	CXL_FEATURES_NONE = 0,
>+	CXL_FEATURES_RO,
>+	CXL_FEATURES_RW,
>+};
>+
>+#endif
>diff --git a/include/cxl/mailbox.h b/include/cxl/mailbox.h index
>cc894f07a435..c4e99e2e3a9d 100644
>--- a/include/cxl/mailbox.h
>+++ b/include/cxl/mailbox.h
>@@ -3,6 +3,7 @@
> #ifndef __CXL_MBOX_H__
> #define __CXL_MBOX_H__
> #include <linux/rcuwait.h>
>+#include <cxl/features.h>
> #include <uapi/linux/cxl_mem.h>
>
> /**
>@@ -51,6 +52,7 @@ struct cxl_mbox_cmd {
>  * @mbox_mutex: mutex protects device mailbox and firmware
>  * @mbox_wait: rcuwait for mailbox
>  * @mbox_send: @dev specific transport for transmitting mailbox commands
>+ * @feat_cap: Features capability
>  */
> struct cxl_mailbox {
> 	struct device *host;
>@@ -60,6 +62,7 @@ struct cxl_mailbox {
> 	struct mutex mbox_mutex; /* lock to protect mailbox context */
> 	struct rcuwait mbox_wait;
> 	int (*mbox_send)(struct cxl_mailbox *cxl_mbox, struct cxl_mbox_cmd
>*cmd);
>+	enum cxl_features_capability feat_cap;
> };
>
> int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host);
>--
>2.48.1


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

* RE: [PATCH v6 02/14] cxl: Add Get Supported Features command for kernel usage
  2025-02-18 22:54 ` [PATCH v6 02/14] cxl: Add Get Supported Features command for kernel usage Dave Jiang
@ 2025-02-20 15:37   ` Shiju Jose
  0 siblings, 0 replies; 29+ messages in thread
From: Shiju Jose @ 2025-02-20 15:37 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl@vger.kernel.org
  Cc: dan.j.williams@intel.com, ira.weiny@intel.com,
	vishal.l.verma@intel.com, alison.schofield@intel.com,
	Jonathan Cameron, dave@stgolabs.net, jgg@nvidia.com,
	saeed@kernel.org, Li Ming

>-----Original Message-----
>From: Dave Jiang <dave.jiang@intel.com>
>Sent: 18 February 2025 22:55
>To: linux-cxl@vger.kernel.org
>Cc: dan.j.williams@intel.com; ira.weiny@intel.com; vishal.l.verma@intel.com;
>alison.schofield@intel.com; Jonathan Cameron
><jonathan.cameron@huawei.com>; dave@stgolabs.net; jgg@nvidia.com; Shiju
>Jose <shiju.jose@huawei.com>; saeed@kernel.org; Li Ming
><ming.li@zohomail.com>
>Subject: [PATCH v6 02/14] cxl: Add Get Supported Features command for kernel
>usage
>
>CXL spec r3.2 8.2.9.6.1 Get Supported Features (Opcode 0500h) The command
>retrieve the list of supported device-specific features (identified by UUID) and
>general information about each Feature.
>
>The driver will retrieve the Feature entries in order to make checks and provide
>information for the Get Feature and Set Feature command. One of the main
>piece of information retrieved are the effects a Set Feature command would
>have for a particular feature. The retrieved Feature entries are stored in the
>cxl_mailbox context.
>
>The setup of Features is initiated via devm_cxl_setup_features() during the pci
>probe function before the cxl_memdev is enumerated.
>
>Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>Reviewed-by: Li Ming <ming.li@zohomail.com>
>Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Tested-by: Shiju Jose <shiju.jose@huawei.com>

>---
>v6:
>- Remove @cap comment from 'cxl_features_state' (Shiju)
>---
> drivers/cxl/Kconfig          |  11 +++
> drivers/cxl/core/Makefile    |   1 +
> drivers/cxl/core/features.c  | 175 +++++++++++++++++++++++++++++++++++
> drivers/cxl/cxlmem.h         |   4 +
> drivers/cxl/pci.c            |   4 +
> include/cxl/features.h       |  87 +++++++++++++++++
> tools/testing/cxl/Kbuild     |   1 +
> tools/testing/cxl/test/mem.c |   4 +
> 8 files changed, 287 insertions(+)
> create mode 100644 drivers/cxl/core/features.c
>
>diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig index
>876469e23f7a..ad2e796e4ac6 100644
>--- a/drivers/cxl/Kconfig
>+++ b/drivers/cxl/Kconfig
>@@ -102,6 +102,17 @@ config CXL_MEM
>
> 	  If unsure say 'm'.
>
>+config CXL_FEATURES
>+	bool "CXL: Features"
>+	depends on CXL_PCI
>+	help
>+	  Enable support for CXL Features. A CXL device that includes a mailbox
>+	  supports commands that allows listing, getting, and setting of
>+	  optionally defined features such as memory sparing or post package
>+	  sparing. Vendors may define custom features for the device.
>+
>+	  If unsure say 'n'
>+
> config CXL_PORT
> 	default CXL_BUS
> 	tristate
>diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile index
>9259bcc6773c..b0bfbd9eac9b 100644
>--- a/drivers/cxl/core/Makefile
>+++ b/drivers/cxl/core/Makefile
>@@ -16,3 +16,4 @@ cxl_core-y += pmu.o
> cxl_core-y += cdat.o
> cxl_core-$(CONFIG_TRACING) += trace.o
> cxl_core-$(CONFIG_CXL_REGION) += region.o
>+cxl_core-$(CONFIG_CXL_FEATURES) += features.o
>diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c new file
>mode 100644 index 000000000000..f2898a56bd63
>--- /dev/null
>+++ b/drivers/cxl/core/features.c
>@@ -0,0 +1,175 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/* Copyright(c) 2024-2025 Intel Corporation. All rights reserved. */
>+#include <linux/device.h> #include <cxl/mailbox.h> #include
>+<cxl/features.h> #include "cxl.h"
>+#include "cxlmem.h"
>+
>+inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds)
>+{
>+	return cxlds->cxlfs;
>+}
>+EXPORT_SYMBOL_NS_GPL(to_cxlfs, "CXL");
>+
>+static int cxl_get_supported_features_count(struct cxl_mailbox
>+*cxl_mbox) {
>+	struct cxl_mbox_get_sup_feats_out mbox_out;
>+	struct cxl_mbox_get_sup_feats_in mbox_in;
>+	struct cxl_mbox_cmd mbox_cmd;
>+	int rc;
>+
>+	memset(&mbox_in, 0, sizeof(mbox_in));
>+	mbox_in.count = cpu_to_le32(sizeof(mbox_out));
>+	memset(&mbox_out, 0, sizeof(mbox_out));
>+	mbox_cmd = (struct cxl_mbox_cmd) {
>+		.opcode = CXL_MBOX_OP_GET_SUPPORTED_FEATURES,
>+		.size_in = sizeof(mbox_in),
>+		.payload_in = &mbox_in,
>+		.size_out = sizeof(mbox_out),
>+		.payload_out = &mbox_out,
>+		.min_out = sizeof(mbox_out),
>+	};
>+	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>+	if (rc < 0)
>+		return rc;
>+
>+	return le16_to_cpu(mbox_out.supported_feats);
>+}
>+
>+static struct cxl_feat_entries *
>+get_supported_features(struct cxl_features_state *cxlfs) {
>+	int remain_feats, max_size, max_feats, start, rc, hdr_size;
>+	struct cxl_mailbox *cxl_mbox = &cxlfs->cxlds->cxl_mbox;
>+	int feat_size = sizeof(struct cxl_feat_entry);
>+	struct cxl_mbox_get_sup_feats_in mbox_in;
>+	struct cxl_feat_entry *entry;
>+	struct cxl_mbox_cmd mbox_cmd;
>+	int count;
>+
>+	count = cxl_get_supported_features_count(cxl_mbox);
>+	if (count <= 0)
>+		return NULL;
>+
>+	struct cxl_feat_entries *entries __free(kvfree) =
>+		kvmalloc(struct_size(entries, ent, count), GFP_KERNEL);
>+	if (!entries)
>+		return NULL;
>+
>+	struct cxl_mbox_get_sup_feats_out *mbox_out __free(kvfree) =
>+		kvmalloc(cxl_mbox->payload_size, GFP_KERNEL);
>+	if (!mbox_out)
>+		return NULL;
>+
>+	hdr_size = struct_size(mbox_out, ents, 0);
>+	max_size = cxl_mbox->payload_size - hdr_size;
>+	/* max feat entries that can fit in mailbox max payload size */
>+	max_feats = max_size / feat_size;
>+	entry = entries->ent;
>+
>+	start = 0;
>+	remain_feats = count;
>+	do {
>+		int retrieved, alloc_size, copy_feats;
>+		int num_entries;
>+
>+		if (remain_feats > max_feats) {
>+			alloc_size = struct_size(mbox_out, ents, max_feats);
>+			remain_feats = remain_feats - max_feats;
>+			copy_feats = max_feats;
>+		} else {
>+			alloc_size = struct_size(mbox_out, ents, remain_feats);
>+			copy_feats = remain_feats;
>+			remain_feats = 0;
>+		}
>+
>+		memset(&mbox_in, 0, sizeof(mbox_in));
>+		mbox_in.count = cpu_to_le32(alloc_size);
>+		mbox_in.start_idx = cpu_to_le16(start);
>+		memset(mbox_out, 0, alloc_size);
>+		mbox_cmd = (struct cxl_mbox_cmd) {
>+			.opcode = CXL_MBOX_OP_GET_SUPPORTED_FEATURES,
>+			.size_in = sizeof(mbox_in),
>+			.payload_in = &mbox_in,
>+			.size_out = alloc_size,
>+			.payload_out = mbox_out,
>+			.min_out = hdr_size,
>+		};
>+		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>+		if (rc < 0)
>+			return NULL;
>+
>+		if (mbox_cmd.size_out <= hdr_size)
>+			return NULL;
>+
>+		/*
>+		 * Make sure retrieved out buffer is multiple of feature
>+		 * entries.
>+		 */
>+		retrieved = mbox_cmd.size_out - hdr_size;
>+		if (retrieved % feat_size)
>+			return NULL;
>+
>+		num_entries = le16_to_cpu(mbox_out->num_entries);
>+		/*
>+		 * If the reported output entries * defined entry size !=
>+		 * retrieved output bytes, then the output package is incorrect.
>+		 */
>+		if (num_entries * feat_size != retrieved)
>+			return NULL;
>+
>+		memcpy(entry, mbox_out->ents, retrieved);
>+		entry += num_entries;
>+		/*
>+		 * If the number of output entries is less than expected, add the
>+		 * remaining entries to the next batch.
>+		 */
>+		remain_feats += copy_feats - num_entries;
>+		start += num_entries;
>+	} while (remain_feats);
>+
>+	entries->num_features = count;
>+
>+	return no_free_ptr(entries);
>+}
>+
>+static void free_cxlfs(void *_cxlfs)
>+{
>+	struct cxl_features_state *cxlfs = _cxlfs;
>+	struct cxl_dev_state *cxlds = cxlfs->cxlds;
>+
>+	cxlds->cxlfs = NULL;
>+	kvfree(cxlfs->entries);
>+	kfree(cxlfs);
>+}
>+
>+/**
>+ * devm_cxl_setup_features() - Allocate and initialize features context
>+ * @cxlds: CXL device context
>+ *
>+ * Return 0 on success or -errno on failure.
>+ */
>+int devm_cxl_setup_features(struct cxl_dev_state *cxlds) {
>+	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
>+
>+	if (cxl_mbox->feat_cap < CXL_FEATURES_RO)
>+		return -ENODEV;
>+
>+	struct cxl_features_state *cxlfs __free(kfree) =
>+		kzalloc(sizeof(*cxlfs), GFP_KERNEL);
>+	if (!cxlfs)
>+		return -ENOMEM;
>+
>+	cxlfs->cxlds = cxlds;
>+
>+	cxlfs->entries = get_supported_features(cxlfs);
>+	if (!cxlfs->entries)
>+		return -ENOMEM;
>+
>+	cxlds->cxlfs = cxlfs;
>+
>+	return devm_add_action_or_reset(cxlds->dev, free_cxlfs,
>+no_free_ptr(cxlfs)); } EXPORT_SYMBOL_NS_GPL(devm_cxl_setup_features,
>+"CXL");
>diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index
>55c55685cb39..0dc2682eb379 100644
>--- a/drivers/cxl/cxlmem.h
>+++ b/drivers/cxl/cxlmem.h
>@@ -392,6 +392,7 @@ struct cxl_dpa_perf {
>  * @serial: PCIe Device Serial Number
>  * @type: Generic Memory Class device or Vendor Specific Memory device
>  * @cxl_mbox: CXL mailbox context
>+ * @cxl_features: CXL features context
>  */
> struct cxl_dev_state {
> 	struct device *dev;
>@@ -407,6 +408,9 @@ struct cxl_dev_state {
> 	u64 serial;
> 	enum cxl_devtype type;
> 	struct cxl_mailbox cxl_mbox;
>+#ifdef CONFIG_CXL_FEATURES
>+	struct cxl_features_state *cxlfs;
>+#endif
> };
>
> static inline struct cxl_dev_state *mbox_to_cxlds(struct cxl_mailbox *cxl_mbox)
>diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index
>a96e54c6259e..3e666ec51580 100644
>--- a/drivers/cxl/pci.c
>+++ b/drivers/cxl/pci.c
>@@ -997,6 +997,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const
>struct pci_device_id *id)
> 	if (rc)
> 		return rc;
>
>+	rc = devm_cxl_setup_features(cxlds);
>+	if (rc)
>+		dev_dbg(&pdev->dev, "No CXL Features discovered\n");
>+
> 	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
> 	if (IS_ERR(cxlmd))
> 		return PTR_ERR(cxlmd);
>diff --git a/include/cxl/features.h b/include/cxl/features.h index
>357d3acf8429..5b3485eb0f77 100644
>--- a/include/cxl/features.h
>+++ b/include/cxl/features.h
>@@ -3,6 +3,8 @@
> #ifndef __CXL_FEATURES_H__
> #define __CXL_FEATURES_H__
>
>+#include <linux/uuid.h>
>+
> /* Feature commands capability supported by a device */  enum
>cxl_features_capability {
> 	CXL_FEATURES_NONE = 0,
>@@ -10,4 +12,89 @@ enum cxl_features_capability {
> 	CXL_FEATURES_RW,
> };
>
>+/* Get Supported Features (0x500h) CXL r3.2 8.2.9.6.1 */ struct
>+cxl_mbox_get_sup_feats_in {
>+	__le32 count;
>+	__le16 start_idx;
>+	u8 reserved[2];
>+} __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)
>+
>+/*
>+ * CXL spec r3.2 Table 8-109
>+ * Get Supported Features Supported Feature Entry  */ struct
>+cxl_feat_entry {
>+	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];
>+} __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)
>+
>+/*
>+ * 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); } __packed;
>+
>+/**
>+ * struct cxl_features_state - The Features state for the device
>+ * @cxlds: Pointer to CXL device state
>+ * @entries: CXl feature entry context
>+ *	@num_features: total Features supported by the device
>+ *	@ent: Flex array of Feature detail entries from the device
>+ */
>+struct cxl_features_state {
>+	struct cxl_dev_state *cxlds;
>+	struct cxl_feat_entries {
>+		int num_features;
>+		struct cxl_feat_entry ent[] __counted_by(num_features);
>+	} *entries;
>+};
>+
>+#ifdef CONFIG_CXL_FEATURES
>+inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state
>+*cxlds); int devm_cxl_setup_features(struct cxl_dev_state *cxlds);
>+#else static inline struct cxl_features_state *to_cxlfs(struct
>+cxl_dev_state *cxlds) {
>+	return NULL;
>+}
>+
>+static inline int devm_cxl_setup_features(struct cxl_dev_state *cxlds)
>+{
>+	return -EOPNOTSUPP;
>+}
>+#endif
>+
> #endif
>diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild index
>b1256fee3567..0a6572ab6f37 100644
>--- a/tools/testing/cxl/Kbuild
>+++ b/tools/testing/cxl/Kbuild
>@@ -63,6 +63,7 @@ cxl_core-y += $(CXL_CORE_SRC)/pmu.o  cxl_core-y +=
>$(CXL_CORE_SRC)/cdat.o
> cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
> cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
>+cxl_core-$(CONFIG_CXL_FEATURES) += $(CXL_CORE_SRC)/features.o
> cxl_core-y += config_check.o
> cxl_core-y += cxl_core_test.o
> cxl_core-y += cxl_core_exports.o
>diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c index
>8d731bd63988..e9494cd446ef 100644
>--- a/tools/testing/cxl/test/mem.c
>+++ b/tools/testing/cxl/test/mem.c
>@@ -1558,6 +1558,10 @@ static int cxl_mock_mem_probe(struct
>platform_device *pdev)
> 	if (rc)
> 		return rc;
>
>+	rc = devm_cxl_setup_features(cxlds);
>+	if (rc)
>+		dev_dbg(dev, "No CXL Features discovered\n");
>+
> 	cxl_mock_add_event_logs(&mdata->mes);
>
> 	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlds);
>--
>2.48.1


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

* RE: [PATCH v6 06/14] cxl: Setup exclusive CXL features that are reserved for the kernel
  2025-02-18 22:54 ` [PATCH v6 06/14] cxl: Setup exclusive CXL features that are reserved for the kernel Dave Jiang
@ 2025-02-20 15:37   ` Shiju Jose
  0 siblings, 0 replies; 29+ messages in thread
From: Shiju Jose @ 2025-02-20 15:37 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl@vger.kernel.org
  Cc: dan.j.williams@intel.com, ira.weiny@intel.com,
	vishal.l.verma@intel.com, alison.schofield@intel.com,
	Jonathan Cameron, dave@stgolabs.net, jgg@nvidia.com,
	saeed@kernel.org, Li Ming

>-----Original Message-----
>From: Dave Jiang <dave.jiang@intel.com>
>Sent: 18 February 2025 22:55
>To: linux-cxl@vger.kernel.org
>Cc: dan.j.williams@intel.com; ira.weiny@intel.com; vishal.l.verma@intel.com;
>alison.schofield@intel.com; Jonathan Cameron
><jonathan.cameron@huawei.com>; dave@stgolabs.net; jgg@nvidia.com; Shiju
>Jose <shiju.jose@huawei.com>; saeed@kernel.org; Li Ming
><ming.li@zohomail.com>
>Subject: [PATCH v6 06/14] cxl: Setup exclusive CXL features that are reserved for
>the kernel
>
>Certain features will be exclusively used by components such as in kernel RAS
>driver. Setup an exclusion list that can be used to detect if a feature is exclusive
>to the kernel.
>
>Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>Reviewed-by: Li Ming <ming.li@zohomail.com>
>Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Tested-by: Shiju Jose <shiju.jose@huawei.com>

>---
> drivers/cxl/core/features.c | 28 ++++++++++++++++++++++++++++
> include/cxl/features.h      | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+)
>
>diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c index
>17a82734a340..048ba4fc3538 100644
>--- a/drivers/cxl/core/features.c
>+++ b/drivers/cxl/core/features.c
>@@ -7,6 +7,28 @@
> #include "core.h"
> #include "cxlmem.h"
>
>+/* All the features below are exclusive to the kernel */ static const
>+uuid_t cxl_exclusive_feats[] = {
>+	CXL_FEAT_PATROL_SCRUB_UUID,
>+	CXL_FEAT_ECS_UUID,
>+	CXL_FEAT_SPPR_UUID,
>+	CXL_FEAT_HPPR_UUID,
>+	CXL_FEAT_CACHELINE_SPARING_UUID,
>+	CXL_FEAT_ROW_SPARING_UUID,
>+	CXL_FEAT_BANK_SPARING_UUID,
>+	CXL_FEAT_RANK_SPARING_UUID,
>+};
>+
>+static bool is_cxl_feature_exclusive(struct cxl_feat_entry *entry) {
>+	for (int i = 0; i < ARRAY_SIZE(cxl_exclusive_feats); i++) {
>+		if (uuid_equal(&entry->uuid, &cxl_exclusive_feats[i]))
>+			return true;
>+	}
>+
>+	return false;
>+}
>+
> inline struct cxl_features_state *to_cxlfs(struct cxl_dev_state *cxlds)  {
> 	return cxlds->cxlfs;
>@@ -47,6 +69,7 @@ get_supported_features(struct cxl_features_state *cxlfs)
> 	struct cxl_mbox_get_sup_feats_in mbox_in;
> 	struct cxl_feat_entry *entry;
> 	struct cxl_mbox_cmd mbox_cmd;
>+	int user_feats = 0;
> 	int count;
>
> 	count = cxl_get_supported_features_count(cxl_mbox);
>@@ -121,6 +144,10 @@ get_supported_features(struct cxl_features_state
>*cxlfs)
> 			return NULL;
>
> 		memcpy(entry, mbox_out->ents, retrieved);
>+		for (int i = 0; i < num_entries; i++) {
>+			if (!is_cxl_feature_exclusive(entry + i))
>+				user_feats++;
>+		}
> 		entry += num_entries;
> 		/*
> 		 * If the number of output entries is less than expected, add the
>@@ -131,6 +158,7 @@ get_supported_features(struct cxl_features_state *cxlfs)
> 	} while (remain_feats);
>
> 	entries->num_features = count;
>+	entries->num_user_features = user_feats;
>
> 	return no_free_ptr(entries);
> }
>diff --git a/include/cxl/features.h b/include/cxl/features.h index
>ae56ef7ec9ab..d2cde46b0fec 100644
>--- a/include/cxl/features.h
>+++ b/include/cxl/features.h
>@@ -5,6 +5,39 @@
>
> #include <linux/uuid.h>
>
>+/* Feature UUIDs used by the kernel */
>+#define CXL_FEAT_PATROL_SCRUB_UUID
>		\
>+	UUID_INIT(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33, 0x75, 0x77, 0x4e,
>	\
>+		  0x06, 0xdb, 0x8a)
>+
>+#define CXL_FEAT_ECS_UUID
>	\
>+	UUID_INIT(0xe5b13f22, 0x2328, 0x4a14, 0xb8, 0xba, 0xb9, 0x69, 0x1e,
>	\
>+		  0x89, 0x33, 0x86)
>+
>+#define CXL_FEAT_SPPR_UUID
>	\
>+	UUID_INIT(0x892ba475, 0xfad8, 0x474e, 0x9d, 0x3e, 0x69, 0x2c, 0x91,
>	\
>+		  0x75, 0x68, 0xbb)
>+
>+#define CXL_FEAT_HPPR_UUID
>	\
>+	UUID_INIT(0x80ea4521, 0x786f, 0x4127, 0xaf, 0xb1, 0xec, 0x74, 0x59,
>	\
>+		  0xfb, 0x0e, 0x24)
>+
>+#define CXL_FEAT_CACHELINE_SPARING_UUID
>		\
>+	UUID_INIT(0x96C33386, 0x91dd, 0x44c7, 0x9e, 0xcb, 0xfd, 0xaf, 0x65,
>	\
>+		  0x03, 0xba, 0xc4)
>+
>+#define CXL_FEAT_ROW_SPARING_UUID
>		\
>+	UUID_INIT(0x450ebf67, 0xb135, 0x4f97, 0xa4, 0x98, 0xc2, 0xd5, 0x7f,
>	\
>+		  0x27, 0x9b, 0xed)
>+
>+#define CXL_FEAT_BANK_SPARING_UUID
>		\
>+	UUID_INIT(0x78b79636, 0x90ac, 0x4b64, 0xa4, 0xef, 0xfa, 0xac, 0x5d,
>	\
>+		  0x18, 0xa8, 0x63)
>+
>+#define CXL_FEAT_RANK_SPARING_UUID
>		\
>+	UUID_INIT(0x34dbaff5, 0x0552, 0x4281, 0x8f, 0x76, 0xda, 0x0b, 0x5e,
>	\
>+		  0x7a, 0x76, 0xa7)
>+
> /* Feature commands capability supported by a device */  enum
>cxl_features_capability {
> 	CXL_FEATURES_NONE = 0,
>@@ -134,6 +167,7 @@ struct cxl_features_state {
> 	struct cxl_dev_state *cxlds;
> 	struct cxl_feat_entries {
> 		int num_features;
>+		int num_user_features;
> 		struct cxl_feat_entry ent[] __counted_by(num_features);
> 	} *entries;
> };
>--
>2.48.1


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

end of thread, other threads:[~2025-02-20 15:37 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-18 22:54 [PATCH v6 00/14] cxl: Add CXL feature commands support via fwctl Dave Jiang
2025-02-18 22:54 ` [PATCH v6 01/14] cxl: Enumerate feature commands Dave Jiang
2025-02-20 15:37   ` Shiju Jose
2025-02-18 22:54 ` [PATCH v6 02/14] cxl: Add Get Supported Features command for kernel usage Dave Jiang
2025-02-20 15:37   ` Shiju Jose
2025-02-18 22:54 ` [PATCH v6 03/14] cxl/test: Add Get Supported Features mailbox command support Dave Jiang
2025-02-18 22:54 ` [PATCH v6 04/14] cxl/mbox: Add GET_FEATURE mailbox command Dave Jiang
2025-02-18 22:54 ` [PATCH v6 05/14] cxl/mbox: Add SET_FEATURE " Dave Jiang
2025-02-18 22:54 ` [PATCH v6 06/14] cxl: Setup exclusive CXL features that are reserved for the kernel Dave Jiang
2025-02-20 15:37   ` Shiju Jose
2025-02-18 22:54 ` [PATCH v6 07/14] cxl: Add FWCTL support to CXL Dave Jiang
2025-02-19  7:25   ` Li Ming
2025-02-19 17:09   ` Jonathan Cameron
2025-02-18 22:54 ` [PATCH v6 08/14] cxl: Move cxl feature command structs to user header Dave Jiang
2025-02-19  7:26   ` Li Ming
2025-02-18 22:54 ` [PATCH v6 09/14] cxl: Add support for fwctl RPC command to enable CXL feature commands Dave Jiang
2025-02-19  7:27   ` Li Ming
2025-02-19 17:53   ` Jonathan Cameron
2025-02-19 17:56     ` Jason Gunthorpe
2025-02-19 18:00       ` Dave Jiang
2025-02-19 18:29     ` Dave Jiang
2025-02-18 22:54 ` [PATCH v6 10/14] cxl: Add support to handle user feature commands for get feature Dave Jiang
2025-02-19 17:57   ` Jonathan Cameron
2025-02-18 22:54 ` [PATCH v6 11/14] cxl: Add support to handle user feature commands for set feature Dave Jiang
2025-02-19 18:12   ` Jonathan Cameron
2025-02-19 18:31     ` Dave Jiang
2025-02-18 22:54 ` [PATCH v6 12/14] cxl/test: Add Get Feature support to cxl_test Dave Jiang
2025-02-18 22:54 ` [PATCH v6 13/14] cxl/test: Add Set " Dave Jiang
2025-02-18 22:54 ` [PATCH v6 14/14] fwctl/cxl: Add documentation to FWCTL CXL Dave Jiang

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