| Message ID | 20250413-dcd-type2-upstream-v9-16-1d4911a0b365@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | DCD: Add support for Dynamic Capacity Devices (DCD) | expand |
On Sun, 13 Apr 2025 17:52:24 -0500 Ira Weiny <ira.weiny@intel.com> wrote: > Dynamic capacity device extents may be left in an accepted state on a > device due to an unexpected host crash. In this case it is expected > that the creation of a new region on top of a DC partition can read > those extents and surface them for continued use. > > Once all endpoint decoders are part of a region and the region is being > realized, a read of the 'devices extent list' can reveal these > previously accepted extents. > > CXL r3.1 specifies the mailbox call Get Dynamic Capacity Extent List for > this purpose. The call returns all the extents for all dynamic capacity > partitions. If the fabric manager is adding extents to any DCD > partition, the extent list for the recovered region may change. In this > case the query must retry. Upon retry the query could encounter extents > which were accepted on a previous list query. Adding such extents is > ignored without error because they are entirely within a previous > accepted extent. Instead warn on this case to allow for differentiating > bad devices from this normal condition. > > Latch any errors to be bubbled up to ensure notification to the user > even if individual errors are rate limited or otherwise ignored. > > The scan for existing extents races with the dax_cxl driver. This is > synchronized through the region device lock. Extents which are found > after the driver has loaded will surface through the normal notification > path while extents seen prior to the driver are read during driver load. > > Based on an original patch by Navneet Singh. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Reviewed-by: Fan Ni <fan.ni@samsung.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> A couple of minor things noticed on taking another look. > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index de01c6684530..8af3a4173b99 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -1737,6 +1737,115 @@ int cxl_dev_dc_identify(struct cxl_mailbox *mbox, > } > EXPORT_SYMBOL_NS_GPL(cxl_dev_dc_identify, "CXL"); > > +/* Return -EAGAIN if the extent list changes while reading */ > +static int __cxl_process_extent_list(struct cxl_endpoint_decoder *cxled) > +{ > + u32 current_index, total_read, total_expected, initial_gen_num; > + struct cxl_memdev_state *mds = cxled_to_mds(cxled); > + struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox; > + struct device *dev = mds->cxlds.dev; > + struct cxl_mbox_cmd mbox_cmd; > + u32 max_extent_count; > + int latched_rc = 0; > + bool first = true; > + > + struct cxl_mbox_get_extent_out *extents __free(kvfree) = > + kvmalloc(cxl_mbox->payload_size, GFP_KERNEL); > + if (!extents) > + return -ENOMEM; > + > + total_read = 0; > + current_index = 0; > + total_expected = 0; > + max_extent_count = (cxl_mbox->payload_size - sizeof(*extents)) / > + sizeof(struct cxl_extent); > + do { > + u32 nr_returned, current_total, current_gen_num; > + struct cxl_mbox_get_extent_in get_extent; > + int rc; > + > + get_extent = (struct cxl_mbox_get_extent_in) { > + .extent_cnt = cpu_to_le32(max(max_extent_count, > + total_expected - current_index)), > + .start_extent_index = cpu_to_le32(current_index), > + }; > + > + mbox_cmd = (struct cxl_mbox_cmd) { > + .opcode = CXL_MBOX_OP_GET_DC_EXTENT_LIST, > + .payload_in = &get_extent, > + .size_in = sizeof(get_extent), > + .size_out = cxl_mbox->payload_size, > + .payload_out = extents, > + .min_out = 1, Similar to earlier comment (I might well have forgotten how this works) but why not 16 which is what I think we should get even if no extents. > + }; > + > + rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd); > + if (rc < 0) > + return rc; > + > + /* Save initial data */ > + if (first) { > + total_expected = le32_to_cpu(extents->total_extent_count); > + initial_gen_num = le32_to_cpu(extents->generation_num); > + first = false; > + } > + > + nr_returned = le32_to_cpu(extents->returned_extent_count); > + total_read += nr_returned; > + current_total = le32_to_cpu(extents->total_extent_count); > + current_gen_num = le32_to_cpu(extents->generation_num); > + > + dev_dbg(dev, "Got extent list %d-%d of %d generation Num:%d\n", > + current_index, total_read - 1, current_total, current_gen_num); > + > + if (current_gen_num != initial_gen_num || total_expected != current_total) { > + dev_warn(dev, "Extent list change detected; gen %u != %u : cnt %u != %u\n", > + current_gen_num, initial_gen_num, > + total_expected, current_total); > + return -EAGAIN; > + } > + > + for (int i = 0; i < nr_returned ; i++) { > + struct cxl_extent *extent = &extents->extent[i]; > + > + dev_dbg(dev, "Processing extent %d/%d\n", > + current_index + i, total_expected); > + > + rc = validate_add_extent(mds, extent); > + if (rc) > + latched_rc = rc; > + } > + > + current_index += nr_returned; > + } while (total_expected > total_read); > + > + return latched_rc; > +} > +/* > + * Get Dynamic Capacity Extent List; Output Payload > + * CXL rev 3.1 section 8.2.9.9.9.2; Table 8-167 > + */ > +struct cxl_mbox_get_extent_out { > + __le32 returned_extent_count; > + __le32 total_extent_count; > + __le32 generation_num; > + u8 rsvd[4]; > + struct cxl_extent extent[]; Throw some counted_by magic at this? > +} __packed; > + > struct cxl_mbox_get_supported_logs { > __le16 entries; > u8 rsvd[6]; >
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h index 027dd1504d77..e06a46fec217 100644 --- a/drivers/cxl/core/core.h +++ b/drivers/cxl/core/core.h @@ -22,6 +22,7 @@ cxled_to_mds(struct cxl_endpoint_decoder *cxled) return container_of(cxlds, struct cxl_memdev_state, cxlds); } +int cxl_process_extent_list(struct cxl_endpoint_decoder *cxled); int cxl_region_invalidate_memregion(struct cxl_region *cxlr); #ifdef CONFIG_CXL_REGION diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index de01c6684530..8af3a4173b99 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -1737,6 +1737,115 @@ int cxl_dev_dc_identify(struct cxl_mailbox *mbox, } EXPORT_SYMBOL_NS_GPL(cxl_dev_dc_identify, "CXL"); +/* Return -EAGAIN if the extent list changes while reading */ +static int __cxl_process_extent_list(struct cxl_endpoint_decoder *cxled) +{ + u32 current_index, total_read, total_expected, initial_gen_num; + struct cxl_memdev_state *mds = cxled_to_mds(cxled); + struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox; + struct device *dev = mds->cxlds.dev; + struct cxl_mbox_cmd mbox_cmd; + u32 max_extent_count; + int latched_rc = 0; + bool first = true; + + struct cxl_mbox_get_extent_out *extents __free(kvfree) = + kvmalloc(cxl_mbox->payload_size, GFP_KERNEL); + if (!extents) + return -ENOMEM; + + total_read = 0; + current_index = 0; + total_expected = 0; + max_extent_count = (cxl_mbox->payload_size - sizeof(*extents)) / + sizeof(struct cxl_extent); + do { + u32 nr_returned, current_total, current_gen_num; + struct cxl_mbox_get_extent_in get_extent; + int rc; + + get_extent = (struct cxl_mbox_get_extent_in) { + .extent_cnt = cpu_to_le32(max(max_extent_count, + total_expected - current_index)), + .start_extent_index = cpu_to_le32(current_index), + }; + + mbox_cmd = (struct cxl_mbox_cmd) { + .opcode = CXL_MBOX_OP_GET_DC_EXTENT_LIST, + .payload_in = &get_extent, + .size_in = sizeof(get_extent), + .size_out = cxl_mbox->payload_size, + .payload_out = extents, + .min_out = 1, + }; + + rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd); + if (rc < 0) + return rc; + + /* Save initial data */ + if (first) { + total_expected = le32_to_cpu(extents->total_extent_count); + initial_gen_num = le32_to_cpu(extents->generation_num); + first = false; + } + + nr_returned = le32_to_cpu(extents->returned_extent_count); + total_read += nr_returned; + current_total = le32_to_cpu(extents->total_extent_count); + current_gen_num = le32_to_cpu(extents->generation_num); + + dev_dbg(dev, "Got extent list %d-%d of %d generation Num:%d\n", + current_index, total_read - 1, current_total, current_gen_num); + + if (current_gen_num != initial_gen_num || total_expected != current_total) { + dev_warn(dev, "Extent list change detected; gen %u != %u : cnt %u != %u\n", + current_gen_num, initial_gen_num, + total_expected, current_total); + return -EAGAIN; + } + + for (int i = 0; i < nr_returned ; i++) { + struct cxl_extent *extent = &extents->extent[i]; + + dev_dbg(dev, "Processing extent %d/%d\n", + current_index + i, total_expected); + + rc = validate_add_extent(mds, extent); + if (rc) + latched_rc = rc; + } + + current_index += nr_returned; + } while (total_expected > total_read); + + return latched_rc; +} + +#define CXL_READ_EXTENT_LIST_RETRY 10 + +/** + * cxl_process_extent_list() - Read existing extents + * @cxled: Endpoint decoder which is part of a region + * + * Issue the Get Dynamic Capacity Extent List command to the device + * and add existing extents if found. + * + * A retry of 10 is somewhat arbitrary, however, extent changes should be + * relatively rare while bringing up a region. So 10 should be plenty. + */ +int cxl_process_extent_list(struct cxl_endpoint_decoder *cxled) +{ + int retry = CXL_READ_EXTENT_LIST_RETRY; + int rc; + + do { + rc = __cxl_process_extent_list(cxled); + } while (rc == -EAGAIN && retry--); + + return rc; +} + static void add_part(struct cxl_dpa_info *info, u64 start, u64 size, enum cxl_partition_mode mode) { int i = info->nr_partitions; diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index eeabc5a6b18a..a43b43972bae 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -3196,6 +3196,26 @@ static int devm_cxl_add_pmem_region(struct cxl_region *cxlr) return rc; } +static int cxlr_add_existing_extents(struct cxl_region *cxlr) +{ + struct cxl_region_params *p = &cxlr->params; + int i, latched_rc = 0; + + for (i = 0; i < p->nr_targets; i++) { + struct device *dev = &p->targets[i]->cxld.dev; + int rc; + + rc = cxl_process_extent_list(p->targets[i]); + if (rc) { + dev_err(dev, "Existing extent processing failed %d\n", + rc); + latched_rc = rc; + } + } + + return latched_rc; +} + static void cxlr_dax_unregister(void *_cxlr_dax) { struct cxl_dax_region *cxlr_dax = _cxlr_dax; @@ -3231,6 +3251,11 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr) dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent), dev_name(dev)); + if (cxlr->mode == CXL_PARTMODE_DYNAMIC_RAM_A) + if (cxlr_add_existing_extents(cxlr)) + dev_err(&cxlr->dev, "Existing extent processing failed %d\n", + rc); + return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister, cxlr_dax); err: diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 63a38e449454..f80f70549c0b 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -600,6 +600,27 @@ struct cxl_mbox_dc_response { } __packed extent_list[]; } __packed; +/* + * Get Dynamic Capacity Extent List; Input Payload + * CXL rev 3.1 section 8.2.9.9.9.2; Table 8-166 + */ +struct cxl_mbox_get_extent_in { + __le32 extent_cnt; + __le32 start_extent_index; +} __packed; + +/* + * Get Dynamic Capacity Extent List; Output Payload + * CXL rev 3.1 section 8.2.9.9.9.2; Table 8-167 + */ +struct cxl_mbox_get_extent_out { + __le32 returned_extent_count; + __le32 total_extent_count; + __le32 generation_num; + u8 rsvd[4]; + struct cxl_extent extent[]; +} __packed; + struct cxl_mbox_get_supported_logs { __le16 entries; u8 rsvd[6];