Re: [PATCH v9 17/27] cxl: define a driver interface for DPA allocation
flat view
From: Jonathan Cameron <hidden>
Date: 2025-01-02 15:16:00
Also in:
linux-cxl
On Mon, 30 Dec 2024 21:44:35 +0000 [off-list ref] wrote:
From: Alejandro Lucero <redacted> Region creation involves finding available DPA (device-physical-address) capacity to map into HPA (host-physical-address) space. Given the HPA capacity constraint, define an API, cxl_request_dpa(), that has the flexibility to map the minimum amount of memory the driver needs to operate vs the total possible that can be mapped given HPA availability. Factor out the core of cxl_dpa_alloc, that does free space scanning, into a cxl_dpa_freespace() helper, and use that to balance the capacity available to map vs the @min and @max arguments to cxl_request_dpa. Based on https://lore.kernel.org/linux-cxl/168592158743.1948938.7622563891193802610.stgit@dwillia2-xfh.jf.intel.com/ (local) Signed-off-by: Alejandro Lucero <redacted> Co-developed-by: Dan Williams <redacted>
A couple of really minor things inline. Either way Reviewed-by: Jonathan Cameron <redacted>
quoted hunk ↗ jump to hunk
+/** + * cxl_request_dpa - search and reserve DPA given input constraints + * @cxlmd: memdev with an endpoint port with available decoders + * @is_ram: DPA operation mode (ram vs pmem) + * @min: the minimum amount of capacity the call needs + * @max: HPA capacity available + * + * Given that a region needs to allocate from limited HPA capacity it + * may be the case that a device has more mappable DPA capacity than + * available HPA. So, the expectation is that @min is a driver known + * value for how much capacity is needed, and @max is based the limit of
is the limit? Not sure what the "is based" means
quoted hunk ↗ jump to hunk
+ * how much HPA space is available for a new region. + * + * Returns a pinned cxl_decoder with at least @min bytes of capacity + * reserved, or an error pointer. The caller is also expected to own the + * lifetime of the memdev registration associated with the endpoint to + * pin the decoder registered as well. + */ +struct cxl_endpoint_decoder *cxl_request_dpa(struct cxl_memdev *cxlmd, + bool is_ram, + resource_size_t min, + resource_size_t max) +{ + struct cxl_port *endpoint = cxlmd->endpoint; + struct cxl_endpoint_decoder *cxled; + enum cxl_decoder_mode mode; + struct device *cxled_dev; + resource_size_t alloc; + int rc; + + if (!IS_ALIGNED(min | max, SZ_256M)) + return ERR_PTR(-EINVAL); + + down_read(&cxl_dpa_rwsem); + cxled_dev = device_find_child(&endpoint->dev, NULL, find_free_decoder); + up_read(&cxl_dpa_rwsem); + + if (!cxled_dev) + return ERR_PTR(-ENXIO); + + cxled = to_cxl_endpoint_decoder(cxled_dev); + + if (!cxled) { + put_device(cxled_dev); + return ERR_PTR(-ENODEV);
Ah. My suggestion on v8 missed that there is an error block below. More consistent with rest of the code as rc = -ENODEV; goto err;
quoted hunk ↗ jump to hunk
+ } + + if (is_ram) + mode = CXL_DECODER_RAM; + else + mode = CXL_DECODER_PMEM; + + rc = cxl_dpa_set_mode(cxled, mode); + if (rc) + goto err; + + down_read(&cxl_dpa_rwsem); + alloc = cxl_dpa_freespace(cxled, NULL, NULL); + up_read(&cxl_dpa_rwsem); + + if (max) + alloc = min(max, alloc); + if (alloc < min) { + rc = -ENOMEM; + goto err; + } + + rc = cxl_dpa_alloc(cxled, alloc); + if (rc) + goto err; + + return cxled; +err: + put_device(cxled_dev); + return ERR_PTR(rc); +} +EXPORT_SYMBOL_NS_GPL(cxl_request_dpa, "CXL");