[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bef6c9774c93486184af38091cb6207e@huawei.com>
Date: Wed, 21 Aug 2024 16:11:24 +0000
From: Shiju Jose <shiju.jose@...wei.com>
To: Dave Jiang <dave.jiang@...el.com>, "linux-edac@...r.kernel.org"
<linux-edac@...r.kernel.org>, "linux-cxl@...r.kernel.org"
<linux-cxl@...r.kernel.org>, "linux-acpi@...r.kernel.org"
<linux-acpi@...r.kernel.org>, "linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "bp@...en8.de" <bp@...en8.de>, "tony.luck@...el.com"
<tony.luck@...el.com>, "rafael@...nel.org" <rafael@...nel.org>,
"lenb@...nel.org" <lenb@...nel.org>, "mchehab@...nel.org"
<mchehab@...nel.org>, "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
"dave@...olabs.net" <dave@...olabs.net>, Jonathan Cameron
<jonathan.cameron@...wei.com>, "alison.schofield@...el.com"
<alison.schofield@...el.com>, "vishal.l.verma@...el.com"
<vishal.l.verma@...el.com>, "ira.weiny@...el.com" <ira.weiny@...el.com>,
"david@...hat.com" <david@...hat.com>, "Vilas.Sridharan@....com"
<Vilas.Sridharan@....com>, "leo.duran@....com" <leo.duran@....com>,
"Yazen.Ghannam@....com" <Yazen.Ghannam@....com>, "rientjes@...gle.com"
<rientjes@...gle.com>, "jiaqiyan@...gle.com" <jiaqiyan@...gle.com>,
"Jon.Grimm@....com" <Jon.Grimm@....com>, "dave.hansen@...ux.intel.com"
<dave.hansen@...ux.intel.com>, "naoya.horiguchi@....com"
<naoya.horiguchi@....com>, "james.morse@....com" <james.morse@....com>,
"jthoughton@...gle.com" <jthoughton@...gle.com>, "somasundaram.a@....com"
<somasundaram.a@....com>, "erdemaktas@...gle.com" <erdemaktas@...gle.com>,
"pgonda@...gle.com" <pgonda@...gle.com>, "duenwen@...gle.com"
<duenwen@...gle.com>, "mike.malvestuto@...el.com"
<mike.malvestuto@...el.com>, "gthelen@...gle.com" <gthelen@...gle.com>,
"wschwartz@...erecomputing.com" <wschwartz@...erecomputing.com>,
"dferguson@...erecomputing.com" <dferguson@...erecomputing.com>,
"wbs@...amperecomputing.com" <wbs@...amperecomputing.com>,
"nifan.cxl@...il.com" <nifan.cxl@...il.com>, "jgroves@...ron.com"
<jgroves@...ron.com>, "vsalve@...ron.com" <vsalve@...ron.com>, tanxiaofei
<tanxiaofei@...wei.com>, "Zengtao (B)" <prime.zeng@...ilicon.com>, "Roberto
Sassu" <roberto.sassu@...wei.com>, "kangkang.shen@...urewei.com"
<kangkang.shen@...urewei.com>, wanghuiqiang <wanghuiqiang@...wei.com>,
Linuxarm <linuxarm@...wei.com>
Subject: RE: [PATCH v11 04/14] cxl/mbox: Add GET_SUPPORTED_FEATURES mailbox
command
Hi Dave,
>-----Original Message-----
>From: Dave Jiang <dave.jiang@...el.com>
>Sent: 20 August 2024 23:46
>To: Shiju Jose <shiju.jose@...wei.com>; linux-edac@...r.kernel.org; linux-
>cxl@...r.kernel.org; linux-acpi@...r.kernel.org; linux-mm@...ck.org; linux-
>kernel@...r.kernel.org
>Cc: bp@...en8.de; tony.luck@...el.com; rafael@...nel.org; lenb@...nel.org;
>mchehab@...nel.org; dan.j.williams@...el.com; dave@...olabs.net; Jonathan
>Cameron <jonathan.cameron@...wei.com>; alison.schofield@...el.com;
>vishal.l.verma@...el.com; ira.weiny@...el.com; david@...hat.com;
>Vilas.Sridharan@....com; leo.duran@....com; Yazen.Ghannam@....com;
>rientjes@...gle.com; jiaqiyan@...gle.com; Jon.Grimm@....com;
>dave.hansen@...ux.intel.com; naoya.horiguchi@....com;
>james.morse@....com; jthoughton@...gle.com; somasundaram.a@....com;
>erdemaktas@...gle.com; pgonda@...gle.com; duenwen@...gle.com;
>mike.malvestuto@...el.com; gthelen@...gle.com;
>wschwartz@...erecomputing.com; dferguson@...erecomputing.com;
>wbs@...amperecomputing.com; nifan.cxl@...il.com; jgroves@...ron.com;
>vsalve@...ron.com; tanxiaofei <tanxiaofei@...wei.com>; Zengtao (B)
><prime.zeng@...ilicon.com>; Roberto Sassu <roberto.sassu@...wei.com>;
>kangkang.shen@...urewei.com; wanghuiqiang <wanghuiqiang@...wei.com>;
>Linuxarm <linuxarm@...wei.com>
>Subject: Re: [PATCH v11 04/14] cxl/mbox: Add GET_SUPPORTED_FEATURES
>mailbox command
>
>
>
>On 8/16/24 9:42 AM, shiju.jose@...wei.com wrote:
>> From: Shiju Jose <shiju.jose@...wei.com>
>>
>> Add support for GET_SUPPORTED_FEATURES mailbox command.
>>
>> CXL spec 3.1 section 8.2.9.6 describes optional device specific features.
>> CXL devices support features with changeable attributes.
>>
>> CXL spec 3.1 section 8.2.9.6.1 describes Get Supported features command.
>> Get Supported Features retrieves the list of supported device specific
>> features. The settings of a feature can be retrieved using Get Feature
>> and optionally modified using Set Feature.
>>
>> Signed-off-by: Shiju Jose <shiju.jose@...wei.com>
>> ---
>> drivers/cxl/core/mbox.c | 68
>+++++++++++++++++++++++++++++++++++++++++
>> drivers/cxl/cxlmem.h | 63 ++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 131 insertions(+)
>>
>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index
>> 2626f3fff201..760fa3e1075f 100644
>> --- a/drivers/cxl/core/mbox.c
>> +++ b/drivers/cxl/core/mbox.c
>> @@ -1324,6 +1324,74 @@ int cxl_set_timestamp(struct cxl_memdev_state
>> *mds) } EXPORT_SYMBOL_NS_GPL(cxl_set_timestamp, CXL);
>>
>> +int cxl_get_supported_features(struct cxl_memdev_state *mds,
>> + u32 count, u16 start_index,
>> + struct cxl_mbox_get_supp_feats_out *feats_out) {
>> + struct cxl_mbox_get_supp_feats_in pi;
>> + struct cxl_mbox_cmd mbox_cmd;
>> + int rc;
>> +
>> + pi.count = cpu_to_le32(count);
>> + pi.start_index = cpu_to_le16(start_index);
>> +
>> + mbox_cmd = (struct cxl_mbox_cmd) {
>> + .opcode = CXL_MBOX_OP_GET_SUPPORTED_FEATURES,
>> + .size_in = sizeof(pi),
>> + .payload_in = &pi,
>> + .size_out = count,
>> + .payload_out = feats_out,
>> + .min_out = sizeof(*feats_out),
>> + };
>> + rc = cxl_internal_send_cmd(mds, &mbox_cmd);
>> + if (rc < 0)
>> + return rc;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_get_supported_features, CXL);
>> +
>> +int cxl_get_supported_feature_entry(struct cxl_memdev_state *mds, const
>uuid_t *feat_uuid,
>> + struct cxl_mbox_supp_feat_entry
>*feat_entry_out)
>
>Hi Shiju,
>thoughts on storing all the supported features meta info from
>GET_SUPPORTED_FEATURES command in the cxl core driver during device
>enumeration and the CXL EDAC code can just query the core instead of reading
>from the device? Just looking forward to supporting other read/set features
>operations in the future and avoiding going through this entire read and find
>routine for each feature.
Thanks for the suggestion.
I saw you have implementation for storing all the supported features meta info from GET_SUPPORTED_FEATURES command here.
https://patchwork.kernel.org/project/cxl/patch/20240718213446.1750135-5-dave.jiang@intel.com/
I am fine basing on it if this patch is ready to go?
>
>DJ
Thanks,
Shiju
>
>> +{
>> + struct cxl_mbox_supp_feat_entry *feat_entry;
>> + int feat_index, feats_out_size;
>> + int nentries, count;
>> + int ret;
>> +
>> + feat_index = 0;
>> + feats_out_size = sizeof(struct cxl_mbox_get_supp_feats_out) +
>> + sizeof(struct cxl_mbox_supp_feat_entry);
>> + struct cxl_mbox_get_supp_feats_out *feats_out __free(kfree) =
>> + kmalloc(feats_out_size, GFP_KERNEL);
>> + if (!feats_out)
>> + return -ENOMEM;
>> +
>> + while (true) {
>> + memset(feats_out, 0, feats_out_size);
>> + ret = cxl_get_supported_features(mds, feats_out_size,
>> + feat_index, feats_out);
>> + if (ret)
>> + return ret;
>> +
>> + nentries = feats_out->nr_entries;
>> + if (!nentries)
>> + return -EOPNOTSUPP;
>> +
>> + /* Check CXL memdev supports the feature */
>> + feat_entry = feats_out->feat_entries;
>> + for (count = 0; count < nentries; count++, feat_entry++) {
>> + if (uuid_equal(&feat_entry->uuid, feat_uuid)) {
>> + memcpy(feat_entry_out, feat_entry,
>> + sizeof(*feat_entry_out));
>> + return 0;
>> + }
>> + }
>> + feat_index += nentries;
>> + }
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_get_supported_feature_entry, 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
>> af8169ccdbc0..9939c771f642 100644
>> --- a/drivers/cxl/cxlmem.h
>> +++ b/drivers/cxl/cxlmem.h
>> @@ -531,6 +531,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,
>> @@ -700,6 +701,63 @@ struct cxl_mbox_set_timestamp_in {
>>
>> } __packed;
>>
>> +/*
>> + * Get Supported Features CXL 3.1 Spec 8.2.9.6.1 */
>> +
>> +/*
>> + * Get Supported Features input payload
>> + * CXL rev 3.1 section 8.2.9.6.1 Table 8-95 */ struct
>> +cxl_mbox_get_supp_feats_in {
>> + __le32 count;
>> + __le16 start_index;
>> + u8 rsvd[2];
>> +} __packed;
>> +
>> +/*
>> + * Get Supported Features Supported Feature Entry
>> + * CXL rev 3.1 section 8.2.9.6.1 Table 8-97 */
>> +/* 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_mbox_supp_feat_entry {
>> + uuid_t uuid;
>> + __le16 index;
>> + __le16 get_size;
>> + __le16 set_size;
>> + __le32 attr_flags;
>> + u8 get_version;
>> + u8 set_version;
>> + __le16 set_effects;
>> + u8 rsvd[18];
>> +} __packed;
>> +
>> +/*
>> + * Get Supported Features output payload
>> + * CXL rev 3.1 section 8.2.9.6.1 Table 8-96 */ struct
>> +cxl_mbox_get_supp_feats_out {
>> + __le16 nr_entries;
>> + __le16 nr_supported;
>> + u8 rsvd[4];
>> + struct cxl_mbox_supp_feat_entry feat_entries[]; } __packed;
>> +
>> /* Get Poison List CXL 3.0 Spec 8.2.9.8.4.1 */ struct
>> cxl_mbox_poison_in {
>> __le64 offset;
>> @@ -831,6 +889,11 @@ void cxl_event_trace_record(const struct
>cxl_memdev *cxlmd,
>> enum cxl_event_type event_type,
>> const uuid_t *uuid, union cxl_event *evt); 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);
>int
>> +cxl_get_supported_feature_entry(struct cxl_memdev_state *mds, const
>uuid_t *feat_uuid,
>> + struct cxl_mbox_supp_feat_entry
>*feat_entry_out);
>> 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);
Powered by blists - more mailing lists