Message-ID: <d920b7d0-61db-481f-b256-a1f51ac7f743@amd.com>
Date: Fri, 24 Oct 2025 13:08:19 -0700
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, 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

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) {
+
+               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