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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

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.


> +
> +	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.
> +
> +		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?

> +	}
> +
> +	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
> 
>