Re: [PATCH v4 7/7] cxl/dax: Defer DAX consumption of SOFT RESERVED resources until after CXL region creation

[Alison Schofield <alison.schofield@intel.com>]

On Tue, Jul 15, 2025 at 11:19:05AM -0700, Koralahalli Channabasappa, Smita wrote:
> Hi Alison,
> 
> Sorry I missed this email before sending out v5. Comments inline.

Ah, I see v5 now and will try it out. Thanks!


> 
> On 7/15/2025 9:27 AM, Alison Schofield wrote:
> > On Tue, Jun 03, 2025 at 10:19:49PM +0000, Smita Koralahalli wrote:
> > > From: Nathan Fontenot <nathan.fontenot@amd.com>
> > > 
> > > The DAX HMEM driver currently consumes all SOFT RESERVED iomem resources
> > > during initialization. This interferes with the CXL driver’s ability to
> > > create regions and trim overlapping SOFT RESERVED ranges before DAX uses
> > > them.
> > > 
> > > To resolve this, defer the DAX driver's resource consumption if the
> > > cxl_acpi driver is enabled. The DAX HMEM initialization skips walking the
> > > iomem resource tree in this case. After CXL region creation completes,
> > > any remaining SOFT RESERVED resources are explicitly registered with the
> > > DAX driver by the CXL driver.
> > > 
> > > This sequencing ensures proper handling of overlaps and fixes hotplug
> > > failures.
> > 
> > Hi Smita,
> > 
> > About the issue I first mentioned here [1]. The HMEM driver is not
> > waiting for region probe to finish. By the time region probe attempts
> > to hand off the memory to DAX, the memory is already marked as System RAM.
> > 
> > See 'case CXL_PARTMODE_RAM:' in cxl_region_probe(). The is_system_ram()
> > test fails so devm_cxl_add_dax_region() not possible.
> > 
> > This means that in appearance, just looking at /proc/iomem/, this
> > seems to have worked. There is no soft reserved and the dax and
> > kmem resources are child resources of the region resource. But they
> > were not set up by the region driver, hence no unregister callback
> > is triggered when the region is disabled.
> 
> I believe this should be resolved in v5. I see the following dmesg entries
> indicating that devm_cxl_add_dax_region() is being called successfully for
> all regions:
> 
> # dmesg | grep devm_cxl_add_dax_region
> [   40.730864] devm_cxl_add_dax_region: cxl_region region0: region0:
> register dax_region0
> [   40.756307] devm_cxl_add_dax_region: cxl_region region1: region1:
> register dax_region1
> [   43.689882] devm_cxl_add_dax_region: cxl_region region2: region2:
> register dax_region2
> 
> cat /proc/iomem
> 
> 850000000-284fffffff : CXL Window 0
>   850000000-284fffffff : region0
>     850000000-284fffffff : dax0.0
>       850000000-284fffffff : System RAM (kmem)
> 2850000000-484fffffff : CXL Window 1
>   2850000000-484fffffff : region1
>     2850000000-484fffffff : dax1.0
>       2850000000-484fffffff : System RAM (kmem)
> 4850000000-684fffffff : CXL Window 2
>   4850000000-684fffffff : region2
>     4850000000-684fffffff : dax2.0
>       4850000000-684fffffff : System RAM (kmem)
> 
> I suspect devm_cxl_add_dax_region() didn't execute in v4 because
> hmem_register_resource() (called from hmat.c) preemptively created
> hmem_active entries. These were consumed during walk_hmem_resources() and
> registered by hmem_register_device() before the CXL region probe could
> complete.
> 
> In v5, if CONFIG_CXL_ACPI is enabled, soft reserved resources are stored in
> hmem_deferred_active instead and hmem_register_resource() calls from hmat.c
> are disabled. This ensures DAX registration happens only through CXL drivers
> probing.
> 
> > 
> > It appears like this:
> > 
> > c080000000-17dbfffffff : CXL Window 0
> >    c080000000-c47fffffff : region2
> >      c080000000-c47fffffff : dax0.0
> >        c080000000-c47fffffff : System RAM (kmem)
> > 
> > Now, to make the memory available for reuse, need to do:
> > # daxctl offline-memory dax0.0
> > # daxctl destroy-device --force dax0.0
> > # cxl disable-region 2
> > # cxl destroy-region 2
> > 
> > Whereas previously, did this:
> > # daxctl offline-memory dax0.0
> > # cxl disable-region 2
> >    After disabling region, dax device unregistered.
> > # cxl destroy-region 2
> 
> I haven’t yet tested this specific unregister flow with v5. I’ll verify and
> follow up if I see any issues. Meanwhile, please let me know if you still
> observe the same behavior or need additional debug info from my side.
> 
> Thanks
> Smita
> > 
> > I do see that __cxl_region_softreserv_update() is not called until
> > after cxl_region_probe() completes, so that is waiting properly to
> > pick up the scraps. I'm actually not sure there would be any scraps
> > though, if the HMEM driver has already done it's thing. In my case
> > the Soft Reserved size is same as region, so I cannot tell what
> > would happen if that Soft Reserved had more capacity than the region.
> > 
> > If I do this: # CONFIG_DEV_DAX_HMEM is not set, works same as before,
> > which is as expected.
> > 
> > Let me know if I can try anything else out or collect more info.
> > 
> > --Alison
> > 
> > 
> > [1] https://lore.kernel.org/nvdimm/20250603221949.53272-1-Smita.KoralahalliChannabasappa@amd.com/T/#m10c0eb7b258af7cd0c84c7ee2c417c055724f921
> > 
> > 
> > > 
> > > Co-developed-by: Nathan Fontenot <Nathan.Fontenot@amd.com>
> > > Signed-off-by: Nathan Fontenot <Nathan.Fontenot@amd.com>
> > > Co-developed-by: Terry Bowman <terry.bowman@amd.com>
> > > Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> > > Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> > > ---
> > >   drivers/cxl/core/region.c | 10 +++++++++
> > >   drivers/dax/hmem/device.c | 43 ++++++++++++++++++++-------------------
> > >   drivers/dax/hmem/hmem.c   |  3 ++-
> > >   include/linux/dax.h       |  6 ++++++
> > >   4 files changed, 40 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > > index 3a5ca44d65f3..c6c0c7ba3b20 100644
> > > --- a/drivers/cxl/core/region.c
> > > +++ b/drivers/cxl/core/region.c
> > > @@ -10,6 +10,7 @@
> > >   #include <linux/sort.h>
> > >   #include <linux/idr.h>
> > >   #include <linux/memory-tiers.h>
> > > +#include <linux/dax.h>
> > >   #include <cxlmem.h>
> > >   #include <cxl.h>
> > >   #include "core.h"
> > > @@ -3553,6 +3554,11 @@ static struct resource *normalize_resource(struct resource *res)
> > >   	return NULL;
> > >   }
> > > +static int cxl_softreserv_mem_register(struct resource *res, void *unused)
> > > +{
> > > +	return hmem_register_device(phys_to_target_node(res->start), res);
> > > +}
> > > +
> > >   static int __cxl_region_softreserv_update(struct resource *soft,
> > >   					  void *_cxlr)
> > >   {
> > > @@ -3590,6 +3596,10 @@ int cxl_region_softreserv_update(void)
> > >   				    __cxl_region_softreserv_update);
> > >   	}
> > > +	/* Now register any remaining SOFT RESERVES with DAX */
> > > +	walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED, IORESOURCE_MEM,
> > > +			    0, -1, NULL, cxl_softreserv_mem_register);
> > > +
> > >   	return 0;
> > >   }
> > >   EXPORT_SYMBOL_NS_GPL(cxl_region_softreserv_update, "CXL");
> > > diff --git a/drivers/dax/hmem/device.c b/drivers/dax/hmem/device.c
> > > index 59ad44761191..cc1ed7bbdb1a 100644
> > > --- a/drivers/dax/hmem/device.c
> > > +++ b/drivers/dax/hmem/device.c
> > > @@ -8,7 +8,6 @@
> > >   static bool nohmem;
> > >   module_param_named(disable, nohmem, bool, 0444);
> > > -static bool platform_initialized;
> > >   static DEFINE_MUTEX(hmem_resource_lock);
> > >   static struct resource hmem_active = {
> > >   	.name = "HMEM devices",
> > > @@ -35,9 +34,7 @@ EXPORT_SYMBOL_GPL(walk_hmem_resources);
> > >   static void __hmem_register_resource(int target_nid, struct resource *res)
> > >   {
> > > -	struct platform_device *pdev;
> > >   	struct resource *new;
> > > -	int rc;
> > >   	new = __request_region(&hmem_active, res->start, resource_size(res), "",
> > >   			       0);
> > > @@ -47,21 +44,6 @@ static void __hmem_register_resource(int target_nid, struct resource *res)
> > >   	}
> > >   	new->desc = target_nid;
> > > -
> > > -	if (platform_initialized)
> > > -		return;
> > > -
> > > -	pdev = platform_device_alloc("hmem_platform", 0);
> > > -	if (!pdev) {
> > > -		pr_err_once("failed to register device-dax hmem_platform device\n");
> > > -		return;
> > > -	}
> > > -
> > > -	rc = platform_device_add(pdev);
> > > -	if (rc)
> > > -		platform_device_put(pdev);
> > > -	else
> > > -		platform_initialized = true;
> > >   }
> > >   void hmem_register_resource(int target_nid, struct resource *res)
> > > @@ -83,9 +65,28 @@ static __init int hmem_register_one(struct resource *res, void *data)
> > >   static __init int hmem_init(void)
> > >   {
> > > -	walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED,
> > > -			IORESOURCE_MEM, 0, -1, NULL, hmem_register_one);
> > > -	return 0;
> > > +	struct platform_device *pdev;
> > > +	int rc;
> > > +
> > > +	if (!IS_ENABLED(CONFIG_CXL_ACPI)) {
> > > +		walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED,
> > > +				    IORESOURCE_MEM, 0, -1, NULL,
> > > +				    hmem_register_one);
> > > +	}
> > > +
> > > +	pdev = platform_device_alloc("hmem_platform", 0);
> > > +	if (!pdev) {
> > > +		pr_err("failed to register device-dax hmem_platform device\n");
> > > +		return -1;
> > > +	}
> > > +
> > > +	rc = platform_device_add(pdev);
> > > +	if (rc) {
> > > +		pr_err("failed to add device-dax hmem_platform device\n");
> > > +		platform_device_put(pdev);
> > > +	}
> > > +
> > > +	return rc;
> > >   }
> > >   /*
> > > diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
> > > index 3aedef5f1be1..a206b9b383e4 100644
> > > --- a/drivers/dax/hmem/hmem.c
> > > +++ b/drivers/dax/hmem/hmem.c
> > > @@ -61,7 +61,7 @@ static void release_hmem(void *pdev)
> > >   	platform_device_unregister(pdev);
> > >   }
> > > -static int hmem_register_device(int target_nid, const struct resource *res)
> > > +int hmem_register_device(int target_nid, const struct resource *res)
> > >   {
> > >   	struct device *host = &dax_hmem_pdev->dev;
> > >   	struct platform_device *pdev;
> > > @@ -124,6 +124,7 @@ static int hmem_register_device(int target_nid, const struct resource *res)
> > >   	platform_device_put(pdev);
> > >   	return rc;
> > >   }
> > > +EXPORT_SYMBOL_GPL(hmem_register_device);
> > >   static int dax_hmem_platform_probe(struct platform_device *pdev)
> > >   {
> > > diff --git a/include/linux/dax.h b/include/linux/dax.h
> > > index a4ad3708ea35..5052dca8b3bc 100644
> > > --- a/include/linux/dax.h
> > > +++ b/include/linux/dax.h
> > > @@ -299,10 +299,16 @@ static inline int dax_mem2blk_err(int err)
> > >   #ifdef CONFIG_DEV_DAX_HMEM_DEVICES
> > >   void hmem_register_resource(int target_nid, struct resource *r);
> > > +int hmem_register_device(int target_nid, const struct resource *res);
> > >   #else
> > >   static inline void hmem_register_resource(int target_nid, struct resource *r)
> > >   {
> > >   }
> > > +
> > > +static inline int hmem_register_device(int target_nid, const struct resource *res)
> > > +{
> > > +	return 0;
> > > +}
> > >   #endif
> > >   typedef int (*walk_hmem_fn)(int target_nid, const struct resource *res);
> > > -- 
> > > 2.17.1
> > > 
> 
>