Re: [PATCH v5 2/3] cxl/core/region: move dax region device logic into region_dax.c

[Dave Jiang]

On 4/10/26 5:02 PM, Anisa Su wrote:
> On Thu, Mar 26, 2026 at 10:02:02PM -0400, Gregory Price wrote:
>> core/region.c is overloaded with per-region control logic (pmem, dax,
>> sysram, etc). Move the CXL DAX region device infrastructure from
>> region.c into a new region_dax.c file.
>>
>> This will also allow us to add additional dax-driver integration paths
>> that don't further dirty the core region.c logic.
>>
>> No functional changes.
>>
>> Signed-off-by: Gregory Price <gourry@xxxxxxxxxx>
>> Co-developed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
>> Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
>> ---
>>  drivers/cxl/core/Makefile     |   2 +-
>>  drivers/cxl/core/core.h       |   1 +
>>  drivers/cxl/core/region.c     |  99 ------------------------------
>>  drivers/cxl/core/region_dax.c | 109 ++++++++++++++++++++++++++++++++++
>>  tools/testing/cxl/Kbuild      |   2 +-
>>  5 files changed, 112 insertions(+), 101 deletions(-)
>>  create mode 100644 drivers/cxl/core/region_dax.c
>>
>> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
>> index f73776fe323b..ce7213818d3c 100644
>> --- a/drivers/cxl/core/Makefile
>> +++ b/drivers/cxl/core/Makefile
>> @@ -15,7 +15,7 @@ cxl_core-y += hdm.o
>>  cxl_core-y += pmu.o
>>  cxl_core-y += cdat.o
>>  cxl_core-$(CONFIG_TRACING) += trace.o
>> -cxl_core-$(CONFIG_CXL_REGION) += region.o region_pmem.o
>> +cxl_core-$(CONFIG_CXL_REGION) += region.o region_pmem.o region_dax.o
>>  cxl_core-$(CONFIG_CXL_MCE) += mce.o
>>  cxl_core-$(CONFIG_CXL_FEATURES) += features.o
>>  cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
>> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
>> index ef03966eeabd..82ca3a476708 100644
>> --- a/drivers/cxl/core/core.h
>> +++ b/drivers/cxl/core/core.h
>> @@ -50,6 +50,7 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
>>  struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
>>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>>  		   u64 dpa);
>> +int devm_cxl_add_dax_region(struct cxl_region *cxlr);
>>  int devm_cxl_add_pmem_region(struct cxl_region *cxlr);
>>  
>>  #else
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 72aee1408efb..6a2ef3475764 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -3436,105 +3436,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>>  	return -ENXIO;
>>  }
>>  
>> -static void cxl_dax_region_release(struct device *dev)
>> -{
>> -	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
>> -
>> -	kfree(cxlr_dax);
>> -}
>> -
>> -static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
>> -	&cxl_base_attribute_group,
>> -	NULL,
>> -};
>> -
>> -const struct device_type cxl_dax_region_type = {
>> -	.name = "cxl_dax_region",
>> -	.release = cxl_dax_region_release,
>> -	.groups = cxl_dax_region_attribute_groups,
>> -};
>> -
>> -static bool is_cxl_dax_region(struct device *dev)
>> -{
>> -	return dev->type == &cxl_dax_region_type;
>> -}
>> -
>> -struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
>> -{
>> -	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
>> -			  "not a cxl_dax_region device\n"))
>> -		return NULL;
>> -	return container_of(dev, struct cxl_dax_region, dev);
>> -}
>> -EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
>> -
>> -static struct lock_class_key cxl_dax_region_key;
>> -
>> -static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>> -{
>> -	struct cxl_region_params *p = &cxlr->params;
>> -	struct cxl_dax_region *cxlr_dax;
>> -	struct device *dev;
>> -
>> -	guard(rwsem_read)(&cxl_rwsem.region);
>> -	if (p->state != CXL_CONFIG_COMMIT)
>> -		return ERR_PTR(-ENXIO);
>> -
>> -	cxlr_dax = kzalloc_obj(*cxlr_dax);
>> -	if (!cxlr_dax)
>> -		return ERR_PTR(-ENOMEM);
>> -
>> -	cxlr_dax->hpa_range.start = p->res->start;
>> -	cxlr_dax->hpa_range.end = p->res->end;
>> -
>> -	dev = &cxlr_dax->dev;
>> -	cxlr_dax->cxlr = cxlr;
>> -	device_initialize(dev);
>> -	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
>> -	device_set_pm_not_required(dev);
>> -	dev->parent = &cxlr->dev;
>> -	dev->bus = &cxl_bus_type;
>> -	dev->type = &cxl_dax_region_type;
>> -
>> -	return cxlr_dax;
>> -}
>> -
>> -static void cxlr_dax_unregister(void *_cxlr_dax)
>> -{
>> -	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
>> -
>> -	device_unregister(&cxlr_dax->dev);
>> -}
>> -
>> -static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
>> -{
>> -	struct cxl_dax_region *cxlr_dax;
>> -	struct device *dev;
>> -	int rc;
>> -
>> -	cxlr_dax = cxl_dax_region_alloc(cxlr);
>> -	if (IS_ERR(cxlr_dax))
>> -		return PTR_ERR(cxlr_dax);
>> -
>> -	dev = &cxlr_dax->dev;
>> -	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
>> -	if (rc)
>> -		goto err;
>> -
>> -	rc = device_add(dev);
>> -	if (rc)
>> -		goto err;
>> -
>> -	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
>> -		dev_name(dev));
>> -
>> -	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
>> -					cxlr_dax);
>> -err:
>> -	put_device(dev);
>> -	return rc;
>> -}
>> -
>>  static int match_root_decoder(struct device *dev, const void *data)
>>  {
>>  	const struct range *r1, *r2 = data;
>> diff --git a/drivers/cxl/core/region_dax.c b/drivers/cxl/core/region_dax.c
>> new file mode 100644
>> index 000000000000..fe367759ac69
>> --- /dev/null
>> +++ b/drivers/cxl/core/region_dax.c
>> @@ -0,0 +1,109 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright(c) 2022 Intel Corporation. All rights reserved.
>> + * Copyright(c) 2026 Meta Technologies Inc. All rights reserved.
>> + */
>> +#include <linux/device.h>
>> +#include <linux/slab.h>
>> +#include <cxlmem.h>
>> +#include <cxl.h>
>> +#include "core.h"
>> +
>> +static void cxl_dax_region_release(struct device *dev)
>> +{
>> +	struct cxl_dax_region *cxlr_dax = to_cxl_dax_region(dev);
>> +
>> +	kfree(cxlr_dax);
>> +}
>> +
>> +static const struct attribute_group *cxl_dax_region_attribute_groups[] = {
>> +	&cxl_base_attribute_group,
>> +	NULL
>> +};
>> +
>> +const struct device_type cxl_dax_region_type = {
>> +	.name = "cxl_dax_region",
>> +	.release = cxl_dax_region_release,
>> +	.groups = cxl_dax_region_attribute_groups,
>> +};
>> +
>> +static bool is_cxl_dax_region(struct device *dev)
>> +{
>> +	return dev->type == &cxl_dax_region_type;
>> +}
>> +
>> +struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
>> +{
>> +	if (dev_WARN_ONCE(dev, !is_cxl_dax_region(dev),
>> +			  "not a cxl_dax_region device\n"))
>> +		return NULL;
>> +	return container_of(dev, struct cxl_dax_region, dev);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(to_cxl_dax_region, "CXL");
>> +
>> +static struct lock_class_key cxl_dax_region_key;
>> +
>> +static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>> +{
>> +	struct cxl_region_params *p = &cxlr->params;
>> +	struct cxl_dax_region *cxlr_dax;
>> +	struct device *dev;
>> +
>> +	guard(rwsem_read)(&cxl_rwsem.region);
>> +	if (p->state != CXL_CONFIG_COMMIT)
>> +		return ERR_PTR(-ENXIO);
>> +
>> +	cxlr_dax = kzalloc_obj(*cxlr_dax);
>> +	if (!cxlr_dax)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	cxlr_dax->hpa_range.start = p->res->start;
>> +	cxlr_dax->hpa_range.end = p->res->end;
>> +
>> +	dev = &cxlr_dax->dev;
>> +	cxlr_dax->cxlr = cxlr;
> 	cxlr->cxlr_dax = cxlr_dax;
> 
> Running into segfaults without this ^

Can you pls create a fixes patch?

DJ

>> +	device_initialize(dev);
>> +	lockdep_set_class(&dev->mutex, &cxl_dax_region_key);
>> +	device_set_pm_not_required(dev);
>> +	dev->parent = &cxlr->dev;
>> +	dev->bus = &cxl_bus_type;
>> +	dev->type = &cxl_dax_region_type;
>> +
>> +	return cxlr_dax;
>> +}
>> +
>> +static void cxlr_dax_unregister(void *_cxlr_dax)
>> +{
>> +	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
>> +
>> +	device_unregister(&cxlr_dax->dev);
>> +}
>> +
>> +int devm_cxl_add_dax_region(struct cxl_region *cxlr)
>> +{
>> +	struct cxl_dax_region *cxlr_dax;
>> +	struct device *dev;
>> +	int rc;
>> +
>> +	cxlr_dax = cxl_dax_region_alloc(cxlr);
>> +	if (IS_ERR(cxlr_dax))
>> +		return PTR_ERR(cxlr_dax);
>> +
>> +	dev = &cxlr_dax->dev;
>> +	rc = dev_set_name(dev, "dax_region%d", cxlr->id);
>> +	if (rc)
>> +		goto err;
>> +
>> +	rc = device_add(dev);
>> +	if (rc)
>> +		goto err;
>> +
>> +	dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
>> +		dev_name(dev));
>> +
>> +	return devm_add_action_or_reset(&cxlr->dev, cxlr_dax_unregister,
>> +					cxlr_dax);
>> +err:
>> +	put_device(dev);
>> +	return rc;
>> +}
>> diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
>> index f53d79a05661..d2b291e5f842 100644
>> --- a/tools/testing/cxl/Kbuild
>> +++ b/tools/testing/cxl/Kbuild
>> @@ -59,7 +59,7 @@ cxl_core-y += $(CXL_CORE_SRC)/hdm.o
>>  cxl_core-y += $(CXL_CORE_SRC)/pmu.o
>>  cxl_core-y += $(CXL_CORE_SRC)/cdat.o
>>  cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
>> -cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o $(CXL_CORE_SRC)/region_pmem.o
>> +cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o $(CXL_CORE_SRC)/region_pmem.o $(CXL_CORE_SRC)/region_dax.o
>>  cxl_core-$(CONFIG_CXL_MCE) += $(CXL_CORE_SRC)/mce.o
>>  cxl_core-$(CONFIG_CXL_FEATURES) += $(CXL_CORE_SRC)/features.o
>>  cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += $(CXL_CORE_SRC)/edac.o
>> -- 
>> 2.53.0
>>