Re: [NDCTL PATCH] cxl: Add helper function to verify port is in memdev hierarchy

[Dave Jiang <dave.jiang@intel.com>]

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.jiang@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
>>
>>