Thread (88 messages) 88 messages, 6 authors, 2025-10-07

Re: [PATCH v18 09/20] cxl: Define a driver interface for HPA free space enumeration

From: Alejandro Lucero Palau <hidden>
Date: 2025-09-24 16:54:05
Also in: linux-cxl

On 9/22/25 22:09, Cheatham, Benjamin wrote:
[snip]
quoted
+
+/**
+ * cxl_get_hpa_freespace - find a root decoder with free capacity per constraints
+ * @endpoint: the endpoint requiring the HPA
+ * @interleave_ways: number of entries in @host_bridges
+ * @flags: CXL_DECODER_F flags for selecting RAM vs PMEM, and Type2 device
+ * @max_avail_contig: output parameter of max contiguous bytes available in the
+ *		      returned decoder
+ *
+ * Returns a pointer to a struct cxl_root_decoder
+ *
+ * The return tuple of a 'struct cxl_root_decoder' and 'bytes available given
+ * in (@max_avail_contig))' is a point in time snapshot. If by the time the
+ * caller goes to use this root decoder's capacity the capacity is reduced then
s/capacity the/capacity and the

Or you could cleanup to: "If by the time the caller goes to use this root decoder
and its capacity is reduced then...".

OK.

quoted
+ * caller needs to loop and retry.
+ *
+ * The returned root decoder has an elevated reference count that needs to be
+ * put with cxl_put_root_decoder(cxlrd).
+ */
+struct cxl_root_decoder *cxl_get_hpa_freespace(struct cxl_memdev *cxlmd,
+					       int interleave_ways,
+					       unsigned long flags,
+					       resource_size_t *max_avail_contig)
+{
+	struct cxl_root *root __free(put_cxl_root) = NULL;
+	struct cxl_port *endpoint = cxlmd->endpoint;
+	struct cxlrd_max_context ctx = {
+		.host_bridges = &endpoint->host_bridge,
+		.flags = flags,
+	};
+	struct cxl_port *root_port;
+
+	if (!endpoint) {
+		dev_dbg(&cxlmd->dev, "endpoint not linked to memdev\n");
+		return ERR_PTR(-ENXIO);
+	}
I'm 99% sure you'll crash in this case. The endpoint pointer is dereferenced when assigning to ctx above.

Yes, I need to delay the host_bridges assignment.
quoted
+
+	root  = find_cxl_root(endpoint);
+	if (!root) {
+		dev_dbg(&endpoint->dev, "endpoint can not be related to a root port\n");
s/can not/is not

OK

quoted
+		return ERR_PTR(-ENXIO);
+	}
+
+	root_port = &root->port;
+	scoped_guard(rwsem_read, &cxl_rwsem.region)
+		device_for_each_child(&root_port->dev, &ctx, find_max_hpa);
Do you need the brackets for the scoped guard? I'm not sure how the macro expands when they aren't
there.

I can see other uses like this in core/region.c so I guess they are not 
needed.


Thanks!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help