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

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

On Thu, 20 Apr 2023 12:33:50 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> 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!
> 
Ah. Got to next patch.  Perhaps a forwards reference to that will avoid
anyone else wondering what is going on here!

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