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

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

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

>> +		if (!is_cxl_feature_exclusive(entry))
>> +			user_feats++;
> 
> I guess that here should check all entries copied from mbox_out->ents, so it should be
> 
> 
> for (int i = 0; i < num_entries; i++)
> 
>     if (!is_cxl_feature_exclusive(entry + i))
> 
>         user_feats++;
> 
> 
> Other looks good to me.
> 
> Reviewed-by: Li Ming <ming.li@zohomail.com>
> 
> 
>>  		entry += num_entries;
>>  		/*
>>  		 * If the number of output entries is less than expected, add the
>> @@ -130,6 +155,7 @@ get_supported_features(struct cxl_features_state *cxlfs)
>>  	} while (remain_feats);
>>  
>>  	entries->num_features = count;
>> +	entries->num_user_features = user_feats;
>>  
>>  	return no_free_ptr(entries);
>>  }