Re: [PATCH v4 06/15] cxl: Setup exclusive CXL features that are reserved for the kernel

[Dave Jiang <dave.jiang@intel.com>]

On 2/10/25 10:46 PM, Li Ming wrote:
> On 2/11/2025 1:50 AM, Dave Jiang wrote:
>>
>> On 2/7/25 11:22 PM, Li Ming wrote:
>>> On 2/8/2025 7:37 AM, Dave Jiang wrote:
>>>> Certain features will be exclusively used by components such as in
>>>> kernel RAS driver. Setup an exclusion list that can be later filtered
>>>> out before exposing to user space.
>>>>
>>>> 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>
>>>> ---
>>>>  drivers/cxl/core/features.c | 26 ++++++++++++++++++++++++++
>>>>  include/cxl/features.h      | 34 ++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 60 insertions(+)
>>>>
>>>> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
>>>> index d337224158fa..82f21f64452a 100644
>>>> --- a/drivers/cxl/core/features.c
>>>> +++ b/drivers/cxl/core/features.c
>>>> @@ -6,6 +6,28 @@
>>>>  #include "cxl.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;
>>>> @@ -46,6 +68,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);
>>>> @@ -120,6 +143,8 @@ get_supported_features(struct cxl_features_state *cxlfs)
>>>>  			return NULL;
>>>>  
>>>>  		memcpy(entry, mbox_out->ents, retrieved);
>> While you are pointing out the issue with copying multiple entries, I noticed that this needs to be fixed in patch2 as well. It should be:
>> memcpy(entry, mbox_out->ents + start, retrieved);
>>
>> DJ
> 
> I noticed there is a 'mbox_in.start_idx = cpu_to_le16(start)' before sending GET_SUPPORTED_FEATURES command every time.
> 
> I think the 'mbox_out->ents[0]' is the first feature info starting from 'start', so the change you mentioned is not needed?

Ah yep. Thanks. 
> 
> 
> Ming
>