Message-ID: <5b4988a0-90e2-4f85-83bd-bf54b3f69a12@amd.com>
Date: Tue, 27 Jan 2026 13:45:50 -0800
From: "Koralahalli Channabasappa, Smita" <skoralah@....com>
To: Alison Schofield <alison.schofield@...el.com>
Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>,
 linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
 nvdimm@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
 linux-pm@...r.kernel.org, Ard Biesheuvel <ardb@...nel.org>,
 Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
 Dan Williams <dan.j.williams@...el.com>,
 Jonathan Cameron <jonathan.cameron@...wei.com>,
 Yazen Ghannam <yazen.ghannam@....com>, Dave Jiang <dave.jiang@...el.com>,
 Davidlohr Bueso <dave@...olabs.net>, Matthew Wilcox <willy@...radead.org>,
 Jan Kara <jack@...e.cz>, "Rafael J . Wysocki" <rafael@...nel.org>,
 Len Brown <len.brown@...el.com>, Pavel Machek <pavel@...nel.org>,
 Li Ming <ming.li@...omail.com>, Jeff Johnson
 <jeff.johnson@....qualcomm.com>, Ying Huang <huang.ying.caritas@...il.com>,
 Yao Xingtao <yaoxt.fnst@...itsu.com>, Peter Zijlstra <peterz@...radead.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Nathan Fontenot <nathan.fontenot@....com>,
 Terry Bowman <terry.bowman@....com>, Robert Richter <rrichter@....com>,
 Benjamin Cheatham <benjamin.cheatham@....com>,
 Zhijian Li <lizhijian@...itsu.com>, Borislav Petkov <bp@...en8.de>,
 Tomasz Wolski <tomasz.wolski@...itsu.com>
Subject: Re: [PATCH v5 6/7] dax/hmem, cxl: Defer and resolve ownership of Soft
 Reserved memory ranges

Hi Alison,

On 1/26/2026 2:33 PM, Alison Schofield wrote:
> On Mon, Jan 26, 2026 at 01:05:47PM -0800, Koralahalli Channabasappa, Smita wrote:
>> Hi Alison,
>>
>> On 1/22/2026 10:35 PM, Alison Schofield wrote:
>>> On Thu, Jan 22, 2026 at 04:55:42AM +0000, Smita Koralahalli wrote:
>>>> The current probe time ownership check for Soft Reserved memory based
>>>> solely on CXL window intersection is insufficient. dax_hmem probing is not
>>>> always guaranteed to run after CXL enumeration and region assembly, which
>>>> can lead to incorrect ownership decisions before the CXL stack has
>>>> finished publishing windows and assembling committed regions.
>>>>
>>>> Introduce deferred ownership handling for Soft Reserved ranges that
>>>> intersect CXL windows at probe time by scheduling deferred work from
>>>> dax_hmem and waiting for the CXL stack to complete enumeration and region
>>>> assembly before deciding ownership.
>>>>
>>>> Evaluate ownership of Soft Reserved ranges based on CXL region
>>>> containment.
>>>>
>>>>      - If all Soft Reserved ranges are fully contained within committed CXL
>>>>        regions, DROP handling Soft Reserved ranges from dax_hmem and allow
>>>>        dax_cxl to bind.
>>>>
>>>>      - If any Soft Reserved range is not fully claimed by committed CXL
>>>>        region, tear down all CXL regions and REGISTER the Soft Reserved
>>>>        ranges with dax_hmem instead.
>>>>
>>>> While ownership resolution is pending, gate dax_cxl probing to avoid
>>>> binding prematurely.
>>>
>>> This patch is the point in the set where I begin to fail creating DAX
>>> regions on my non soft-reserved platforms.
>>>
>>> Before this patch, at region probe, devm_cxl_add_dax_region(cxlr) succeeded
>>> without delay, but now those calls result in EPROBE DEFER.
>>>
>>> That deferral is wanted for platforms with Soft Reserveds, but for
>>> platforms without, those probes will never resume.
>>>
>>> IIUC this will impact platforms without SRs, not just my test setup.
>>> In my testing it's visible during both QEMU and cxl-test region creation.
>>>
>>> Can we abandon this whole deferral scheme if there is nothing in the
>>> new soft_reserved resource tree?
>>>
>>> Or maybe another way to get the dax probes UN-deferred in this case?
>>
>> Thanks for pointing this. I didn't think through this.
>>
>> I was thinking to make the deferral conditional on HMEM actually observing a
>> CXL-overlapping range. Rough flow:
>>
>> One assumption I'm relying on here is that dax_hmem and "initial"
>> hmem_register_device() walk happens before dax_cxl probes. If that
>> assumption doesn’t hold this approach may not be sufficient.
>>
>> 1. Keep dax_cxl_mode default as DEFER as it is now in dax/bus.c
>> 2. Introduce need_deferral flag initialized to false in dax/bus.c
>> 3. During the initial dax_hmem walk, in hmem_register_device() if HMEM
>> observes SR that intersects IORES_DESC_CXL, set a need_deferral flag and
>> schedule the deferred work. (case DEFER)
>> 4. In dax_cxl probe: only return -EPROBE_DEFER when dax_cxl_mode == DEFER
>> and need_deferral is set, otherwise proceed with cxl_dax.
>>
>> Please call out if you see issues with this approach (especially around the
>> ordering assumption).
> 
> 
> A quick thought to share -
> 
> Will the 'need_deferral' flag be cleared when all deferred work is
> done, so that case 2) below can succeed:

