Re: [PATCH v15 3/8] cxl/cdat: Gather DSMAS data for DCD partitions

[Anisa Su <anisa.su887@gmail.com>]

On Wed, Oct 07, 2026 at 08:33:12PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] cxl: Ineffective DCD disablement leaves partially configured partitions exposed
> - [Medium] cxl: QoS class 0 incorrectly overwrites CXL_QOS_CLASS_INVALID
> - [Low] cxl: Lockless modification of DPA partition attributes exposes torn reads
> --
> 
> commit 6722908eab4fa5c8dd9820d34f1ec3beb729c8f0
> Author: Ira Weiny <iweiny@kernel.org>
> 
> cxl/cdat: Gather DSMAS data for DCD partitions
> 
> Additional DCD partition (AKA region) information is contained in the
> DSMAS CDAT tables, including performance, shareable and read-only
> attributes. Match DCD partitions with DSMAS tables and store the meta
> data.
> 
> > diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> > index 5c9f072625135..07aedff657d9e 100644
> > --- a/drivers/cxl/core/cdat.c
> > +++ b/drivers/cxl/core/cdat.c
> [ ... ]
> > @@ -219,6 +226,13 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
> >  	xa_for_each(dsmas_xa, index, dent) {
> >  		int qos_class;
> >  
> > +		/* No DSLBIS for this DSMAS; nothing to hand the QTG _DSM */
> > +		if (!dent->cdat_coord[ACCESS_COORDINATE_CPU].read_bandwidth &&
> > +		    !dent->cdat_coord[ACCESS_COORDINATE_CPU].write_bandwidth &&
> > +		    !dent->cdat_coord[ACCESS_COORDINATE_CPU].read_latency &&
> > +		    !dent->cdat_coord[ACCESS_COORDINATE_CPU].write_latency)
> > +			continue;
> 
> [Severity: Medium]
> When this continue statement is taken, dent->qos_class remains 0 (from
> kzalloc_obj zero-initialization).
> 
> If at least one other entry succeeds, cxl_memdev_apply_dsmas() will be called
> with CXL_DSMAS_APPLY_FLAGS_PERF, which unconditionally calls update_perf_entry()
> and overwrites part->perf.qos_class (previously CXL_QOS_CLASS_INVALID) with 0.
> 
> Since 0 is a valid QoS class in CXL, does this falsely advertise a QoS class of
> 0 to userspace for partitions lacking DSLBIS performance coordinates?
> 
Fixed. dsams_entry.qos_class is initialized to CXL_QOS_CLASS_INVALID in
cdat_dsmas_handler().

> > +
> >  		cxl_coordinates_combine(dent->coord, dent->cdat_coord, ep_c);
> >  		dent->entries = 1;
> >  		rc = cxl_root->ops.qos_class(cxl_root,
> [ ... ]
> > @@ -255,35 +269,90 @@ static void update_perf_entry(struct device *dev, struct dsmas_entry *dent,
> >  		dent->coord[ACCESS_COORDINATE_CPU].write_latency);
> >  }
> >  
> > -static void cxl_memdev_set_qos_class(struct cxl_dev_state *cxlds,
> > -				     struct xarray *dsmas_xa)
> > +/*
> > + * A DCD shall describe its DC partition in a DSMAS entry (CXL r4.0 9.13.3).
> > + * Without one the partition's attributes are unknown, so disable DCD.
> > + */
> > +static void cxl_dcd_dsmas_missing(struct cxl_dev_state *cxlds, const char *why)
> > +{
> > +	struct cxl_memdev_state *mds;
> > +
> > +	if (cxlds->type != CXL_DEVTYPE_CLASSMEM)
> > +		return;
> > +
> > +	mds = to_cxl_memdev_state(cxlds);
> > +	if (!cxl_dcd_supported(mds) ||
> > +	    !cxl_part_size(cxlds, CXL_PARTMODE_DYNAMIC_RAM))
> > +		return;
> > +
> > +	dev_warn(cxlds->dev, "DC partition has no valid DSMAS entry (%s), disabling DCD\n",
> > +		 why);
> > +	cxl_disable_dcd(mds);
> 
> [Severity: High]
> Does clearing the mds->dcd_supported flag effectively disable the partition?
> 
> Since cxl_configure_dcd() and cxl_dpa_setup() have already executed during
> cxl_pci_probe(), the DCD partition is permanently added to cxlds->part and its
> DPA resource is registered.
> 
> Because mds->dcd_supported is never evaluated again during region creation or
> sysfs access, does the partition remain fully exposed and usable by userspace
> without valid DSMAS attributes?
> 
Nothing is exposed in sysfs by this series. The rest of the series that does
adds those capabilities will hide the memdev dynamic_ram group and refuse the
decoder mode if dcd is not supported.
> > +}
> > +
> > +enum cxl_dsmas_apply {
> > +	CXL_DSMAS_APPLY_FLAGS,		/* shareable etc.; no QTG data */
> > +	CXL_DSMAS_APPLY_FLAGS_PERF,	/* perf coordinates and qos_class */
> > +};
> > +
> > +/*
> > + * DSMAS flags such as shareable are properties of the CDAT entry and are
> > + * applied regardless; perf data is applied only if the QTG lookup produced
> > + * recommendations.
> > + */
> > +static void cxl_memdev_apply_dsmas(struct cxl_dev_state *cxlds,
> > +				   struct xarray *dsmas_xa,
> > +				   enum cxl_dsmas_apply attrs)
> >  {
> >  	struct device *dev = cxlds->dev;
> >  	struct dsmas_entry *dent;
> > +	bool dc_matched = false;
> >  	unsigned long index;
> >  
> >  	xa_for_each(dsmas_xa, index, dent) {
> >  		bool found = false;
> >  
> >  		for (int i = 0; i < cxlds->nr_partitions; i++) {
> > -			struct resource *res = &cxlds->part[i].res;
> > +			struct cxl_dpa_partition *part = &cxlds->part[i];
> [ ... ]
> > +			if (attrs == CXL_DSMAS_APPLY_FLAGS_PERF)
> > +				update_perf_entry(dev, dent, &part->perf);
> > +			if (part->mode == CXL_PARTMODE_DYNAMIC_RAM) {
> > +				part->shareable = dent->shareable;
> > +				part->read_only = dent->read_only;
> > +				dc_matched = true;
> > +			}
> 
> [Severity: Low]
> Are these partition attributes being modified locklessly?
> 
> The CXL memdev and its sysfs attributes are registered and visible during
> cxl_pci_probe(), while this CDAT parsing happens later in
> cxl_endpoint_port_probe().
> 
> If part->perf, part->shareable, and part->read_only are updated without holding
> the cxl_rwsem.dpa lock, can concurrent userspace sysfs readers observe
> intermediate, torn, or partially updated performance states?
> 
update_perf_entry() is the function that writes to part->perf and not
touched by this series.

shareable and read-only are bools, so stores are atomic.

> > +			break;
> >  		}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261007201828.952-1-anisa.su@samsung.com?part=3