Re: [PATCH V3 13/20] cxl/mem: Refactor cxl pmem region auto-assembling

[Neeraj Kumar <s.neeraj@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 4587 bytes --]

On 06/10/25 08:55AM, Dave Jiang wrote:
>
>
>On 9/29/25 6:30 AM, Neeraj Kumar wrote:
>> On 23/09/25 03:37PM, Dave Jiang wrote:
>>>
>>>
>>> On 9/17/25 6:41 AM, Neeraj Kumar wrote:
>>>> In 84ec985944ef3, devm_cxl_add_nvdimm() sequence was changed and called
>>>> before devm_cxl_add_endpoint(). It's because cxl pmem region auto-assembly
>>>> used to get called at last in cxl_endpoint_port_probe(), which requires
>>>> cxl_nvd presence.
>>>>
>>>> For cxl region persistency, region creation happens during nvdimm_probe
>>>> which need the completion of endpoint probe.
>>>>
>>>> In order to accommodate both cxl pmem region auto-assembly and cxl region
>>>> persistency, refactored following
>>>>
>>>> 1. Re-Sequence devm_cxl_add_nvdimm() after devm_cxl_add_endpoint(). This
>>>>    will be called only after successful completion of endpoint probe.
>>>>
>>>> 2. Moved cxl pmem region auto-assembly from cxl_endpoint_port_probe() to
>>>>    cxl_mem_probe() after devm_cxl_add_nvdimm(). It gurantees both the
>>>>    completion of endpoint probe and cxl_nvd presence before its call.
>>>
>>> Given that we are going forward with this implementation [1] from Dan and drivers like the type2 enabling are going to be using it as well, can you please consider converting this change to Dan's mechanism instead of creating a whole new one?
>>>
>>> I think the region discovery can be done via the ops->probe() callback. Thanks.
>>>
>>> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/commit/?h=for-6.18/cxl-probe-order&id=88aec5ea7a24da00dc92c7778df4851fe4fd3ec6
>>>
>>> DJ
>>>
>>
>> Sure, Let me revisit this.
>> It seems [1] is there in seperate branch "for-6.18/cxl-probe-order", and not yet merged into next, right?
>
>Right. I believe Smita and Alejandro are using that as well. Depending on who gets there first. We can setup an immutable branch at some point.
>
>[1]: https://lore.kernel.org/linux-cxl/20250822034202.26896-1-Smita.KoralahalliChannabasappa@amd.com/T/#t
>
>DJ

Hi Dave,

As per Dan's [1] newly introduced infra, Following is my understanding.

Currently cxl_pci does not care about the attach state of the cxl_memdev
because all generic memory expansion functionality can be handled by the
cxl_core. For accelerators, the driver needs to know and perform driver
specific initialization if CXL is available, or exectute a fallback to PCIe
only operation.

Dan's new infra is needed for CXL accelerator device drivers that need to
make decisions about enabling CXL dependent functionality in the device, or
falling back to PCIe-only operation.

During cxl_pci_probe() we call devm_cxl_add_memdev(struct cxl_memdev_ops *ops)
where function pointer as ops gets registered which gets called in cxl_mem_probe()
using cxlmd->ops->probe()

The probe callback runs after the port topology is successfully attached for
the given memdev.

So to use this infra we have to pass cxl_region_discovery() as ops parameter
of devm_cxl_add_memdev() getting called from cxl_pci_probe().
  
In this patch-set cxl_region_discovery() signature is different from cxlmd->ops->probe()

    {{{
	void cxl_region_discovery(struct cxl_port *port)
	{
         	device_for_each_child(&port->dev, NULL, discover_region);
	}

	struct cxl_memdev_ops {
	        int (*probe)(struct cxl_memdev *cxlmd);
	};
    }}}

Even after changing the signature of cxl_region_discovery() as per cxlmd->ops->probe()
may create problem as when the ops->probe() fails, then it will halts the probe sequence
of cxl_pci_probe()

It is because discover_region() may fail if two memdevs are participating into one region

Also, region auto assembly is mandatory functionality which creates region
if (cxled->state == CXL_DECODER_STATE_AUTO) gets satisfied.

Currently region auto assembly (added by a32320b71f085) happens after successfull
enumeration of endpoint decoders at cxl_endpoint_port_probe(), which I have moved at
cxl_mem_probe() after devm_cxl_add_nvdimm() which prepares cxl_nvd infra required by it.

As discussed in [1], this patch-set does the movement of auto region assembly from
cxl_endpoint_port_probe() to cxl_mem_probe() and resolved the conflicting dependency
of cxl_nvd infra required by both region creation using LSA and auto region assembly.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/commit/?h=for-6.18/cxl-probe-order&id=88aec5ea7a24da00dc92c7778df4851fe4fd3ec6 
[2]: https://lore.kernel.org/linux-cxl/1931444790.41752909482841.JavaMail.epsvc@epcpadp2new/

Please let me know if my understanding is correct or I am missing something?


Regards,
Neeraj


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]