[PATCH v12 07/17] cxl: Add Get Supported Features command for kernel usage

shiju.jose@huawei.com posted 17 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v12 07/17] cxl: Add Get Supported Features command for kernel usage
Posted by shiju.jose@huawei.com 2 months, 2 weeks ago
From: Dave Jiang <dave.jiang@intel.com>

CXL spec r3.1 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.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/cxl/core/mbox.c      | 175 +++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlmem.h         |  51 ++++++++++
 drivers/cxl/pci.c            |   4 +
 include/uapi/linux/cxl_mem.h |   1 +
 4 files changed, 231 insertions(+)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index fa1ee495a4e3..fe965ec5802f 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -67,6 +67,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
 	CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0),
 	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
 	CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0),
+	CXL_CMD(GET_SUPPORTED_FEATURES, 0x8, CXL_VARIABLE_PAYLOAD, 0),
 };
 
 /*
@@ -790,6 +791,180 @@ static const uuid_t log_uuid[] = {
 	[VENDOR_DEBUG_UUID] = DEFINE_CXL_VENDOR_DEBUG_UUID,
 };
 
+static void cxl_free_features(void *features)
+{
+	kvfree(features);
+}
+
+static int cxl_get_supported_features_count(struct cxl_dev_state *cxlds)
+{
+	struct cxl_mailbox *cxl_mbox = &cxlds->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 = 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;
+
+	cxlds->num_features = le16_to_cpu(mbox_out.supported_feats);
+	if (!cxlds->num_features)
+		return -ENOENT;
+
+	return 0;
+}
+
+int cxl_get_supported_features(struct cxl_dev_state *cxlds)
+{
+	int remain_feats, max_size, max_feats, start, rc;
+	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
+	int feat_size = sizeof(struct cxl_feat_entry);
+	struct cxl_mbox_get_sup_feats_out *mbox_out;
+	struct cxl_mbox_get_sup_feats_in mbox_in;
+	int hdr_size = sizeof(*mbox_out);
+	struct cxl_mbox_cmd mbox_cmd;
+	struct cxl_mem_command *cmd;
+	void *ptr;
+
+	/* Get supported features is optional, need to check */
+	cmd = cxl_mem_find_command(CXL_MBOX_OP_GET_SUPPORTED_FEATURES);
+	if (!cmd)
+		return -EOPNOTSUPP;
+	if (!test_bit(cmd->info.id, cxl_mbox->enabled_cmds))
+		return -EOPNOTSUPP;
+
+	rc = cxl_get_supported_features_count(cxlds);
+	if (rc)
+		return rc;
+
+	struct cxl_feat_entry *entries __free(kvfree) =
+		kvmalloc(cxlds->num_features * feat_size, GFP_KERNEL);
+
+	if (!entries)
+		return -ENOMEM;
+
+	cxlds->entries = no_free_ptr(entries);
+	rc = devm_add_action_or_reset(cxl_mbox->host, cxl_free_features,
+				      cxlds->entries);
+	if (rc)
+		return rc;
+
+	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;
+	ptr = &cxlds->entries[0];
+
+	mbox_out = kvmalloc(cxl_mbox->payload_size, GFP_KERNEL);
+	if (!mbox_out)
+		return -ENOMEM;
+
+	start = 0;
+	remain_feats = cxlds->num_features;
+	do {
+		int retrieved, alloc_size, copy_feats;
+
+		if (remain_feats > max_feats) {
+			alloc_size = sizeof(*mbox_out) + max_feats * feat_size;
+			remain_feats = remain_feats - max_feats;
+			copy_feats = max_feats;
+		} else {
+			alloc_size = sizeof(*mbox_out) + remain_feats * feat_size;
+			copy_feats = remain_feats;
+			remain_feats = 0;
+		}
+
+		memset(&mbox_in, 0, sizeof(mbox_in));
+		mbox_in.count = alloc_size;
+		mbox_in.start_idx = 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) {
+			kfree(mbox_out);
+			return rc;
+		}
+		if (mbox_cmd.size_out <= hdr_size) {
+			rc = -ENXIO;
+			goto err;
+		}
+
+		/*
+		 * Make sure retrieved out buffer is multiple of feature
+		 * entries.
+		 */
+		retrieved = mbox_cmd.size_out - hdr_size;
+		if (retrieved % feat_size) {
+			rc = -ENXIO;
+			goto err;
+		}
+
+		/*
+		 * If the reported output entries * defined entry size !=
+		 * retrieved output bytes, then the output package is incorrect.
+		 */
+		if (mbox_out->num_entries * feat_size != retrieved) {
+			rc = -ENXIO;
+			goto err;
+		}
+
+		memcpy(ptr, mbox_out->ents, retrieved);
+		ptr += retrieved;
+		/*
+		 * If the number of output entries is less than expected, add the
+		 * remaining entries to the next batch.
+		 */
+		remain_feats += copy_feats - mbox_out->num_entries;
+		start += mbox_out->num_entries;
+	} while (remain_feats);
+
+	kfree(mbox_out);
+	return 0;
+
+err:
+	kfree(mbox_out);
+	cxlds->num_features = 0;
+	return rc;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_get_supported_features, CXL);
+
+int cxl_get_supported_feature_entry(struct cxl_dev_state *cxlds, const uuid_t *feat_uuid,
+				    struct cxl_feat_entry *feat_entry_out)
+{
+	struct cxl_feat_entry *feat_entry;
+	int count;
+
+	/* Check CXL dev supports the feature */
+	feat_entry = &cxlds->entries[0];
+	for (count = 0; count < cxlds->num_features; count++, feat_entry++) {
+		if (uuid_equal(&feat_entry->uuid, feat_uuid)) {
+			memcpy(feat_entry_out, feat_entry, sizeof(*feat_entry_out));
+			return 0;
+		}
+	}
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_get_supported_feature_entry, CXL);
+
 /**
  * cxl_enumerate_cmds() - Enumerate commands for a device.
  * @mds: The driver data for the operation
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index d7c6ffe2a884..5d149e64c247 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -389,6 +389,8 @@ struct cxl_dpa_perf {
  * @ram_res: Active Volatile memory capacity configuration
  * @serial: PCIe Device Serial Number
  * @type: Generic Memory Class device or Vendor Specific Memory device
+ * @features: number of supported features
+ * @entries: list of supported feature entries.
  */
 struct cxl_dev_state {
 	struct device *dev;
@@ -404,6 +406,8 @@ struct cxl_dev_state {
 	u64 serial;
 	enum cxl_devtype type;
 	struct cxl_mailbox cxl_mbox;
+	int num_features;
+	struct cxl_feat_entry *entries;
 };
 
 /**
@@ -482,6 +486,7 @@ 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_IDENTIFY		= 0x4000,
 	CXL_MBOX_OP_GET_PARTITION_INFO	= 0x4100,
 	CXL_MBOX_OP_SET_PARTITION_INFO	= 0x4101,
@@ -765,6 +770,48 @@ enum {
 	CXL_PMEM_SEC_PASS_USER,
 };
 
+/* Get Supported Features (0x500h) CXL r3.1 8.2.9.6.1 */
+struct cxl_mbox_get_sup_feats_in {
+	__le32 count;
+	__le16 start_idx;
+	u8 reserved[2];
+} __packed;
+
+/* Supported Feature Entry : Payload out attribute flags */
+#define CXL_FEAT_ENTRY_FLAG_CHANGABLE	BIT(0)
+#define CXL_FEAT_ENTRY_FLAG_DEEPEST_RESET_PERSISTENCE_MASK	GENMASK(3, 1)
+#define CXL_FEAT_ENTRY_FLAG_PERSIST_ACROSS_FIRMWARE_UPDATE	BIT(4)
+#define CXL_FEAT_ENTRY_FLAG_SUPPORT_DEFAULT_SELECTION	BIT(5)
+#define CXL_FEAT_ENTRY_FLAG_SUPPORT_SAVED_SELECTION	BIT(6)
+
+enum cxl_feat_attr_value_persistence {
+	CXL_FEAT_ATTR_VALUE_PERSISTENCE_NONE,
+	CXL_FEAT_ATTR_VALUE_PERSISTENCE_CXL_RESET,
+	CXL_FEAT_ATTR_VALUE_PERSISTENCE_HOT_RESET,
+	CXL_FEAT_ATTR_VALUE_PERSISTENCE_WARM_RESET,
+	CXL_FEAT_ATTR_VALUE_PERSISTENCE_COLD_RESET,
+	CXL_FEAT_ATTR_VALUE_PERSISTENCE_MAX
+};
+
+struct cxl_feat_entry {
+	uuid_t uuid;
+	__le16 id;
+	__le16 get_feat_size;
+	__le16 set_feat_size;
+	__le32 attr_flags;
+	u8 get_feat_ver;
+	u8 set_feat_ver;
+	__le16 set_effects;
+	u8 reserved[18];
+} __packed;
+
+struct cxl_mbox_get_sup_feats_out {
+	__le16 num_entries;
+	__le16 supported_feats;
+	u8 reserved[4];
+	struct cxl_feat_entry ents[] __counted_by(le32_to_cpu(supported_feats));
+} __packed;
+
 int cxl_internal_send_cmd(struct cxl_mailbox *cxl_mbox,
 			  struct cxl_mbox_cmd *cmd);
 int cxl_dev_state_identify(struct cxl_memdev_state *mds);
@@ -824,4 +871,8 @@ struct cxl_hdm {
 struct seq_file;
 struct dentry *cxl_debugfs_create_dir(const char *dir);
 void cxl_dpa_debug(struct seq_file *file, struct cxl_dev_state *cxlds);
+
+int cxl_get_supported_features(struct cxl_dev_state *cxlds);
+int cxl_get_supported_feature_entry(struct cxl_dev_state *cxlds, const uuid_t *feat_uuid,
+				    struct cxl_feat_entry *feat_entry_out);
 #endif /* __CXL_MEM_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 3c73de475bf3..cec88e3a1754 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -872,6 +872,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
+	rc = cxl_get_supported_features(cxlds);
+	if (rc)
+		dev_dbg(&pdev->dev, "No features enumerated.\n");
+
 	rc = cxl_set_timestamp(mds);
 	if (rc)
 		return rc;
diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
index c6c0fe27495d..bd2535962f70 100644
--- a/include/uapi/linux/cxl_mem.h
+++ b/include/uapi/linux/cxl_mem.h
@@ -50,6 +50,7 @@
 	___C(GET_LOG_CAPS, "Get Log Capabilities"),			  \
 	___C(CLEAR_LOG, "Clear Log"),					  \
 	___C(GET_SUP_LOG_SUBLIST, "Get Supported Logs Sub-List"),	  \
+	___C(GET_SUPPORTED_FEATURES, "Get Supported Features"),		  \
 	___C(MAX, "invalid / last command")
 
 #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
-- 
2.34.1
Re: [PATCH v12 07/17] cxl: Add Get Supported Features command for kernel usage
Posted by Dave Jiang 2 months, 1 week ago

On 9/11/24 2:04 AM, shiju.jose@huawei.com wrote:
> From: Dave Jiang <dave.jiang@intel.com>
> 
> CXL spec r3.1 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.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> ---
>  drivers/cxl/core/mbox.c      | 175 +++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxlmem.h         |  51 ++++++++++
>  drivers/cxl/pci.c            |   4 +
>  include/uapi/linux/cxl_mem.h |   1 +
>  4 files changed, 231 insertions(+)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index fa1ee495a4e3..fe965ec5802f 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -67,6 +67,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
>  	CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0),
>  	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
>  	CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0),
> +	CXL_CMD(GET_SUPPORTED_FEATURES, 0x8, CXL_VARIABLE_PAYLOAD, 0),
>  };
>  
>  /*
> @@ -790,6 +791,180 @@ static const uuid_t log_uuid[] = {
>  	[VENDOR_DEBUG_UUID] = DEFINE_CXL_VENDOR_DEBUG_UUID,
>  };
>  
> +static void cxl_free_features(void *features)
> +{
> +	kvfree(features);
> +}
> +
> +static int cxl_get_supported_features_count(struct cxl_dev_state *cxlds)
> +{
> +	struct cxl_mailbox *cxl_mbox = &cxlds->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 = 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;
> +
> +	cxlds->num_features = le16_to_cpu(mbox_out.supported_feats);
> +	if (!cxlds->num_features)
> +		return -ENOENT;
> +
> +	return 0;
> +}
> +
> +int cxl_get_supported_features(struct cxl_dev_state *cxlds)
> +{
> +	int remain_feats, max_size, max_feats, start, rc;
> +	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
> +	int feat_size = sizeof(struct cxl_feat_entry);
> +	struct cxl_mbox_get_sup_feats_out *mbox_out;
> +	struct cxl_mbox_get_sup_feats_in mbox_in;
> +	int hdr_size = sizeof(*mbox_out);
> +	struct cxl_mbox_cmd mbox_cmd;
> +	struct cxl_mem_command *cmd;
> +	void *ptr;
> +
> +	/* Get supported features is optional, need to check */
> +	cmd = cxl_mem_find_command(CXL_MBOX_OP_GET_SUPPORTED_FEATURES);
> +	if (!cmd)
> +		return -EOPNOTSUPP;
> +	if (!test_bit(cmd->info.id, cxl_mbox->enabled_cmds))
> +		return -EOPNOTSUPP;
> +
> +	rc = cxl_get_supported_features_count(cxlds);
> +	if (rc)
> +		return rc;
> +
> +	struct cxl_feat_entry *entries __free(kvfree) =
> +		kvmalloc(cxlds->num_features * feat_size, GFP_KERNEL);
> +
> +	if (!entries)
> +		return -ENOMEM;
> +
> +	cxlds->entries = no_free_ptr(entries);
> +	rc = devm_add_action_or_reset(cxl_mbox->host, cxl_free_features,
> +				      cxlds->entries);
> +	if (rc)
> +		return rc;
> +
> +	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;
> +	ptr = &cxlds->entries[0];
> +
> +	mbox_out = kvmalloc(cxl_mbox->payload_size, GFP_KERNEL);
> +	if (!mbox_out)
> +		return -ENOMEM;
> +
> +	start = 0;
> +	remain_feats = cxlds->num_features;
> +	do {
> +		int retrieved, alloc_size, copy_feats;
> +
> +		if (remain_feats > max_feats) {
> +			alloc_size = sizeof(*mbox_out) + max_feats * feat_size;
> +			remain_feats = remain_feats - max_feats;
> +			copy_feats = max_feats;
> +		} else {
> +			alloc_size = sizeof(*mbox_out) + remain_feats * feat_size;
> +			copy_feats = remain_feats;
> +			remain_feats = 0;
> +		}
> +
> +		memset(&mbox_in, 0, sizeof(mbox_in));
> +		mbox_in.count = alloc_size;
> +		mbox_in.start_idx = 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) {
> +			kfree(mbox_out);
> +			return rc;
> +		}
> +		if (mbox_cmd.size_out <= hdr_size) {
> +			rc = -ENXIO;
> +			goto err;
> +		}
> +
> +		/*
> +		 * Make sure retrieved out buffer is multiple of feature
> +		 * entries.
> +		 */
> +		retrieved = mbox_cmd.size_out - hdr_size;
> +		if (retrieved % feat_size) {
> +			rc = -ENXIO;
> +			goto err;
> +		}
> +
> +		/*
> +		 * If the reported output entries * defined entry size !=
> +		 * retrieved output bytes, then the output package is incorrect.
> +		 */
> +		if (mbox_out->num_entries * feat_size != retrieved) {
> +			rc = -ENXIO;
> +			goto err;
> +		}
> +
> +		memcpy(ptr, mbox_out->ents, retrieved);
> +		ptr += retrieved;
> +		/*
> +		 * If the number of output entries is less than expected, add the
> +		 * remaining entries to the next batch.
> +		 */
> +		remain_feats += copy_feats - mbox_out->num_entries;
> +		start += mbox_out->num_entries;
> +	} while (remain_feats);
> +
> +	kfree(mbox_out);
> +	return 0;
> +
> +err:
> +	kfree(mbox_out);
> +	cxlds->num_features = 0;
> +	return rc;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_get_supported_features, CXL);
> +
> +int cxl_get_supported_feature_entry(struct cxl_dev_state *cxlds, const uuid_t *feat_uuid,
> +				    struct cxl_feat_entry *feat_entry_out)
> +{
> +	struct cxl_feat_entry *feat_entry;
> +	int count;
> +
> +	/* Check CXL dev supports the feature */
> +	feat_entry = &cxlds->entries[0];
> +	for (count = 0; count < cxlds->num_features; count++, feat_entry++) {
> +		if (uuid_equal(&feat_entry->uuid, feat_uuid)) {
> +			memcpy(feat_entry_out, feat_entry, sizeof(*feat_entry_out));
> +			return 0;
> +		}
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_get_supported_feature_entry, CXL);
> +
>  /**
>   * cxl_enumerate_cmds() - Enumerate commands for a device.
>   * @mds: The driver data for the operation
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index d7c6ffe2a884..5d149e64c247 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -389,6 +389,8 @@ struct cxl_dpa_perf {
>   * @ram_res: Active Volatile memory capacity configuration
>   * @serial: PCIe Device Serial Number
>   * @type: Generic Memory Class device or Vendor Specific Memory device
> + * @features: number of supported features
> + * @entries: list of supported feature entries.
>   */
>  struct cxl_dev_state {
>  	struct device *dev;
> @@ -404,6 +406,8 @@ struct cxl_dev_state {
>  	u64 serial;
>  	enum cxl_devtype type;
>  	struct cxl_mailbox cxl_mbox;
> +	int num_features;
> +	struct cxl_feat_entry *entries;

Hi Shiju,
Going back and refactoring the fwctl code, I think this needs to stay with cxl_mailbox. Otherwise it makes it really hard to verify the feature effects for userspace. Preferably I don't think we want to expose 'struct cxl_dev_state' to fwctl if we don't need to.

DJ

>  };
>  
>  /**
> @@ -482,6 +486,7 @@ 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_IDENTIFY		= 0x4000,
>  	CXL_MBOX_OP_GET_PARTITION_INFO	= 0x4100,
>  	CXL_MBOX_OP_SET_PARTITION_INFO	= 0x4101,
> @@ -765,6 +770,48 @@ enum {
>  	CXL_PMEM_SEC_PASS_USER,
>  };
>  
> +/* Get Supported Features (0x500h) CXL r3.1 8.2.9.6.1 */
> +struct cxl_mbox_get_sup_feats_in {
> +	__le32 count;
> +	__le16 start_idx;
> +	u8 reserved[2];
> +} __packed;
> +
> +/* Supported Feature Entry : Payload out attribute flags */
> +#define CXL_FEAT_ENTRY_FLAG_CHANGABLE	BIT(0)
> +#define CXL_FEAT_ENTRY_FLAG_DEEPEST_RESET_PERSISTENCE_MASK	GENMASK(3, 1)
> +#define CXL_FEAT_ENTRY_FLAG_PERSIST_ACROSS_FIRMWARE_UPDATE	BIT(4)
> +#define CXL_FEAT_ENTRY_FLAG_SUPPORT_DEFAULT_SELECTION	BIT(5)
> +#define CXL_FEAT_ENTRY_FLAG_SUPPORT_SAVED_SELECTION	BIT(6)
> +
> +enum cxl_feat_attr_value_persistence {
> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_NONE,
> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_CXL_RESET,
> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_HOT_RESET,
> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_WARM_RESET,
> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_COLD_RESET,
> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_MAX
> +};
> +
> +struct cxl_feat_entry {
> +	uuid_t uuid;
> +	__le16 id;
> +	__le16 get_feat_size;
> +	__le16 set_feat_size;
> +	__le32 attr_flags;
> +	u8 get_feat_ver;
> +	u8 set_feat_ver;
> +	__le16 set_effects;
> +	u8 reserved[18];
> +} __packed;
> +
> +struct cxl_mbox_get_sup_feats_out {
> +	__le16 num_entries;
> +	__le16 supported_feats;
> +	u8 reserved[4];
> +	struct cxl_feat_entry ents[] __counted_by(le32_to_cpu(supported_feats));
> +} __packed;
> +
>  int cxl_internal_send_cmd(struct cxl_mailbox *cxl_mbox,
>  			  struct cxl_mbox_cmd *cmd);
>  int cxl_dev_state_identify(struct cxl_memdev_state *mds);
> @@ -824,4 +871,8 @@ struct cxl_hdm {
>  struct seq_file;
>  struct dentry *cxl_debugfs_create_dir(const char *dir);
>  void cxl_dpa_debug(struct seq_file *file, struct cxl_dev_state *cxlds);
> +
> +int cxl_get_supported_features(struct cxl_dev_state *cxlds);
> +int cxl_get_supported_feature_entry(struct cxl_dev_state *cxlds, const uuid_t *feat_uuid,
> +				    struct cxl_feat_entry *feat_entry_out);
>  #endif /* __CXL_MEM_H__ */
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 3c73de475bf3..cec88e3a1754 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -872,6 +872,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		return rc;
>  
> +	rc = cxl_get_supported_features(cxlds);
> +	if (rc)
> +		dev_dbg(&pdev->dev, "No features enumerated.\n");
> +
>  	rc = cxl_set_timestamp(mds);
>  	if (rc)
>  		return rc;
> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> index c6c0fe27495d..bd2535962f70 100644
> --- a/include/uapi/linux/cxl_mem.h
> +++ b/include/uapi/linux/cxl_mem.h
> @@ -50,6 +50,7 @@
>  	___C(GET_LOG_CAPS, "Get Log Capabilities"),			  \
>  	___C(CLEAR_LOG, "Clear Log"),					  \
>  	___C(GET_SUP_LOG_SUBLIST, "Get Supported Logs Sub-List"),	  \
> +	___C(GET_SUPPORTED_FEATURES, "Get Supported Features"),		  \
>  	___C(MAX, "invalid / last command")
>  
>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
RE: [PATCH v12 07/17] cxl: Add Get Supported Features command for kernel usage
Posted by Shiju Jose 2 months ago

>-----Original Message-----
>From: Dave Jiang <dave.jiang@intel.com>
>Sent: 24 September 2024 00:34
>To: Shiju Jose <shiju.jose@huawei.com>; linux-edac@vger.kernel.org; linux-
>cxl@vger.kernel.org; linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-
>kernel@vger.kernel.org
>Cc: bp@alien8.de; tony.luck@intel.com; rafael@kernel.org; lenb@kernel.org;
>mchehab@kernel.org; dan.j.williams@intel.com; dave@stgolabs.net; Jonathan
>Cameron <jonathan.cameron@huawei.com>; alison.schofield@intel.com;
>vishal.l.verma@intel.com; ira.weiny@intel.com; david@redhat.com;
>Vilas.Sridharan@amd.com; leo.duran@amd.com; Yazen.Ghannam@amd.com;
>rientjes@google.com; jiaqiyan@google.com; Jon.Grimm@amd.com;
>dave.hansen@linux.intel.com; naoya.horiguchi@nec.com;
>james.morse@arm.com; jthoughton@google.com; somasundaram.a@hpe.com;
>erdemaktas@google.com; pgonda@google.com; duenwen@google.com;
>mike.malvestuto@intel.com; gthelen@google.com;
>wschwartz@amperecomputing.com; dferguson@amperecomputing.com;
>wbs@os.amperecomputing.com; nifan.cxl@gmail.com; jgroves@micron.com;
>vsalve@micron.com; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>; Roberto Sassu <roberto.sassu@huawei.com>;
>kangkang.shen@futurewei.com; wanghuiqiang <wanghuiqiang@huawei.com>;
>Linuxarm <linuxarm@huawei.com>
>Subject: Re: [PATCH v12 07/17] cxl: Add Get Supported Features command for
>kernel usage
>
>
>
>On 9/11/24 2:04 AM, shiju.jose@huawei.com wrote:
>> From: Dave Jiang <dave.jiang@intel.com>
>>
>> CXL spec r3.1 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.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>> ---
>>  drivers/cxl/core/mbox.c      | 175 +++++++++++++++++++++++++++++++++++
>>  drivers/cxl/cxlmem.h         |  51 ++++++++++
>>  drivers/cxl/pci.c            |   4 +
>>  include/uapi/linux/cxl_mem.h |   1 +
>>  4 files changed, 231 insertions(+)
>>
>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index
>> fa1ee495a4e3..fe965ec5802f 100644
>> --- a/drivers/cxl/core/mbox.c
>> +++ b/drivers/cxl/core/mbox.c
>> @@ -67,6 +67,7 @@ static struct cxl_mem_command
>cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
>>  	CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0),
>>  	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
>>  	CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0),
>> +	CXL_CMD(GET_SUPPORTED_FEATURES, 0x8, CXL_VARIABLE_PAYLOAD,
>0),
>>  };
>>
>>  /*
>> @@ -790,6 +791,180 @@ static const uuid_t log_uuid[] = {
>>  	[VENDOR_DEBUG_UUID] = DEFINE_CXL_VENDOR_DEBUG_UUID,  };
>>
>> +static void cxl_free_features(void *features) {
>> +	kvfree(features);
>> +}
>> +
>> +static int cxl_get_supported_features_count(struct cxl_dev_state
>> +*cxlds) {
>> +	struct cxl_mailbox *cxl_mbox = &cxlds->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 = 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;
>> +
>> +	cxlds->num_features = le16_to_cpu(mbox_out.supported_feats);
>> +	if (!cxlds->num_features)
>> +		return -ENOENT;
>> +
>> +	return 0;
>> +}
>> +
>> +int cxl_get_supported_features(struct cxl_dev_state *cxlds) {
>> +	int remain_feats, max_size, max_feats, start, rc;
>> +	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
>> +	int feat_size = sizeof(struct cxl_feat_entry);
>> +	struct cxl_mbox_get_sup_feats_out *mbox_out;
>> +	struct cxl_mbox_get_sup_feats_in mbox_in;
>> +	int hdr_size = sizeof(*mbox_out);
>> +	struct cxl_mbox_cmd mbox_cmd;
>> +	struct cxl_mem_command *cmd;
>> +	void *ptr;
>> +
>> +	/* Get supported features is optional, need to check */
>> +	cmd =
>cxl_mem_find_command(CXL_MBOX_OP_GET_SUPPORTED_FEATURES);
>> +	if (!cmd)
>> +		return -EOPNOTSUPP;
>> +	if (!test_bit(cmd->info.id, cxl_mbox->enabled_cmds))
>> +		return -EOPNOTSUPP;
>> +
>> +	rc = cxl_get_supported_features_count(cxlds);
>> +	if (rc)
>> +		return rc;
>> +
>> +	struct cxl_feat_entry *entries __free(kvfree) =
>> +		kvmalloc(cxlds->num_features * feat_size, GFP_KERNEL);
>> +
>> +	if (!entries)
>> +		return -ENOMEM;
>> +
>> +	cxlds->entries = no_free_ptr(entries);
>> +	rc = devm_add_action_or_reset(cxl_mbox->host, cxl_free_features,
>> +				      cxlds->entries);
>> +	if (rc)
>> +		return rc;
>> +
>> +	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;
>> +	ptr = &cxlds->entries[0];
>> +
>> +	mbox_out = kvmalloc(cxl_mbox->payload_size, GFP_KERNEL);
>> +	if (!mbox_out)
>> +		return -ENOMEM;
>> +
>> +	start = 0;
>> +	remain_feats = cxlds->num_features;
>> +	do {
>> +		int retrieved, alloc_size, copy_feats;
>> +
>> +		if (remain_feats > max_feats) {
>> +			alloc_size = sizeof(*mbox_out) + max_feats * feat_size;
>> +			remain_feats = remain_feats - max_feats;
>> +			copy_feats = max_feats;
>> +		} else {
>> +			alloc_size = sizeof(*mbox_out) + remain_feats *
>feat_size;
>> +			copy_feats = remain_feats;
>> +			remain_feats = 0;
>> +		}
>> +
>> +		memset(&mbox_in, 0, sizeof(mbox_in));
>> +		mbox_in.count = alloc_size;
>> +		mbox_in.start_idx = 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) {
>> +			kfree(mbox_out);
>> +			return rc;
>> +		}
>> +		if (mbox_cmd.size_out <= hdr_size) {
>> +			rc = -ENXIO;
>> +			goto err;
>> +		}
>> +
>> +		/*
>> +		 * Make sure retrieved out buffer is multiple of feature
>> +		 * entries.
>> +		 */
>> +		retrieved = mbox_cmd.size_out - hdr_size;
>> +		if (retrieved % feat_size) {
>> +			rc = -ENXIO;
>> +			goto err;
>> +		}
>> +
>> +		/*
>> +		 * If the reported output entries * defined entry size !=
>> +		 * retrieved output bytes, then the output package is incorrect.
>> +		 */
>> +		if (mbox_out->num_entries * feat_size != retrieved) {
>> +			rc = -ENXIO;
>> +			goto err;
>> +		}
>> +
>> +		memcpy(ptr, mbox_out->ents, retrieved);
>> +		ptr += retrieved;
>> +		/*
>> +		 * If the number of output entries is less than expected, add the
>> +		 * remaining entries to the next batch.
>> +		 */
>> +		remain_feats += copy_feats - mbox_out->num_entries;
>> +		start += mbox_out->num_entries;
>> +	} while (remain_feats);
>> +
>> +	kfree(mbox_out);
>> +	return 0;
>> +
>> +err:
>> +	kfree(mbox_out);
>> +	cxlds->num_features = 0;
>> +	return rc;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_get_supported_features, CXL);
>> +
>> +int cxl_get_supported_feature_entry(struct cxl_dev_state *cxlds, const uuid_t
>*feat_uuid,
>> +				    struct cxl_feat_entry *feat_entry_out) {
>> +	struct cxl_feat_entry *feat_entry;
>> +	int count;
>> +
>> +	/* Check CXL dev supports the feature */
>> +	feat_entry = &cxlds->entries[0];
>> +	for (count = 0; count < cxlds->num_features; count++, feat_entry++) {
>> +		if (uuid_equal(&feat_entry->uuid, feat_uuid)) {
>> +			memcpy(feat_entry_out, feat_entry,
>sizeof(*feat_entry_out));
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	return -EOPNOTSUPP;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_get_supported_feature_entry, CXL);
>> +
>>  /**
>>   * cxl_enumerate_cmds() - Enumerate commands for a device.
>>   * @mds: The driver data for the operation diff --git
>> a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index
>> d7c6ffe2a884..5d149e64c247 100644
>> --- a/drivers/cxl/cxlmem.h
>> +++ b/drivers/cxl/cxlmem.h
>> @@ -389,6 +389,8 @@ struct cxl_dpa_perf {
>>   * @ram_res: Active Volatile memory capacity configuration
>>   * @serial: PCIe Device Serial Number
>>   * @type: Generic Memory Class device or Vendor Specific Memory
>> device
>> + * @features: number of supported features
>> + * @entries: list of supported feature entries.
>>   */
>>  struct cxl_dev_state {
>>  	struct device *dev;
>> @@ -404,6 +406,8 @@ struct cxl_dev_state {
>>  	u64 serial;
>>  	enum cxl_devtype type;
>>  	struct cxl_mailbox cxl_mbox;
>> +	int num_features;
>> +	struct cxl_feat_entry *entries;
>
>Hi Shiju,
>Going back and refactoring the fwctl code, I think this needs to stay with
>cxl_mailbox. Otherwise it makes it really hard to verify the feature effects for
>userspace. Preferably I don't think we want to expose 'struct cxl_dev_state' to
>fwctl if we don't need to.
>
>DJ

Hi Dave,
Ok. Revert to stay with struct cxl_mailbox and tested. Changes will be in the next version.

Thanks,
Shiju
>
>>  };
>>
>>  /**
>> @@ -482,6 +486,7 @@ 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_IDENTIFY		= 0x4000,
>>  	CXL_MBOX_OP_GET_PARTITION_INFO	= 0x4100,
>>  	CXL_MBOX_OP_SET_PARTITION_INFO	= 0x4101,
>> @@ -765,6 +770,48 @@ enum {
>>  	CXL_PMEM_SEC_PASS_USER,
>>  };
>>
>> +/* Get Supported Features (0x500h) CXL r3.1 8.2.9.6.1 */ struct
>> +cxl_mbox_get_sup_feats_in {
>> +	__le32 count;
>> +	__le16 start_idx;
>> +	u8 reserved[2];
>> +} __packed;
>> +
>> +/* Supported Feature Entry : Payload out attribute flags */
>> +#define CXL_FEAT_ENTRY_FLAG_CHANGABLE	BIT(0)
>> +#define CXL_FEAT_ENTRY_FLAG_DEEPEST_RESET_PERSISTENCE_MASK
>	GENMASK(3, 1)
>> +#define CXL_FEAT_ENTRY_FLAG_PERSIST_ACROSS_FIRMWARE_UPDATE
>	BIT(4)
>> +#define CXL_FEAT_ENTRY_FLAG_SUPPORT_DEFAULT_SELECTION	BIT(5)
>> +#define CXL_FEAT_ENTRY_FLAG_SUPPORT_SAVED_SELECTION	BIT(6)
>> +
>> +enum cxl_feat_attr_value_persistence {
>> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_NONE,
>> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_CXL_RESET,
>> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_HOT_RESET,
>> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_WARM_RESET,
>> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_COLD_RESET,
>> +	CXL_FEAT_ATTR_VALUE_PERSISTENCE_MAX
>> +};
>> +
>> +struct cxl_feat_entry {
>> +	uuid_t uuid;
>> +	__le16 id;
>> +	__le16 get_feat_size;
>> +	__le16 set_feat_size;
>> +	__le32 attr_flags;
>> +	u8 get_feat_ver;
>> +	u8 set_feat_ver;
>> +	__le16 set_effects;
>> +	u8 reserved[18];
>> +} __packed;
>> +
>> +struct cxl_mbox_get_sup_feats_out {
>> +	__le16 num_entries;
>> +	__le16 supported_feats;
>> +	u8 reserved[4];
>> +	struct cxl_feat_entry ents[]
>> +__counted_by(le32_to_cpu(supported_feats));
>> +} __packed;
>> +
>>  int cxl_internal_send_cmd(struct cxl_mailbox *cxl_mbox,
>>  			  struct cxl_mbox_cmd *cmd);
>>  int cxl_dev_state_identify(struct cxl_memdev_state *mds); @@ -824,4
>> +871,8 @@ struct cxl_hdm {  struct seq_file;  struct dentry
>> *cxl_debugfs_create_dir(const char *dir);  void cxl_dpa_debug(struct
>> seq_file *file, struct cxl_dev_state *cxlds);
>> +
>> +int cxl_get_supported_features(struct cxl_dev_state *cxlds); int
>> +cxl_get_supported_feature_entry(struct cxl_dev_state *cxlds, const uuid_t
>*feat_uuid,
>> +				    struct cxl_feat_entry *feat_entry_out);
>>  #endif /* __CXL_MEM_H__ */
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index
>> 3c73de475bf3..cec88e3a1754 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -872,6 +872,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const
>struct pci_device_id *id)
>>  	if (rc)
>>  		return rc;
>>
>> +	rc = cxl_get_supported_features(cxlds);
>> +	if (rc)
>> +		dev_dbg(&pdev->dev, "No features enumerated.\n");
>> +
>>  	rc = cxl_set_timestamp(mds);
>>  	if (rc)
>>  		return rc;
>> diff --git a/include/uapi/linux/cxl_mem.h
>> b/include/uapi/linux/cxl_mem.h index c6c0fe27495d..bd2535962f70 100644
>> --- a/include/uapi/linux/cxl_mem.h
>> +++ b/include/uapi/linux/cxl_mem.h
>> @@ -50,6 +50,7 @@
>>  	___C(GET_LOG_CAPS, "Get Log Capabilities"),			  \
>>  	___C(CLEAR_LOG, "Clear Log"),					  \
>>  	___C(GET_SUP_LOG_SUBLIST, "Get Supported Logs Sub-List"),	  \
>> +	___C(GET_SUPPORTED_FEATURES, "Get Supported Features"),
>	  \
>>  	___C(MAX, "invalid / last command")
>>
>>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
>