On 6/18/25 2:54 PM, Alison Schofield wrote:
> On Wed, Jun 18, 2025 at 01:41:17PM -0700, Dave Jiang wrote:
>> 'cxl enable-port -m' uses cxl_port_get_dport_by_memdev() to find the
>> memdevs that are associated with a port in order to enable those
>> associated memdevs. When the kernel switch to delayed dport
>> initialization by enumerating the dports during memdev probe, the
>> dports are no longer valid until the memdev is probed. This means
>> that cxl_port_get_dport_by_memdev() will not find any memdevs under
>> the port.
>>
>> Add a new helper function cxl_port_is_memdev_hierarchy() that checks if a
>> port is in the memdev hierarchy via the memdev->host_path where the sysfs
>> path contains all the devices in the hierarchy. This call is also backward
>> compatible with the old behavior.
>
> I get how this new function works w the delayed dport init that is
> coming soon to the CXL driver. I'm not so clear on why we leave the
> existing function in place when we know it will fail in some use
> cases. (It is a libcxl fcn afterall)
>
> Why not change the behavior of the existing function?
> How come this usage of cxl_port_get_dport_by_memdev() needs to change
> to the new helper and not the other usage in action_disable()?
>
> If the 'sometimes fails to find' function stays, how about libcxl
> docs explaining the limitations.
>
> Just stirring the pot to better understand ;)
What's the process of retiring API calls? Add deprecated in the doc? Add
warnings when called?
>
> --Alison
>
>
>>
>> Signed-off-by: Dave Jiang <dave.ji...@intel.com>
>> ---
>> cxl/lib/libcxl.c | 31 +++++++++++++++++++++++++++++++
>> cxl/lib/libcxl.sym | 5 +++++
>> cxl/libcxl.h | 3 +++
>> cxl/port.c | 2 +-
>> 4 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
>> index 5d97023377ec..cafde1cee4e8 100644
>> --- a/cxl/lib/libcxl.c
>> +++ b/cxl/lib/libcxl.c
>> @@ -2024,6 +2024,37 @@ CXL_EXPORT int cxl_memdev_nvdimm_bridge_active(struct
>> cxl_memdev *memdev)
>> return is_enabled(path);
>> }
>>
>> +CXL_EXPORT bool cxl_memdev_is_port_ancestor(struct cxl_memdev *memdev,
>> + struct cxl_port *port)
>> +{
>> + const char *uport = cxl_port_get_host(port);
>> + const char *start = "devices";
>> + const char *pstr = "platform";
>> + char *host, *pos;
>> +
>> + host = strdup(memdev->host_path);
>> + if (!host)
>> + return false;
>> +
>> + pos = strstr(host, start);
>> + pos += strlen(start) + 1;
>> + if (strncmp(pos, pstr, strlen(pstr)) == 0)
>> + pos += strlen(pstr) + 1;
>> + pos = strtok(pos, "/");
>> +
>> + while (pos) {
>> + if (strcmp(pos, uport) == 0) {
>> + free(host);
>> + return true;
>> + }
>> + pos = strtok(NULL, "/");
>> + }
>> +
>> + free(host);
>> +
>> + return false;
>> +}
>> +
>> static int cxl_port_init(struct cxl_port *port, struct cxl_port
>> *parent_port,
>> enum cxl_port_type type, struct cxl_ctx *ctx, int id,
>> const char *cxlport_base)
>> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
>> index 3ad0cd06e25a..e01a676cdeb9 100644
>> --- a/cxl/lib/libcxl.sym
>> +++ b/cxl/lib/libcxl.sym
>> @@ -295,3 +295,8 @@ global:
>> cxl_fwctl_get_major;
>> cxl_fwctl_get_minor;
>> } LIBECXL_8;
>> +
>> +LIBCXL_10 {
>> +global:
>> + cxl_memdev_is_port_ancestor;
>> +} LIBCXL_9;
>> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
>> index 54d97d7bb501..54bc025b121d 100644
>> --- a/cxl/libcxl.h
>> +++ b/cxl/libcxl.h
>> @@ -179,6 +179,9 @@ bool cxl_dport_maps_memdev(struct cxl_dport *dport,
>> struct cxl_memdev *memdev);
>> struct cxl_dport *cxl_port_get_dport_by_memdev(struct cxl_port *port,
>> struct cxl_memdev *memdev);
>>
>> +bool cxl_memdev_is_port_ancestor(struct cxl_memdev *memdev,
>> + struct cxl_port *port);
>> +
>> #define cxl_dport_foreach(port, dport)
>> \
>> for (dport = cxl_dport_get_first(port); dport != NULL; \
>> dport = cxl_dport_get_next(dport))
>> diff --git a/cxl/port.c b/cxl/port.c
>> index 89f3916d85aa..c951c0c771e8 100644
>> --- a/cxl/port.c
>> +++ b/cxl/port.c
>> @@ -102,7 +102,7 @@ static int action_enable(struct cxl_port *port)
>> return rc;
>>
>> cxl_memdev_foreach(ctx, memdev)
>> - if (cxl_port_get_dport_by_memdev(port, memdev))
>> + if (cxl_memdev_is_port_ancestor(memdev, port))
>> cxl_memdev_enable(memdev);
>> return 0;
>> }
>>
>> base-commit: 74b9e411bf13e87df39a517d10143fafa7e2ea92
>> --
>> 2.49.0
>>
>>
