On Wed, Jun 18, 2025 at 02:58:13PM -0700, Dave Jiang wrote:
>
>
> 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?
What is wanted here? Should a v2 of the existing cxl_port_get_dport...
be replaced with a v2 that can differentiate btw memdev not probed vs
NULL for dport not found.
I see example of v2 APIs in ndctl/ndctl lib, so doable, but first need
to define what is wanted.
>
> >
> > --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
> >>
> >>
>
