Message-ID: <aQAmhrS3Im21m_jw@aschofie-mobl2.lan>
Date: Mon, 27 Oct 2025 19:12:22 -0700
From: Alison Schofield <alison.schofield@...el.com>
To: "Koralahalli Channabasappa, Smita" <skoralah@....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>, Davidlohr Bueso <dave@...olabs.net>, "Jonathan
 Cameron" <jonathan.cameron@...wei.com>, Dave Jiang <dave.jiang@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
	"Dan Williams" <dan.j.williams@...el.com>, 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 KH <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>, Ard
 Biesheuvel <ardb@...nel.org>
Subject: Re: [PATCH v3 0/5] dax/hmem, cxl: Coordinate Soft Reserved handling
 with CXL

On Fri, Oct 24, 2025 at 01:08:19PM -0700, Koralahalli Channabasappa, Smita wrote:
> Hi Alison,
> 
> Thanks for the pointers and the branch. Here’s where I landed on the three
> items. Responses inline.
> 
> On 10/20/2025 5:06 PM, Alison Schofield wrote:
> > On Tue, Oct 14, 2025 at 10:52:20AM -0700, Koralahalli Channabasappa, Smita wrote:
> > > Hi Alison,
> > > 
> > > On 10/10/2025 1:49 PM, Alison Schofield wrote:
> > > > On Mon, Oct 06, 2025 at 06:16:24PM -0700, Alison Schofield wrote:
> > > > > On Tue, Sep 30, 2025 at 04:47:52AM +0000, Smita Koralahalli wrote:
> > > > > > This series aims to address long-standing conflicts between dax_hmem and
> > > > > > CXL when handling Soft Reserved memory ranges.
> > > > > 
> > > > > Hi Smita,
> > > > > 
> > > > > Thanks for the updates Smita!
> > > > > 
> > > > > About those "long-standing conflicts": In the next rev, can you resurrect,
> > > > > or recreate the issues list that this set is addressing. It's been a
> > > > > long and winding road with several handoffs (me included) and it'll help
> > > > > keep the focus.
> > > > > 
> > > > > Hotplug works :)  Auto region comes up, we tear it down and can recreate it,
> > > > > in place, because the soft reserved resource is gone (no longer occupying
> > > > > the CXL Window and causing recreate to fail.)
> > > > > 
> > > > > !CONFIG_CXL_REGION works :) All resources go directly to DAX.
> > > > > 
> > > > > The scenario that is failing is handoff to DAX after region assembly
> > > > > failure. (Dan reminded me to check that today.) That is mostly related
> > > > > to Patch4, so I'll respond there.
> > > > > 
> > > > > --Alison
> > > > 
> > > > Hi Smita -
> > > > 
> > > > (after off-list chat w Smita about what is and is not included)
> > > > 
> > > > This CXL failover to DAX case is not implemented. In my response in Patch 4,
> > > > I cobbled something together that made it work in one test case. But to be
> > > > clear, there was some trickery in the CXL region driver to even do that.
> > > > 
> > > > One path forward is to update this set restating the issues it addresses, and
> > > > remove any code and comments that are tied to failing over to DAX after a
> > > > region assembly failure.
> > > > 
> > > > That leaves the issue Dan raised, "shutdown CXL in favor of vanilla DAX devices
> > > > as an emergency fallback for platform configuration quirks and bugs"[1], for a
> > > > future patch.
> > > > 
> > > > -- Alison
> > > > 
> > > > [1] The failover to DAX was last described in response to v5 of the 'prior' patchset.
> > > > https://lore.kernel.org/linux-cxl/20250715180407.47426-1-Smita.KoralahalliChannabasappa@amd.com/
> > > > https://lore.kernel.org/linux-cxl/687ffcc0ee1c8_137e6b100ed@dwillia2-xfh.jf.intel.com.notmuch/
> > > > https://lore.kernel.org/linux-cxl/68808fb4e4cbf_137e6b100cc@dwillia2-xfh.jf.intel.com.notmuch/
> > > 
> > > [+cc Nathan, Terry]
> > > 
> > >  From the AMD side, our primary concern in this series is CXL hotplug. With
> > > the patches as is, the hotplug flows are working for us: region comes up, we
> > > can tear it down, and recreate it in place because the soft reserved window
> > > is released.
> > > 
> > > On our systems I consistently see wait_for_device_probe() block until region
> > > assembly has completed so I don’t currently have evidence of a sequencing
> > > hole there on AMD platforms.
> > > 
> > > Once CXL windows are discovered, would it be acceptable for dax_hmem to
> > > simply ignore soft reserved ranges inside those windows, assuming CXL will
> > > own and manage them? That aligns with Dan’s guidance about letting CXL win
> > > those ranges when present.
> > > https://lore.kernel.org/all/687fef9ec0dd9_137e6b100c8@dwillia2-xfh.jf.intel.com.notmuch/
> > > 
> > > If that approach sounds right, I can reword the commit descriptions in
> > > patches 4/5 and 5/5 to drop the parts about region assembly failures and
> > > remove the REGISTER enum.
> > > 
> > > And then leave the “shutdown CXL in favor of vanilla DAX as an emergency
> > > fallback for platform configuration quirks and bugs” to a future, dedicated
> > > patch.
> > > 
> > > Thanks
> > > Smita
> > 
> > Hi Smita,
> > 
> > I was able to discard the big sleep after picking up the patch "cxl/mem:
> > Arrange for always-synchronous memdev attach" from Alejandro's Type2 set.
> > 
> > With that patch, all CXL probing completed before the HMEM probe so the
> > deferred waiting mechanism of the HMEM driver seems unnecessary. Please
> > take a look.
> > 
> > That patch, is one of four in this branch Dan provided:
> > https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/log/?h=for-6.18/cxl-probe-order
> > 
> > After chats with Dan and DaveJ, we thought the Soft Reserved set was the
> > right place to introduce these probe order patches (let Type 2 follow).
> > So, the SR set adds these three patches:
> > 
> > - **cxl/mem: Arrange for always-synchronous memdev attach**
> > - cxl/port: Arrange for always synchronous endpoint attach
> > - cxl/mem: Introduce a memdev creation ->probe() operation
> > 
> > **I actually grabbed this one from v19 Type2 set, not the CXL branch,
> > so you may need to see if Alejandro changed anything in that one.
> > 
> > When picking those up, there's a bit of wordsmithing to do in the
> > commit logs. Probably replace mentions of needing for accelerators
> > with needing for synchronizing the usage of soft-reserved resources.
> > 
> > Note that the HMEM driver is also not picking up unused SR ranges.
> > That was described in review comments here:
> > https://lore.kernel.org/linux-cxl/aORscMprmQyGlohw@aschofie-mobl2.lan
> > 
> > Summarized for my benefit ;)
> > - pick up all the probe order patches,
> > - determine whether the HMEM deferral is needed, maybe drop it,
> > - register the unused SR, don't drop based on intersect w 'CXL Window'
> > 
> > With all that, nothing would be left undone in the HMEM driver. The region
> > driver would still need to fail gracefully and release resources in a
> > follow-on patch.
> > 
> > Let me know what you find wrt the timing, ie is the wait_for_device_probe()
> > needed at all?
> > 
> > Thanks!
> > -- Alison
> > 
> 
> 1. Pick up all the probe order patches
> I pulled in the three patches you listed.
> They build and run fine here.
> 
> 2. Determine whether HMEM deferral is needed (and maybe drop it)
> On my system, even with those three patches, the HMEM probe still races
> ahead of CXL region assembly. A short dmesg timeline shows HMEM registering
> before init_hdm_decoder() and region construction:
> 
> ..
> [   26.597369] hmem_register_device: hmem_platform hmem_platform.0:
> registering released/unclaimed range with DAX: [mem 0x850000000-0x284fffffff
> flags 0x80000200]
> [   26.602371] init_hdm_decoder: cxl_port port1: decoder1.0: range:
> 0x850000000-0x284fffffff iw: 1 ig: 256
> [   26.628614] init_hdm_decoder: cxl_port endpoint7: decoder7.0: range:
> 0x850000000-0x284fffffff iw: 1 ig: 256
> [   26.628711] __construct_region: cxl_pci 0000:e1:00.0: mem2:decoder7.0:
> __construct_region region0 res: [mem 0x850000000-0x284fffffff flags 0x200]
> iw: 1 ig: 256
> [   26.628714] cxl_calc_interleave_pos: cxl_mem mem2: decoder:decoder7.0
> parent:0000:e1:00.0 port:endpoint7 range:0x850000000-0x284fffffff pos:0
> [   44.022792] __hmem_register_resource: hmem range [mem
> 0x850000000-0x284fffffff flags 0x80000200] already active
> [   49.991221] kmem dax0.0: mapping0: 0x850000000-0x284fffffff could not
> reserve region
> ..
> 
> As, region assembly still completes after HMEM on my platform,
> wait_for_device_probe() might be needed to avoid HMEM claiming ranges before
> CXL region assembly.
> 
> 3. Register unused SR, don’t drop based on intersect with “CXL Window”
> Agree with your review note: checking region_intersects(..., IORES_DESC_CXL)
> is not reliable for 'CXL owns this'. IORES_DESC_CXL marks just the 'CXL
> Windows' so the intersect test is true regardless of whether a region was
> actually assembled.
> 
> I tried the insert SR and rely on -EBUSY approach suggested.
> 
> https://lore.kernel.org/linux-cxl/aORscMprmQyGlohw@aschofie-mobl2.lan/#t
> 
> On my setup it never returns -EBUSY, the SR inserts cleanly even when the
> CXL region has already been assembled successfully before dax_hmem.
> 
> insert_resource() is treating 'fully contains' as a valid hierarchy, not a
> conflict. The SR I insert covers exactly the same range as the CXL
> window/region. In that situation, insert_resource(&iomem_resource, SR) does
> not report a conflict, instead, it inserts SR and reparents the existing CXL
> window/region under SR. That matches what I see in the tree:
> 
> 850000000-284fffffff : Soft Reserved
>   850000000-284fffffff : CXL Window 0
>     850000000-284fffffff : region0
>       850000000-284fffffff : dax0.0
>         850000000-284fffffff : System RAM (kmem)
> ... (same for the other windows)
> 
> So there is no overlap error to trigger -EBUSY, the tree is simply
> restructured.
> 
> insert_resource_conflict() is also behaving the same.
> 
> and hence the kmem failure
> kmem dax6.0: mapping0: 0x850000000-0x284fffffff could not reserve region
> kmem dax6.0: probe with driver kmem failed with error -16
> 
> walk_iomem_res_desc() was also not a good discriminator here: it passes a
> temporary struct resource to the callback (name == NULL, no child/sibling
> links), so I couldn't reliably detect the 'region under window' relationship
> from that walker alone. (only CXL windows were discovered properly).
> 
> Below worked for me instead. I could see the region assembly success and
> failure cases handled properly.
> 
> Walk the real iomem_resource tree: find the enclosing CXL window for the SR
> range, then check if there’s a region child that covers sr->start, sr->end.
> 
> If yes, drop (CXL owns it).
> 
> If no, register as unused SR with DAX.
> 
> 
> +static struct resource *cxl_window_exists(resource_size_t start,
> +                                        resource_size_t end)
> +{
> +       struct resource *r;
> +
> +       for (r = iomem_resource.child; r; r = r->sibling) {
> +               if (r->desc == IORES_DESC_CXL &&
> +                   r->start == start && r->end == end)
> +                       return r;
> +       }
> +
> +       return NULL;
> +}
> +
> +static bool cxl_region_exists(resource_size_t start, resource_size_t end)
> +{
> +       const struct resource *res, *child;
> +
> +       res = cxl_window_exists(start, end);
> +       if (!res)
> +               return false;
> +
> +       for (child = res->child; child; child = child->sibling) {
> +               if (child->start <= start && child->end <= end)
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
>  static int handle_deferred_cxl(struct device *host, int target_nid,
>                                const struct resource *res)
>  {
> -       /* TODO: Handle region assembly failures */
> +       if (region_intersects(res->start, resource_size(res),
> IORESOURCE_MEM,
> +                             IORES_DESC_CXL) != REGION_DISJOINT) {
> +

Will it work to search directly for the region above by using params
IORESOURCE_MEM, IORES_DESC_NONE. This way we only get region conflicts,
no empty windows to examine. I think that might replace cxl_region_exists()
work below.



> +               if (cxl_region_exists(res->start, res->end)) {
> +                       dax_cxl_mode = DAX_CXL_MODE_DROP;
> +                       dev_dbg(host, "dropping CXL range: %pr\n", res);
> +               }
> +               else {
> +                       dax_cxl_mode = DAX_CXL_MODE_REGISTER;
> +                       dev_dbg(host, "registering CXL range: %pr\n", res);
> +               }
> +
> +               hmem_register_device(host, target_nid, res);
> +       }
> +
>         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;
> 
>         /* relies on cxl_acpi and cxl_pci having had a chance to load */
>         wait_for_device_probe();
> 
>         walk_hmem_resources(&pdev->dev, handle_deferred_cxl);
> }
> 
> For region assembly failure (Thanks for the patch to test this!):
> 
> hmem_register_device: hmem_platform hmem_platform.0: deferring range to CXL:
> [mem 0x850000000-0x284fffffff flags 0x80000200]
> handle_deferred_cxl: hmem_platform hmem_platform.0: registering CXL range:
> [mem 0x850000000-0x284fffffff flags 0x80000200]
> hmem_register_device: hmem_platform hmem_platform.0: registering CXL range:
> [mem 0x850000000-0x284fffffff flags 0x80000200]
> 
> For region assembly success:
> 
> hmem_register_device: hmem_platform hmem_platform.0: deferring range to CXL:
> [mem 0x850000000-0x284fffffff flags 0x80000200]
> handle_deferred_cxl: hmem_platform hmem_platform.0: dropping CXL range: [mem
> 0x850000000-0x284fffffff flags 0x80000200]
> hmem_register_device: hmem_platform hmem_platform.0: dropping CXL range:
> [mem 0x850000000-0x284fffffff flags 0x80000200]
> 
> Happy to fold this into v4 if it looks good.
> 
> Thanks
> Smita
> > 
> > > 
> > > > 
> > > > > 
> > > > > 
> > > 
> 

Powered by blists - more mailing lists