On 24/02/26 06:13PM, Alejandro Lucero Palau wrote:
Hi Neeraj,
I could get some free time for reviewing this series. Dan pointed out
to potential conflicts with my Type2 series, so I'm focused on those
patches modifying the cxl region creation and how it is being used.
I do not know if you are aware of the Type2 work, although I dare to
say you are not since some of the functionality is duplicated ... and
in your case skipping some steps.
Hi Alejandro,
Thanks for your review.
Yes I have not looked at Type2 work in detail. I will have a look
and specially the conflicting part with my changes.
Below my comments.
On 1/23/26 11:31, Neeraj Kumar wrote:
devm_cxl_pmem_add_region() is used to create cxl region based on region
information scanned from LSA.
devm_cxl_add_region() is used to just allocate cxlr and its fields are
filled later by userspace tool using device attributes (*_store()).
Inspiration for devm_cxl_pmem_add_region() is taken from these device
attributes (_store*) calls. It allocates cxlr and fills information
parsed from LSA and calls device_add(&cxlr->dev) to initiate further
region creation porbes
Reviewed-by: Dave Jiang <[email protected]>
Signed-off-by: Neeraj Kumar <[email protected]>
---
drivers/cxl/core/region.c | 118 ++++++++++++++++++++++++++++++++++++++
1 file changed, 118 insertions(+)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 2e60e5e72551..e384eacc46ae 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2621,6 +2621,121 @@ static struct cxl_region *devm_cxl_add_region(struct
cxl_root_decoder *cxlrd,
return ERR_PTR(rc);
}
+static ssize_t alloc_region_hpa(struct cxl_region *cxlr, u64 size)
+{
+ int rc;
+
+ if (!size)
+ return -EINVAL;
+
+ ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
+ if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
+ return rc;
+
+ return alloc_hpa(cxlr, size);
+}
Type2 has a more elaborated function preceding the final hpa
allocation. First of all, the root decoder needs to match the
endpoint type or to have support for it. Then interleave ways taken
into account. I know you are only supporting 1 way, what I guess makes
the initial support easier, but although some check for not supporting
1 is fine (I guess this takes a lot of work for matching at least
LSA labels and maybe something harder), the core code should support
that, mainly because there is ongoing work adding it. In my case I did
not need interleaving and it is unlikely other Type2 devices will need
it in the near future, but the support is there:
https://lore.kernel.org/linux-cxl/[email protected]/T/#m56bf70b58bb995082680bf363fd3f6d6f2b074d2
+
+static ssize_t alloc_region_dpa(struct cxl_endpoint_decoder *cxled, u64 size)
+{
+ if (!size)
+ return -EINVAL;
+
+ if (!IS_ALIGNED(size, SZ_256M))
+ return -EINVAL;
+
+ return cxl_dpa_alloc(cxled, size);
+}
I'm not sure if the same applies here as you are passing an endpoint
decoder already, but FWIW:
https://lore.kernel.org/linux-cxl/[email protected]/T/#m38ff266e931fd9c932bc54d000b7ea72186493be
I'm sending type2 individual patches for facilitating the integration,
and I'll focus next on these two referenced ones so you could use them
asap.
Thank you,
Alejandro
Actually I have created alloc_region_hpa() and alloc_region_dpa() taking
inspiration from device attributes (_store*) calls used to create cxl
region using cxl userspace tool.
I am adding the support of multi interleave, there these routines will
not be required as I would be re-using the existing auto region assembly infra.
Even though I will re-check any conflicts with Type2 changes.
Regards,
Neeraj
