2024-02-15 11:15:47

by Shiju Jose

Subject: [RFC PATCH v6 03/12] cxl/mbox: Add SET_FEATURE mailbox command

From: Shiju Jose <[email protected]>

Add support for SET_FEATURE mailbox command.

CXL spec 3.1 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.

Signed-off-by: Shiju Jose <[email protected]>
---
drivers/cxl/core/mbox.c | 14 ++++++++++++++
drivers/cxl/cxlmem.h | 27 +++++++++++++++++++++++++++
2 files changed, 41 insertions(+)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index f43189b6859a..056576862b8a 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1335,6 +1335,20 @@ int cxl_get_feature(struct cxl_memdev_state *mds,
}
EXPORT_SYMBOL_NS_GPL(cxl_get_feature, CXL);

+int cxl_set_feature(struct cxl_memdev_state *mds, void *feat_in, size_t size)
+{
+ struct cxl_mbox_cmd mbox_cmd;
+
+ mbox_cmd = (struct cxl_mbox_cmd) {
+ .opcode = CXL_MBOX_OP_SET_FEATURE,
+ .size_in = size,
+ .payload_in = feat_in,
+ };
+
+ return cxl_internal_send_cmd(mds, &mbox_cmd);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_set_feature, CXL);
+
int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
struct cxl_region *cxlr)
{
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index eaecc3234cfd..2223ef3d3140 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -531,6 +531,7 @@ enum cxl_opcode {
CXL_MBOX_OP_GET_LOG = 0x0401,
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,
@@ -778,6 +779,31 @@ struct cxl_mbox_get_feat_in {
u8 selection;
} __packed;

+/* Set Feature CXL 3.1 Spec 8.2.9.6.3 */
+/*
+ * Set Feature input payload
+ * CXL rev 3.1 section 8.2.9.6.3 Table 8-101
+ */
+/* Set Feature : Payload in flags */
+#define CXL_SET_FEAT_FLAG_DATA_TRANSFER_MASK GENMASK(2, 0)
+enum cxl_set_feat_flag_data_transfer {
+ CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER,
+ 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_MOD_VALUE_SAVED_ACROSS_RESET BIT(3)
+
+struct cxl_mbox_set_feat_in {
+ uuid_t uuid;
+ __le32 flags;
+ __le16 offset;
+ u8 version;
+ u8 rsvd[9];
+} __packed;
+
/* Get Poison List CXL 3.0 Spec 8.2.9.8.4.1 */
struct cxl_mbox_poison_in {
__le64 offset;
@@ -914,6 +940,7 @@ int cxl_get_supported_features(struct cxl_memdev_state *mds,
void *feats_out);
int cxl_get_feature(struct cxl_memdev_state *mds,
struct cxl_mbox_get_feat_in *pi, void *feat_out);
+int cxl_set_feature(struct cxl_memdev_state *mds, void *feat_in, size_t size);
int cxl_poison_state_init(struct cxl_memdev_state *mds);
int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
struct cxl_region *cxlr);
--
2.34.1

2024-02-20 11:28:05

by Jonathan Cameron

Subject: Re: [RFC PATCH v6 03/12] cxl/mbox: Add SET_FEATURE mailbox command

On Thu, 15 Feb 2024 19:14:45 +0800
<[email protected]> wrote:

> From: Shiju Jose <[email protected]>
>
> Add support for SET_FEATURE mailbox command.
>
> CXL spec 3.1 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.

Same thing as patch 2. We should be able to call this with a feature
of any size and have it send multiple commands if necessary due to a small mailbox.
We don't want higher layers to have to deal with complexity.

We work around this for many other similar multipart transfers because they
allow more direct control (e.g. get single item on poison list etc). Can't
do that for features :( Get supported logs sublist will run into the same
thing but the code isn't yet upstream (and the current implementation makes
it a userspace problem).

>
> Signed-off-by: Shiju Jose <[email protected]>
> ---
> drivers/cxl/core/mbox.c | 14 ++++++++++++++
> drivers/cxl/cxlmem.h | 27 +++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index f43189b6859a..056576862b8a 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1335,6 +1335,20 @@ int cxl_get_feature(struct cxl_memdev_state *mds,
> }
> EXPORT_SYMBOL_NS_GPL(cxl_get_feature, CXL);
>
> +int cxl_set_feature(struct cxl_memdev_state *mds, void *feat_in, size_t size)

Signature should be
int cxl_set_feature(struct cxl_memdev_state *mds, uuid_t feat, void *feat_data,
size_t feat_data_size);

And the internal workings of this should deal with multipart transfers if
needed (all current features are fairly small so the probably aren't).


> +{
> + struct cxl_mbox_cmd mbox_cmd;
> +
> + mbox_cmd = (struct cxl_mbox_cmd) {
> + .opcode = CXL_MBOX_OP_SET_FEATURE,
> + .size_in = size,
> + .payload_in = feat_in,
> + };
> +
> + return cxl_internal_send_cmd(mds, &mbox_cmd);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_set_feature, CXL);
> +
> int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
> struct cxl_region *cxlr)
> {
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index eaecc3234cfd..2223ef3d3140 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -531,6 +531,7 @@ enum cxl_opcode {
> CXL_MBOX_OP_GET_LOG = 0x0401,
> 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,
> @@ -778,6 +779,31 @@ struct cxl_mbox_get_feat_in {
> u8 selection;
> } __packed;
>
> +/* Set Feature CXL 3.1 Spec 8.2.9.6.3 */
> +/*
> + * Set Feature input payload
> + * CXL rev 3.1 section 8.2.9.6.3 Table 8-101
> + */
> +/* Set Feature : Payload in flags */
> +#define CXL_SET_FEAT_FLAG_DATA_TRANSFER_MASK GENMASK(2, 0)
> +enum cxl_set_feat_flag_data_transfer {
> + CXL_SET_FEAT_FLAG_FULL_DATA_TRANSFER,
> + 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_MOD_VALUE_SAVED_ACROSS_RESET BIT(3)
> +
> +struct cxl_mbox_set_feat_in {
> + uuid_t uuid;
> + __le32 flags;
> + __le16 offset;
> + u8 version;
> + u8 rsvd[9];
> +} __packed;
> +
> /* Get Poison List CXL 3.0 Spec 8.2.9.8.4.1 */
> struct cxl_mbox_poison_in {
> __le64 offset;
> @@ -914,6 +940,7 @@ int cxl_get_supported_features(struct cxl_memdev_state *mds,
> void *feats_out);
> int cxl_get_feature(struct cxl_memdev_state *mds,
> struct cxl_mbox_get_feat_in *pi, void *feat_out);
> +int cxl_set_feature(struct cxl_memdev_state *mds, void *feat_in, size_t size);
> int cxl_poison_state_init(struct cxl_memdev_state *mds);
> int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
> struct cxl_region *cxlr);