On Thu, May 28, 2026 at 02:34:49PM -0700, Dave Jiang wrote:
>
>
> On 5/23/26 2:43 AM, Anisa Su wrote:
> > Extend cxl_validate_extent() — the per-extent check of the add pipeline
> > to check partition membership.
> >
> > Resolves an extent's DPA to its containing DC partition. Then based on
> > if the partition is shareable:
> >
> > - Shareable: tag must be non-null and shared_extn_seq must be non-zero
> > — multiple hosts reading the same allocation rely on the device-
> > stamped 1..n sequence to assemble extents in agreed order.
> > - Non-sharable: shared_extn_seq must be zero — sequencing is
> > meaningless when only one host consumes the allocation; tag is
> > optional (null UUID permitted).
> >
> > Any cross-mix is a device firmware bug; reject the extent.
> >
> > Based on patches by John Groves.
> >
> > Signed-off-by: Ira Weiny <[email protected]>
> > Signed-off-by: John Groves <[email protected]>
> > Signed-off-by: Anisa Su <[email protected]>
> >
> > ---
> > Changes:
> > [anisa: split out as a separate validation step]
> > ---
> > drivers/cxl/core/core.h | 4 ++
> > drivers/cxl/core/extent.c | 78 +++++++++++++++++++++++++++++++++++++--
> > 2 files changed, 79 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> > index 1bae80dbf991..30b6b05b155b 100644
> > --- a/drivers/cxl/core/core.h
> > +++ b/drivers/cxl/core/core.h
> > @@ -179,6 +179,10 @@ int cxl_pci_get_bandwidth(struct pci_dev *pdev, struct
> > access_coordinate *c);
> > int cxl_port_get_switch_dport_bandwidth(struct cxl_port *port,
> > struct access_coordinate *c);
> > void memdev_release_extent(struct cxl_memdev_state *mds, struct range
> > *range);
> > +const struct cxl_dpa_partition *
> > +cxl_extent_dc_partition(struct cxl_memdev_state *mds,
> > + struct cxl_extent *extent,
> > + struct range *ext_range);
> >
> > static inline struct device *port_to_host(struct cxl_port *port)
> > {
> > diff --git a/drivers/cxl/core/extent.c b/drivers/cxl/core/extent.c
> > index 94128d06f4ed..b01507022cff 100644
> > --- a/drivers/cxl/core/extent.c
> > +++ b/drivers/cxl/core/extent.c
> > @@ -63,11 +63,55 @@ alloc_tag_group(struct cxl_dax_region *cxlr_dax, uuid_t
> > *uuid)
> > return no_free_ptr(group);
> > }
> >
> > +/*
> > + * Find the DC (Dynamic Capacity) partition that fully contains @ext_range,
> > + * or NULL if the extent falls outside every DC partition on this memdev.
> > + * The returned pointer is owned by mds->cxlds.part[] and lives for the
> > + * lifetime of the memdev.
> > + */
> > +const struct cxl_dpa_partition *
> > +cxl_extent_dc_partition(struct cxl_memdev_state *mds,
> > + struct cxl_extent *extent,
> > + struct range *ext_range)
>
> This can be static, given it's only called in extent.c
>
Fixed. Later called in mbox.c so declared non-static in that commit.
> > +{
> > + struct cxl_dev_state *cxlds = &mds->cxlds;
> > + struct device *dev = mds->cxlds.dev;
> > +
> > + for (int i = 0; i < cxlds->nr_partitions; i++) {
> > + struct cxl_dpa_partition *part = &cxlds->part[i];
> > + struct range partition_range = {
> > + .start = part->res.start,
> > + .end = part->res.end,
> > + };
> > +
> > + if (part->mode != CXL_PARTMODE_DYNAMIC_RAM_A)
> > + continue;
> > +
> > + if (range_contains(&partition_range, ext_range)) {
> > + dev_dbg(dev, "DC extent DPA %pra (DCR:%pra)(%pU)\n",
> > + ext_range, &partition_range, extent->uuid);
> > + return part;
> > + }
> > + }
> > +
> > + dev_err_ratelimited(dev,
> > + "DC extent DPA %pra (%pU) is not in a valid DC
> > partition\n",
> > + ext_range, extent->uuid);
> > + return NULL;
> > +}
> > +
> > /*
> > * Stage 1 of the add pipeline: pure, no allocation. Resolve the extent
> > - * to its region/endpoint decoder and ext_range, and verify the range
> > - * fits in the resolved endpoint decoder's DPA resource. Further
> > - * per-extent invariants layer into this function in subsequent commits.
> > + * to its region/endpoint decoder and ext_range, and enforce every
> > + * per-extent invariant the device must satisfy:
> > + *
> > + * - DPA falls inside a Dynamic Capacity partition
> > (cxl_extent_dc_partition).
> > + * - CDAT-sharability rules:
> > + * sharable: tag must be non-null AND shared_extn_seq != 0
> > + * non-sharable: shared_extn_seq must be 0 (tag is optional)
> > + * Any cross-mixing is a device firmware bug.
> > + * - DPA resolves to an endpoint decoder attached to a region.
> > + * - The extent's range is fully contained in that ED's DPA resource.
> > *
> > * Caller must hold cxl_rwsem.region for read (cxl_dpa_to_region()).
> > * On success, @out_cxled / @out_cxlr_dax / @out_ext_range carry the
> > @@ -81,6 +125,10 @@ static int cxl_validate_extent(struct cxl_memdev_state
> > *mds,
> > {
> > u64 start_dpa = le64_to_cpu(extent->start_dpa);
> > struct cxl_memdev *cxlmd = mds->cxlds.cxlmd;
> > + struct device *dev = mds->cxlds.dev;
> > + uuid_t *uuid = (uuid_t *)extent->uuid;
>
> Consider using import_uuid() instead of direct cast.
>
Fixed
> > + u16 seq = le16_to_cpu(extent->shared_extn_seq);
> > + const struct cxl_dpa_partition *part;
> > struct cxl_endpoint_decoder *cxled;
> > struct cxl_region *cxlr;
> > struct range ext_range = (struct range) {
> > @@ -89,6 +137,30 @@ static int cxl_validate_extent(struct cxl_memdev_state
> > *mds,
> > };
> > struct range ed_range;
> >
> > + part = cxl_extent_dc_partition(mds, extent, &ext_range);
> > + if (!part)
> > + return -ENXIO;
> > +
> > + if (part->perf.shareable) {
> > + if (uuid_is_null(uuid)) {
> > + dev_err_ratelimited(dev,
> > + "DC extent DPA %pra: sharable-partition extent
> > has null tag (firmware bug)\n",
> > + &ext_range);
> > + return -ENXIO;
> > + }
> > + if (seq == 0) {
>
> I don't think this complies with the spec language. In r4.0 Table 8-230: "For
> extents describing shareable regions this field shall be within the range of
> 0 to n-1 where n is the number of extents, with each value appearing only
> once." So seq == 0 is an acceptable value.
>
> Also, looking at cxl_add_pending() @ line ~1396, does shared seq number '0'
> holds special meanings? Maybe that needs to change? Also suggest adding
> comments to point out what's happening there if '0' is special.
>
Fixed to enforce 0...n-1 for shared seq num instead of 1...n.
0 no longer used as sentinel val for unshared extents.
'Enforce tag-group semantics', which checks seq num have no
duplicates/gaps, is also fixed
> DJ
>
Thanks,
Anisa
>
> > + dev_err_ratelimited(dev,
> > + "DC extent DPA %pra (%pU): sharable-partition
> > extent missing shared_extn_seq (firmware bug)\n",
> > + &ext_range, uuid);
> > + return -ENXIO;
> > + }
> > + } else if (seq != 0) {
> > + dev_err_ratelimited(dev,
> > + "DC extent DPA %pra (%pU): non-sharable partition but
> > shared_extn_seq=%u (firmware bug)\n",
> > + &ext_range, uuid, seq);
> > + return -ENXIO;
> > + }
> > +
> > cxlr = cxl_dpa_to_region(cxlmd, start_dpa, &cxled);
> > if (!cxlr)
> > return -ENXIO;
>
