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 | org.kernel.vger.linux-cxl,dev.linux.lists.nvdimm,dev.linux.lists.sashiko-reviews |
|---|---|
| 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? >