Re: [PATCH v5 3/7] cxl/acpi: Add background worker to coordinate with cxl_mem probe completion

["Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com>]

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@fujitsu.com>
>> Date:   Fri Aug 20 11:07:15 2025 +0800
>>
>>       Fix probe-order TODO
>>       Signed-off-by: Li Zhijian <lizhijian@fujitsu.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