Re: [PATCH v4 05/23] cxl: Add callback to parse the DSMAS subtables from CDAT

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

On Wed, 19 Apr 2023 13:21:31 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Provide a callback function to the CDAT parser in order to parse the Device
> Scoped Memory Affinity Structure (DSMAS). Each DSMAS structure contains the
> DPA range and its associated attributes in each entry. See the CDAT
> specification for details.
> 
> Coherent Device Attribute Table 1.03 2.1 Device Scoped memory Affinity
> Structure (DSMAS)

I'm not sure what purpose of this is. If it's just detecting problems
with the entry because we aren't interested in the content yet, then fine
but good to make that clear in patch intro.

Maybe I'm missing something!

Thanks,

Jonathan

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> v3:
> - Add spec section number. (Alison)
> - Remove cast from void *. (Alison)
> - Refactor cxl_port_probe() block. (Alison)
> - Move CDAT parse to cxl_endpoint_port_probe()
> 
> v2:
> - Add DSMAS table size check. (Lukas)
> - Use local DSMAS header for LE handling.
> - Remove dsmas lock. (Jonathan)
> - Fix handle size (Jonathan)
> - Add LE to host conversion for DSMAS address and length.
> - Make dsmas_list local


> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 615e0ef6b440..3022bdd52439 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -57,6 +57,16 @@ static int discover_region(struct device *dev, void *root)
>  	return 0;
>  }

>  static int cxl_switch_port_probe(struct cxl_port *port)
>  {
>  	struct cxl_hdm *cxlhdm;
> @@ -125,6 +135,18 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  	device_for_each_child(&port->dev, root, discover_region);
>  	put_device(&root->dev);
>  
> +	if (port->cdat.table) {
> +		LIST_HEAD(dsmas_list);
> +
> +		rc = cdat_table_parse_dsmas(port->cdat.table,
> +					    cxl_dsmas_parse_entry,
> +					    (void *)&dsmas_list);
> +		if (rc < 0)
> +			dev_warn(&port->dev, "Failed to parse DSMAS: %d\n", rc);
> +
> +		dsmas_list_destroy(&dsmas_list);

I'm a little confused here.  What's the point?  Parse them then throw the info away?
Maybe a comment if all we are trying to do is warn about CDAT problems.


> +	}
> +
>  	return 0;
>  }
>  
> 
>