On Tue, Jul 16, 2024 at 04:03:29PM +0100, shiju.jose@xxxxxxxxxx wrote:
> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
>
> Add support for GET_FEATURE mailbox command.
>
> CXL spec 3.1 section 8.2.9.6 describes optional device specific features.
> The settings of a feature can be retrieved using Get Feature command.
>
> Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
> ---
Minor comments inline.
> drivers/cxl/core/mbox.c | 37 +++++++++++++++++++++++++++++++++++++
> drivers/cxl/cxlmem.h | 27 +++++++++++++++++++++++++++
> 2 files changed, 64 insertions(+)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 9b9b1d26454e..b1eeed508459 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1351,6 +1351,43 @@ int cxl_get_supported_features(struct cxl_memdev_state *mds,
> }
> EXPORT_SYMBOL_NS_GPL(cxl_get_supported_features, CXL);
>
> +size_t cxl_get_feature(struct cxl_memdev_state *mds,
> + const uuid_t feat_uuid, void *feat_out,
> + size_t feat_out_size,
> + enum cxl_get_feat_selection selection)
feat_uuid and selection are both payload inputs, maybe more natural to put
them together before feat_out.
> +{
> + 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;
> +
> + size_out = min(feat_out_size, mds->payload_size);
> + pi.uuid = feat_uuid;
> + pi.selection = selection;
> + do {
> + data_to_rd_size = min(feat_out_size - data_rcvd_size, mds->payload_size);
> + pi.offset = cpu_to_le16(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(mds, &mbox_cmd);
> + if (rc < 0 || mbox_cmd.size_out == 0)
Is there other case when size_out will be 0 other than the feat_out_size
is 0?
If feat_out_size is 0, maybe we return directly, or we use while () {},
instead of do {} while.
Anyway, if there is no other case that will return size_out as 0, we can
avoid the check.
Fan
> + return 0;
> + data_rcvd_size += mbox_cmd.size_out;
> + } while (data_rcvd_size < feat_out_size);
> +
> + return data_rcvd_size;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_get_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 b0e1565b9d2e..25698a6fbe66 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -531,6 +531,7 @@ enum cxl_opcode {
> 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_IDENTIFY = 0x4000,
> CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100,
> CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101,
> @@ -757,6 +758,28 @@ struct cxl_mbox_get_supp_feats_out {
> struct cxl_mbox_supp_feat_entry feat_entries[];
> } __packed;
>
> +/*
> + * Get Feature CXL 3.1 Spec 8.2.9.6.2
> + */
> +
> +/*
> + * Get Feature input payload
> + * CXL rev 3.1 section 8.2.9.6.2 Table 8-99
> + */
> +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_mbox_get_feat_in {
> + uuid_t uuid;
> + __le16 offset;
> + __le16 count;
> + u8 selection;
> +} __packed;
> +
> /* Get Poison List CXL 3.0 Spec 8.2.9.8.4.1 */
> struct cxl_mbox_poison_in {
> __le64 offset;
> @@ -891,6 +914,10 @@ int cxl_set_timestamp(struct cxl_memdev_state *mds);
> int cxl_get_supported_features(struct cxl_memdev_state *mds,
> u32 count, u16 start_index,
> struct cxl_mbox_get_supp_feats_out *feats_out);
> +size_t cxl_get_feature(struct cxl_memdev_state *mds,
> + const uuid_t feat_uuid, void *feat_out,
> + size_t feat_out_size,
> + enum cxl_get_feat_selection selection);
> 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
>