My thinking was that we don’t strictly need to clear need_deferral as 
long as dax_cxl_mode is the actual gate. need_deferral would only be set 
when HMEM observes an SR range intersecting IORES_DESC_CXL, and after 
the deferred work runs we should always transition dax_cxl_mode from 
DEFER to either DROP or REGISTER. At that point dax_cxl won’t return 
EPROBE_DEFER anymore regardless of the flag value.

I also had a follow-up thought: rather than a separate need_deferral 
flag, we could make this explicit in the mode enum. For example, keep 
DEFER as the default, and when hmem_register_device() first observes a 
SR and CXL intersection, transition the mode from DEFER to something 
like NEEDS_CHANGE. Then dax_cxl would only return -EPROBE_DEFER in the 
NEEDS_CHANGE state, and once the deferred work completes it would move 
the mode to DROP or REGISTER.

Please correct me if I’m missing a case where dax_cxl_mode could remain 
DEFER even after setting the flag.

Thanks
Smita

> 
> While these changes add sync and fallback for platforms that use Soft
> Reserveds, protect against regressing other use cases like:
> 
> 1) Platforms that don't create SRs but do create auto regions and
> expect them to either automatically create dax regions on successful CXL
> driver assembly.
> 
> 2) Plain old user space creation of ram regions where the user expects
> the result to be a CXL region and a DAX region. These may occur in
> platforms with or without Soft Reserveds.
> 
>>
>> Thanks
>> Smita
>>>
>>> -- Alison
>>>
>>>>
>>>> This enforces a strict ownership. Either CXL fully claims the Soft
>>>> Reserved ranges or it relinquishes it entirely.
>>>>
>>>> Co-developed-by: Dan Williams <dan.j.williams@...el.com>
>>>> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
>>>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
>>>> ---
>>>>    drivers/cxl/core/region.c | 25 ++++++++++++
>>>>    drivers/cxl/cxl.h         |  2 +
>>>>    drivers/dax/cxl.c         |  9 +++++
>>>>    drivers/dax/hmem/hmem.c   | 81 ++++++++++++++++++++++++++++++++++++++-
>>>>    4 files changed, 115 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>>>> index 9827a6dd3187..6c22a2d4abbb 100644
>>>> --- a/drivers/cxl/core/region.c
>>>> +++ b/drivers/cxl/core/region.c
>>>> @@ -3875,6 +3875,31 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
>>>>    DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
>>>>    			 cxl_region_debugfs_poison_clear, "%llx\n");
>>>> +static int cxl_region_teardown_cb(struct device *dev, void *data)
>>>> +{
>>>> +	struct cxl_root_decoder *cxlrd;
>>>> +	struct cxl_region *cxlr;
>>>> +	struct cxl_port *port;
>>>> +
>>>> +	if (!is_cxl_region(dev))
>>>> +		return 0;
>>>> +
>>>> +	cxlr = to_cxl_region(dev);
>>>> +
>>>> +	cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
>>>> +	port = cxlrd_to_port(cxlrd);
>>>> +
>>>> +	devm_release_action(port->uport_dev, unregister_region, cxlr);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +void cxl_region_teardown_all(void)
>>>> +{
>>>> +	bus_for_each_dev(&cxl_bus_type, NULL, NULL, cxl_region_teardown_cb);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cxl_region_teardown_all);
>>>> +
>>>>    static int cxl_region_contains_sr_cb(struct device *dev, void *data)
>>>>    {
>>>>    	struct resource *res = data;
>>>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>>>> index b0ff6b65ea0b..1864d35d5f69 100644
>>>> --- a/drivers/cxl/cxl.h
>>>> +++ b/drivers/cxl/cxl.h
>>>> @@ -907,6 +907,7 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled);
>>>>    struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
>>>>    u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
>>>>    bool cxl_region_contains_soft_reserve(const struct resource *res);
>>>> +void cxl_region_teardown_all(void);
>>>>    #else
>>>>    static inline bool is_cxl_pmem_region(struct device *dev)
>>>>    {
>>>> @@ -933,6 +934,7 @@ static inline bool cxl_region_contains_soft_reserve(const struct resource *res)
>>>>    {
>>>>    	return false;
>>>>    }
>>>> +static inline void cxl_region_teardown_all(void) { }
>>>>    #endif
>>>>    void cxl_endpoint_parse_cdat(struct cxl_port *port);
>>>> diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
>>>> index 13cd94d32ff7..b7e90d6dd888 100644
>>>> --- a/drivers/dax/cxl.c
>>>> +++ b/drivers/dax/cxl.c
>>>> @@ -14,6 +14,15 @@ static int cxl_dax_region_probe(struct device *dev)
>>>>    	struct dax_region *dax_region;
>>>>    	struct dev_dax_data data;
>>>> +	switch (dax_cxl_mode) {
>>>> +	case DAX_CXL_MODE_DEFER:
>>>> +		return -EPROBE_DEFER;
>>>> +	case DAX_CXL_MODE_REGISTER:
>>>> +		return -ENODEV;
>>>> +	case DAX_CXL_MODE_DROP:
>>>> +		break;
>>>> +	}
>>>> +
>>>>    	if (nid == NUMA_NO_NODE)
>>>>    		nid = memory_add_physaddr_to_nid(cxlr_dax->hpa_range.start);
>>>> diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
>>>> index 1e3424358490..bcb57d8678d7 100644
>>>> --- a/drivers/dax/hmem/hmem.c
>>>> +++ b/drivers/dax/hmem/hmem.c
>>>> @@ -3,6 +3,7 @@
>>>>    #include <linux/memregion.h>
>>>>    #include <linux/module.h>
>>>>    #include <linux/dax.h>
>>>> +#include "../../cxl/cxl.h"
>>>>    #include "../bus.h"
>>>>    static bool region_idle;
>>>> @@ -58,9 +59,15 @@ static void release_hmem(void *pdev)
>>>>    	platform_device_unregister(pdev);
>>>>    }
>>>> +struct dax_defer_work {
>>>> +	struct platform_device *pdev;
>>>> +	struct work_struct work;
>>>> +};
>>>> +
>>>>    static int hmem_register_device(struct device *host, int target_nid,
>>>>    				const struct resource *res)
>>>>    {
>>>> +	struct dax_defer_work *work = dev_get_drvdata(host);
>>>>    	struct platform_device *pdev;
>>>>    	struct memregion_info info;
>>>>    	long id;
>>>> @@ -69,8 +76,18 @@ static int hmem_register_device(struct device *host, int target_nid,
>>>>    	if (IS_ENABLED(CONFIG_DEV_DAX_CXL) &&
>>>>    	    region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
>>>>    			      IORES_DESC_CXL) != REGION_DISJOINT) {
>>>> -		dev_dbg(host, "deferring range to CXL: %pr\n", res);
>>>> -		return 0;
>>>> +		switch (dax_cxl_mode) {
>>>> +		case DAX_CXL_MODE_DEFER:
>>>> +			dev_dbg(host, "deferring range to CXL: %pr\n", res);
>>>> +			schedule_work(&work->work);
>>>> +			return 0;
>>>> +		case DAX_CXL_MODE_REGISTER:
>>>> +			dev_dbg(host, "registering CXL range: %pr\n", res);
>>>> +			break;
>>>> +		case DAX_CXL_MODE_DROP:
>>>> +			dev_dbg(host, "dropping CXL range: %pr\n", res);
>>>> +			return 0;
>>>> +		}
>>>>    	}
>>>>    	rc = region_intersects_soft_reserve(res->start, resource_size(res));
>>>> @@ -123,8 +140,67 @@ static int hmem_register_device(struct device *host, int target_nid,
>>>>    	return rc;
>>>>    }
>>>> +static int cxl_contains_soft_reserve(struct device *host, int target_nid,
>>>> +				     const struct resource *res)
>>>> +{
>>>> +	if (region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
>>>> +			      IORES_DESC_CXL) != REGION_DISJOINT) {
>>>> +		if (!cxl_region_contains_soft_reserve(res))
>>>> +			return 1;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static void process_defer_work(struct work_struct *_work)
>>>> +{
>>>> +	struct dax_defer_work *work = container_of(_work, typeof(*work), work);
>>>> +	struct platform_device *pdev = work->pdev;
>>>> +	int rc;
>>>> +
>>>> +	/* relies on cxl_acpi and cxl_pci having had a chance to load */
>>>> +	wait_for_device_probe();
>>>> +
>>>> +	rc = walk_hmem_resources(&pdev->dev, cxl_contains_soft_reserve);
>>>> +
>>>> +	if (!rc) {
>>>> +		dax_cxl_mode = DAX_CXL_MODE_DROP;
>>>> +		rc = bus_rescan_devices(&cxl_bus_type);
>>>> +		if (rc)
>>>> +			dev_warn(&pdev->dev, "CXL bus rescan failed: %d\n", rc);
>>>> +	} else {
>>>> +		dax_cxl_mode = DAX_CXL_MODE_REGISTER;
>>>> +		cxl_region_teardown_all();
>>>> +	}
>>>> +
>>>> +	walk_hmem_resources(&pdev->dev, hmem_register_device);
>>>> +}
>>>> +
>>>> +static void kill_defer_work(void *_work)
>>>> +{
>>>> +	struct dax_defer_work *work = container_of(_work, typeof(*work), work);
>>>> +
>>>> +	cancel_work_sync(&work->work);
>>>> +	kfree(work);
>>>> +}
>>>> +
>>>>    static int dax_hmem_platform_probe(struct platform_device *pdev)
>>>>    {
>>>> +	struct dax_defer_work *work = kzalloc(sizeof(*work), GFP_KERNEL);
>>>> +	int rc;
>>>> +
>>>> +	if (!work)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	work->pdev = pdev;
>>>> +	INIT_WORK(&work->work, process_defer_work);
>>>> +
>>>> +	rc = devm_add_action_or_reset(&pdev->dev, kill_defer_work, work);
>>>> +	if (rc)
>>>> +		return rc;
>>>> +
>>>> +	platform_set_drvdata(pdev, work);
>>>> +
>>>>    	return walk_hmem_resources(&pdev->dev, hmem_register_device);
>>>>    }
>>>> @@ -174,3 +250,4 @@ MODULE_ALIAS("platform:hmem_platform*");
>>>>    MODULE_DESCRIPTION("HMEM DAX: direct access to 'specific purpose' memory");
>>>>    MODULE_LICENSE("GPL v2");
>>>>    MODULE_AUTHOR("Intel Corporation");
>>>> +MODULE_IMPORT_NS("CXL");
>>>> -- 
>>>> 2.17.1
>>>>
>>
> 

Powered by blists - more mailing lists