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

Jonathan Cameron <[email protected]> Tue, 4 Aug 2026 01:07:48 +0100
Newsgroups dev.linux.lists.sashiko-reviews,dev.linux.lists.nvdimm,org.kernel.vger.linux-cxl
Message-ID <20260804010748.7f9f8174@jic23-huawei>
On Fri, 31 Jul 2026 09:02:49 +0000
[email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The commit message claims to extract and store the 'read only' attribute from DSMAS tables, but this logic is completely missing from the code.

Given it will make a lot of difference to a user if they think they have writeable
memory that isn't - I think we probably do want to have readonly here.

> [ ... ]
> > @@ -266,15 +268,26 @@ static void cxl_memdev_set_qos_class(struct cxl_dev_state *cxlds,
> >  		bool found = false;
> >  
> >  		for (int i = 0; i < cxlds->nr_partitions; i++) {
> > +			enum cxl_partition_mode mode = cxlds->part[i].mode;
> >  			struct resource *res = &cxlds->part[i].res;
> > +			u8 handle = cxlds->part[i].handle;
> >  			struct range range = {
> >  				.start = res->start,
> >  				.end = res->end,
> >  			};
> >  
> >  			if (range_contains(&range, &dent->dpa_range)) {
> > +				if (mode == CXL_PARTMODE_DYNAMIC_RAM_1 &&
> > +				    dent->handle != handle) {
> > +					dev_warn(dev,
> > +						 "Dynamic RAM perf mismatch; %pra (%u) vs %pra (%u)\n",
> > +						 &range, handle, &dent->dpa_range,
> > +						 dent->handle);
> > +					continue;
> > +				}  
> 
> [Severity: Medium]
> Could this trigger spurious warnings on valid hardware configurations?
> 
> The CDAT specification allows overlapping DSMAS ranges with different handles
> to support multiple memory types or sharing characteristics for the same DPA
> space. 

Huh?  Where did it get that from.  I'm 99% sure you can't have overlapping
DSMAS entries for the same DPA space.

> 
> If a compliant device provides multiple valid DSMAS entries that overlap
> the partition's range but have different handles, won't iterating over them 
> incorrectly flag the unselected ones as a perf mismatch?
>