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

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

On Thu, 12 Oct 2023 11:55:41 -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. Also add a check to ensure that the device's
> host bridge matches to one of the root decoder's downstream targets.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

So, if I read this right, QTG is required for probe to succeed.
Perhaps that's a little too heavy handed on something we haven't supported at
all until now?

Otherwise LGTM

> 
> ---
> v11:
> - Return when matched in match function (Jonathan)
> - Don't return after matched in caller function, still need to check pmem. (Jonathan)
> - Fix copy/paste error for pmem_qos_class. (Jonathan)
> - Use device_for_each_child() instead of bus_for_each_dev(). (Dan)
> - Add match of host_bridge to a root decoder target. (Dan)
> ---
>  drivers/cxl/mem.c |  121 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 121 insertions(+)
> 
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 317c7548e4e9..74f72e37ccfc 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -104,6 +104,123 @@ 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 (!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;
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +struct qos_hb_ctx {
> +	bool matched;
> +	struct device *host_bridge;
> +};
> +
> +static int match_cxlrd_hb(struct device *dev, void *data)
> +{
> +	struct cxl_switch_decoder *cxlsd;
> +	struct qos_hb_ctx *ctx = data;
> +	struct cxl_root_decoder *cxlrd;
> +	unsigned int seq;
> +
> +	if (!is_root_decoder(dev))
> +		return 0;
> +
> +	cxlrd = to_cxl_root_decoder(dev);
> +	cxlsd = &cxlrd->cxlsd;
> +
> +	do {
> +		seq = read_seqbegin(&cxlsd->target_lock);
> +		for (int i = 0; i < cxlsd->nr_targets; i++) {
> +			if (ctx->host_bridge ==
> +			    cxlsd->target[i]->dport_dev) {
> +				ctx->matched = true;
> +				return 1;
> +			}
> +		}
> +	} while (read_seqretry(&cxlsd->target_lock, seq));
> +
> +	return 0;
> +}
> +
> +static int cxl_qos_class_verify(struct cxl_memdev *cxlmd)
> +{
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> +	struct qos_class_ctx ctx;
> +	struct qos_hb_ctx hbctx;
> +	struct cxl_port *root_port;
> +	int rc;
> +
> +	root_port = find_cxl_root(cxlmd->endpoint);
> +	if (!root_port)
> +		return -ENODEV;
> +
> +	/* Check that the QTG IDs are all sane between end device and root decoders */
> +	if (mds->ram_qos_class != CXL_QOS_CLASS_INVALID) {
> +		ctx = (struct qos_class_ctx) {
> +			.matched = false,
> +			.dev_qos_class =  mds->ram_qos_class,
> +		};
> +		rc = device_for_each_child(&root_port->dev, &ctx, match_cxlrd_qos_class);
> +		if (rc < 0)
> +			goto out;
> +
> +		if (!ctx.matched)
> +			mds->ram_qos_class = CXL_QOS_CLASS_INVALID;
> +	}
> +
> +	if (mds->pmem_qos_class != CXL_QOS_CLASS_INVALID) {
> +		ctx = (struct qos_class_ctx) {
> +			.matched = false,
> +			.dev_qos_class =  mds->pmem_qos_class,
> +		};
> +		rc = device_for_each_child(&root_port->dev, &ctx, match_cxlrd_qos_class);
> +		if (rc < 0)
> +			goto out;
> +
> +		if (!ctx.matched)
> +			mds->pmem_qos_class = CXL_QOS_CLASS_INVALID;
> +	}
> +
> +	/* Check to make sure that the device's host bridge is under a root decoder */
> +	hbctx = (struct qos_hb_ctx) {
> +		.matched = false,
> +		.host_bridge = cxlmd->endpoint->host_bridge,
> +	};
> +	rc = device_for_each_child(&root_port->dev, &hbctx, match_cxlrd_hb);
> +	if (rc < 0)
> +		goto out;
> +
> +	if (!hbctx.matched) {
> +		mds->ram_qos_class = CXL_QOS_CLASS_INVALID;
> +		mds->pmem_qos_class = CXL_QOS_CLASS_INVALID;
> +	}
> +
> +out:
> +	put_device(&root_port->dev);
> +	return rc;
> +}
> +
>  static int cxl_mem_probe(struct device *dev)
>  {
>  	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> @@ -173,6 +290,10 @@ static int cxl_mem_probe(struct device *dev)
>  	if (rc)
>  		return rc;
>  
> +	rc = cxl_qos_class_verify(cxlmd);
> +	if (rc < 0)
> +		return rc;
> +
>  	if (resource_size(&cxlds->pmem_res) && IS_ENABLED(CONFIG_CXL_PMEM)) {
>  		rc = devm_cxl_add_nvdimm(cxlmd);
>  		if (rc == -ENODEV)
> 
> 
>