Re: [PATCH v10 22/22] cxl: Check qos_class validity on memdev probe

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

On 10/11/23 06:29, Jonathan Cameron wrote:
> On Tue, 10 Oct 2023 18:07:11 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> Add a check to make sure the qos_class for the device will match one of
>> the root decoders qos_class. If no match is found, then the qos_class for
>> the device is set to invalid.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  drivers/cxl/mem.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>
>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>> index 317c7548e4e9..3495119d2edf 100644
>> --- a/drivers/cxl/mem.c
>> +++ b/drivers/cxl/mem.c
>> @@ -104,6 +104,70 @@ static int cxl_debugfs_poison_clear(void *data, u64 dpa)
>>  DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
>>  			 cxl_debugfs_poison_clear, "%llx\n");
>>  
>> +struct qos_class_ctx {
>> +	bool matched;
>> +	int dev_qos_class;
>> +};
>> +
>> +static int match_cxlrd_qos_class(struct device *dev, void *data)
>> +{
>> +	struct qos_class_ctx *ctx = data;
>> +	struct cxl_root_decoder *cxlrd;
>> +
>> +	if (ctx->matched)
>> +		return 0;
>> +
>> +	if (!is_root_decoder(dev))
>> +		return 0;
>> +
>> +	cxlrd = to_cxl_root_decoder(dev);
>> +	if (cxlrd->qos_class == CXL_QOS_CLASS_INVALID ||
>> +	    ctx->dev_qos_class == CXL_QOS_CLASS_INVALID)
>> +		return 0;
>> +
>> +	if (cxlrd->qos_class == ctx->dev_qos_class)
>> +		ctx->matched = 1;
> If matched, why not terminate the bus_for_each_dev()
> That is return 1 and amend check to be if (rc < 0)
> Not that this ever returns < 0 anyway.  It might in
> future though so that test makes sense as defensive measure.

Will update.

> 
> 
>> +
>> +	return 0;
>> +}
>> +
>> +static int cxl_qos_class_verify(struct cxl_memdev *cxlmd)
>> +{
>> +	struct device *dev = &cxlmd->dev;
>> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
>> +	struct qos_class_ctx ctx;
>> +	int rc;
>> +
>> +	if (mds->ram_qos_class != CXL_QOS_CLASS_INVALID) {
>> +		ctx.matched = false;
>> +		ctx.dev_qos_class =  mds->ram_qos_class;
>> +		rc = bus_for_each_dev(dev->bus, NULL, &ctx, match_cxlrd_qos_class);
>> +		if (rc)
>> +			return rc;
>> +
>> +		if (ctx.matched)
>> +			return 0;
> Early return doesn't make sense to me given not checked the pmem one yet.

You are right. Will fix.

>> +
>> +		mds->ram_qos_class = CXL_QOS_CLASS_INVALID;
>> +	}
>> +
>> +	if (mds->pmem_qos_class != CXL_QOS_CLASS_INVALID) {
>> +		ctx.matched = false;
>> +		ctx.dev_qos_class = mds->pmem_qos_class;
>> +		rc = bus_for_each_dev(dev->bus, NULL, &ctx, match_cxlrd_qos_class);
>> +		if (rc)
>> +			return rc;
>> +
>> +		if (ctx.matched)
>> +			return 0;
>> +
>> +		mds->ram_qos_class = CXL_QOS_CLASS_INVALID;
> 
> pmem_qos_class?

copy/paste error. will fix.

> 
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int cxl_mem_probe(struct device *dev)
>>  {
>>  	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>> @@ -181,6 +245,10 @@ static int cxl_mem_probe(struct device *dev)
>>  			return rc;
>>  	}
>>  
>> +	rc = cxl_qos_class_verify(cxlmd);
>> +	if (rc)
>> +		return rc;
>> +
>>  	/*
>>  	 * The kernel may be operating out of CXL memory on this device,
>>  	 * there is no spec defined way to determine whether this device
>>
>>
>