linux-kernel - Re: [PATCH v5 3/7] cxl/acpi: Add background worker to coordinate with cxl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <67e6d058-7487-42ec-b5e4-932cb4c3893c@fujitsu.com>
Date: Mon, 25 Aug 2025 07:50:12 +0000
From: "Zhijian Li (Fujitsu)" <lizhijian@...itsu.com>
To: "Koralahalli Channabasappa, Smita"
	<Smita.KoralahalliChannabasappa@....com>, Alison Schofield
	<alison.schofield@...el.com>
CC: "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
	"linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"nvdimm@...ts.linux.dev" <nvdimm@...ts.linux.dev>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-pm@...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>, 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>, "Xingtao Yao (Fujitsu)"
	<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>, PradeepVineshReddy Kodamati
	<PradeepVineshReddy.Kodamati@....com>, "Yasunori Gotou (Fujitsu)"
	<y-goto@...itsu.com>
Subject: Re: [PATCH v5 3/7] cxl/acpi: Add background worker to coordinate with
 cxl_mem probe completion



On 22/08/2025 11:56, Koralahalli Channabasappa, Smita wrote:
>>
>>>
>>>>       ```
>>>>
>>>> 3. When CXL_REGION is disabled, there is a failure to fallback to dax_hmem, in which case only CXL Window X is visible.
>>>
>>> Haven't tested !CXL_REGION yet.
> 
> When CXL_REGION is disabled, DEV_DAX_CXL will also be disabled. So dax_hmem should handle it. 

Yes, falling back to dax_hmem/kmem is the result we expect.
I haven't figured out the root cause of the issue yet, but I can tell you that in my QEMU environment,
there is currently a certain probability that it cannot fall back to dax_hmem/kmem.

Upon its failure, I observed the following warnings and errors (with my local fixup kernel).
[   12.203254] kmem dax0.0: mapping0: 0x5d0000000-0x7cfffffff could not reserve region
[   12.203437] kmem dax0.0: probe with driver kmem failed with error -16



> I was able to fallback to dax_hmem. But let me know if I'm missing something.
> 
> config DEV_DAX_CXL
>          tristate "CXL DAX: direct access to CXL RAM regions"
>          depends on CXL_BUS && CXL_REGION && DEV_DAX
> ..
> 
>>>
>>>>       On failure:
>>>>       ```
>>>>       100000000-27ffffff : System RAM
>>>>       5c0001128-5c00011b7 : port1
>>>>       5c0011128-5c00111b7 : port2
>>>>       5d0000000-6cffffff : CXL Window 0
>>>>       6d0000000-7cffffff : CXL Window 1
>>>>       7000000000-700000ffff : PCI Bus 0000:0c
>>>>         7000000000-700000ffff : 0000:0c:00.0
>>>>           7000001080-70000010d7 : mem1
>>>>       ```
>>>>
>>>>       On success:
>>>>       ```
>>>>       5d0000000-7cffffff : dax0.0
>>>>         5d0000000-7cffffff : System RAM (kmem)
>>>>           5d0000000-6cffffff : CXL Window 0
>>>>           6d0000000-7cffffff : CXL Window 1
>>>>       ```
>>>>
>>>> In term of issues 1 and 2, this arises because hmem_register_device() attempts to register resources of all "HMEM devices," whereas we only need to register the IORES_DESC_SOFT_RESERVED resources. I believe resolving the current TODO will address this.
>>>>
>>>> ```
>>>> -   rc = region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
>>>> -                          IORES_DESC_SOFT_RESERVED);
>>>> -   if (rc != REGION_INTERSECTS)
>>>> -       return 0;
>>>> +   /* TODO: insert "Soft Reserved" into iomem here */
>>>> ```
>>>
>>> Above makes sense.
>>
>> I think the subroutine add_soft_reserved() in your previous patchset[1] are able to cover this TODO
>>
>>>
>>> I'll probably wait for an update from Smita to test again, but if you
>>> or Smita have anything you want me to try out on my hardwware in the
>>> meantime, let me know.
>>>
>>
>> Here is my local fixup based on Dan's RFC, it can resovle issue 1 and 2.
> 
> I almost have the same approach 🙂 Sorry, I missed adding your
> "Signed-off-by".. Will include for next revision..

Never mind.

Glad to see your V6, I will test and take a look at soon




> 
>>
>>
>> -- 8< --
>>    commit e7ccd7a01e168e185971da66f4aa13eb451caeaf
>> Author: Li Zhijian <lizhijian@...itsu.com>
>> Date:   Fri Aug 20 11:07:15 2025 +0800
>>
>>       Fix probe-order TODO
>>       Signed-off-by: Li Zhijian <lizhijian@...itsu.com>
>>
>> diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
>> index 754115da86cc..965ffc622136 100644
>> --- a/drivers/dax/hmem/hmem.c
>> +++ b/drivers/dax/hmem/hmem.c
>> @@ -93,6 +93,26 @@ static void process_defer_work(struct work_struct *_work)
>>        walk_hmem_resources(&pdev->dev, handle_deferred_cxl);
>>    }
>> +static int add_soft_reserved(resource_size_t start, resource_size_t len,
>> +                 unsigned long flags)
>> +{
>> +    struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);
>> +    int rc;
>> +
>> +    if (!res)
>> +        return -ENOMEM;
>> +
>> +    *res = DEFINE_RES_NAMED_DESC(start, len, "Soft Reserved",
>> +                     flags | IORESOURCE_MEM,
>> +                     IORES_DESC_SOFT_RESERVED);
>> +
>> +    rc = insert_resource(&iomem_resource, res);
>> +    if (rc)
>> +        kfree(res);
>> +
>> +    return rc;
>> +}
>> +
>>    static int hmem_register_device(struct device *host, int target_nid,
>>                    const struct resource *res)
>>    {
>> @@ -102,6 +122,10 @@ static int hmem_register_device(struct device *host, int target_nid,
>>        long id;
>>        int rc;
>>
>     > +    if (soft_reserve_res_intersects(res->start, resource_size(res),
>> +              IORESOURCE_MEM, IORES_DESC_NONE) == REGION_DISJOINT)
>> +        return 0;
>> +
> 
> Should also handle CONFIG_EFI_SOFT_RESERVE not enabled case..



I think it’s unnecessary. For !CONFIG_EFI_SOFT_RESERVE, it will return directly because soft_reserve_res_intersects() will always return REGION_DISJOINT.


Thanks
Zhijian

> 
> 
> Thanks
> Smita