[ndctl PATCH 0/4] cxl list and test fixes

[ndctl PATCH 0/4] cxl list and test fixes

[Dan Williams]

Hi Vishal,

These fixups are mostly unrelated, but sending them as a series to make
them easier to pull with b4.

The cxl-list fixups are some nice to haves I found while playing with
the tool on hardware. The test fixups are necessary for executing on
latest kernels and avoiding a compilation warning with latest compilers.

---

Dan Williams (4):
      cxl/list: Fix filtering RCDs
      cxl/list: Filter root decoders by region
      test: Support test modules located in 'updates' instead of 'extra'
      test: Fix dangling pointer warning


 cxl/filter.c     |    6 ++++++
 cxl/lib/libcxl.c |   19 ++++++++++++++++---
 test/core.c      |    2 +-
 test/libndctl.c  |    4 +++-
 4 files changed, 26 insertions(+), 5 deletions(-)

base-commit: b830c4af984e72e5849c0705669aad2ffa19db13

[PATCH 1/4] cxl/list: Fix filtering RCDs

[Dan Williams]

Attempts to filter by memdev fail when the memdev is an RCD (RCH topology):

    # cxl list -BEM -m 11
      Warning: no matching devices found

    [
    ]

This is caused by VH topology assumption where an intervening host-bridge
port is expected between the root CXL port and the endpoint. In an RCH
topology an endpoint is integrated in the host-bridge.

Search for endpoints directly attached to the root:

    # cxl list -BEMu -m 11
    {
      "bus":"root3",
      "provider":"cxl_test",
      "endpoints:root3":[
        {
          "endpoint":"endpoint22",
          "host":"mem11",
          "depth":1,
          "memdev":{
            "memdev":"mem11",
            "ram_size":"2.00 GiB (2.15 GB)",
            "serial":"0xa",
            "numa_node":0,
            "host":"cxl_rcd.10"
          }
        }
      ]
    }


Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/lib/libcxl.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 59e5bdbcc750..e6c94d623303 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1457,8 +1457,9 @@ CXL_EXPORT int cxl_memdev_enable(struct cxl_memdev *memdev)
 	return 0;
 }
 
-static struct cxl_endpoint *cxl_port_find_endpoint(struct cxl_port *parent_port,
-						   struct cxl_memdev *memdev)
+static struct cxl_endpoint *
+cxl_port_recurse_endpoint(struct cxl_port *parent_port,
+			  struct cxl_memdev *memdev)
 {
 	struct cxl_endpoint *endpoint;
 	struct cxl_port *port;
@@ -1468,7 +1469,7 @@ static struct cxl_endpoint *cxl_port_find_endpoint(struct cxl_port *parent_port,
 			if (strcmp(cxl_endpoint_get_host(endpoint),
 				   cxl_memdev_get_devname(memdev)) == 0)
 				return endpoint;
-		endpoint = cxl_port_find_endpoint(port, memdev);
+		endpoint = cxl_port_recurse_endpoint(port, memdev);
 		if (endpoint)
 			return endpoint;
 	}
@@ -1476,6 +1477,18 @@ static struct cxl_endpoint *cxl_port_find_endpoint(struct cxl_port *parent_port,
 	return NULL;
 }
 
+static struct cxl_endpoint *cxl_port_find_endpoint(struct cxl_port *parent_port,
+						   struct cxl_memdev *memdev)
+{
+	struct cxl_endpoint *endpoint;
+
+	cxl_endpoint_foreach(parent_port, endpoint)
+		if (strcmp(cxl_endpoint_get_host(endpoint),
+			   cxl_memdev_get_devname(memdev)) == 0)
+			return endpoint;
+	return cxl_port_recurse_endpoint(parent_port, memdev);
+}
+
 CXL_EXPORT struct cxl_endpoint *
 cxl_memdev_get_endpoint(struct cxl_memdev *memdev)
 {

Re: [PATCH 1/4] cxl/list: Fix filtering RCDs

[Dave Jiang]

On 4/24/23 12:59 PM, Dan Williams wrote:
> Attempts to filter by memdev fail when the memdev is an RCD (RCH topology):
> 
>      # cxl list -BEM -m 11
>        Warning: no matching devices found
> 
>      [
>      ]
> 
> This is caused by VH topology assumption where an intervening host-bridge
> port is expected between the root CXL port and the endpoint. In an RCH
> topology an endpoint is integrated in the host-bridge.
> 
> Search for endpoints directly attached to the root:
> 
>      # cxl list -BEMu -m 11
>      {
>        "bus":"root3",
>        "provider":"cxl_test",
>        "endpoints:root3":[
>          {
>            "endpoint":"endpoint22",
>            "host":"mem11",
>            "depth":1,
>            "memdev":{
>              "memdev":"mem11",
>              "ram_size":"2.00 GiB (2.15 GB)",
>              "serial":"0xa",
>              "numa_node":0,
>              "host":"cxl_rcd.10"
>            }
>          }
>        ]
>      }
> 
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   cxl/lib/libcxl.c |   19 ++++++++++++++++---
>   1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 59e5bdbcc750..e6c94d623303 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1457,8 +1457,9 @@ CXL_EXPORT int cxl_memdev_enable(struct cxl_memdev *memdev)
>   	return 0;
>   }
>   
> -static struct cxl_endpoint *cxl_port_find_endpoint(struct cxl_port *parent_port,
> -						   struct cxl_memdev *memdev)
> +static struct cxl_endpoint *
> +cxl_port_recurse_endpoint(struct cxl_port *parent_port,
> +			  struct cxl_memdev *memdev)
>   {
>   	struct cxl_endpoint *endpoint;
>   	struct cxl_port *port;
> @@ -1468,7 +1469,7 @@ static struct cxl_endpoint *cxl_port_find_endpoint(struct cxl_port *parent_port,
>   			if (strcmp(cxl_endpoint_get_host(endpoint),
>   				   cxl_memdev_get_devname(memdev)) == 0)
>   				return endpoint;
> -		endpoint = cxl_port_find_endpoint(port, memdev);
> +		endpoint = cxl_port_recurse_endpoint(port, memdev);
>   		if (endpoint)
>   			return endpoint;
>   	}
> @@ -1476,6 +1477,18 @@ static struct cxl_endpoint *cxl_port_find_endpoint(struct cxl_port *parent_port,
>   	return NULL;
>   }
>   
> +static struct cxl_endpoint *cxl_port_find_endpoint(struct cxl_port *parent_port,
> +						   struct cxl_memdev *memdev)
> +{
> +	struct cxl_endpoint *endpoint;
> +
> +	cxl_endpoint_foreach(parent_port, endpoint)
> +		if (strcmp(cxl_endpoint_get_host(endpoint),
> +			   cxl_memdev_get_devname(memdev)) == 0)
> +			return endpoint;
> +	return cxl_port_recurse_endpoint(parent_port, memdev);
> +}
> +
>   CXL_EXPORT struct cxl_endpoint *
>   cxl_memdev_get_endpoint(struct cxl_memdev *memdev)
>   {
> 
> 

[PATCH 2/4] cxl/list: Filter root decoders by region

[Dan Williams]

Arrange for util_cxl_decoder_filter_by_region() to consider that root
decoders host multiple regions, unlike switch and endpoint decoders that
have a 1:1 relationship.

Before: (list the root decoders hosting region4 and region9)
    # cxl list -Du -d root -r 4,9
      Warning: no matching devices found

    [
    ]

After:
    # cxl list -Du -d root -r 4,9
    [
      {
        "decoder":"decoder3.0",
        "resource":"0xf010000000",
        "size":"1024.00 MiB (1073.74 MB)",
        "interleave_ways":1,
        "max_available_extent":"512.00 MiB (536.87 MB)",
        "volatile_capable":true,
        "nr_targets":1
      },
      {
        "decoder":"decoder3.5",
        "resource":"0xf1d0000000",
        "size":"256.00 MiB (268.44 MB)",
        "interleave_ways":1,
        "accelmem_capable":true,
        "nr_targets":1
      }
    ]

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/filter.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/cxl/filter.c b/cxl/filter.c
index 90b13be79d9c..6e8d42165205 100644
--- a/cxl/filter.c
+++ b/cxl/filter.c
@@ -661,6 +661,12 @@ util_cxl_decoder_filter_by_region(struct cxl_decoder *decoder,
 	if (!__ident)
 		return decoder;
 
+	/* root decoders filter by children */
+	cxl_region_foreach(decoder, region)
+		if (util_cxl_region_filter(region, __ident))
+			return decoder;
+
+	/* switch and endpoint decoders have a 1:1 association with a region */
 	region = cxl_decoder_get_region(decoder);
 	if (!region)
 		return NULL;

Re: [PATCH 2/4] cxl/list: Filter root decoders by region

[Dave Jiang]

On 4/24/23 12:59 PM, Dan Williams wrote:
> Arrange for util_cxl_decoder_filter_by_region() to consider that root
> decoders host multiple regions, unlike switch and endpoint decoders that
> have a 1:1 relationship.
> 
> Before: (list the root decoders hosting region4 and region9)
>      # cxl list -Du -d root -r 4,9
>        Warning: no matching devices found
> 
>      [
>      ]
> 
> After:
>      # cxl list -Du -d root -r 4,9
>      [
>        {
>          "decoder":"decoder3.0",
>          "resource":"0xf010000000",
>          "size":"1024.00 MiB (1073.74 MB)",
>          "interleave_ways":1,
>          "max_available_extent":"512.00 MiB (536.87 MB)",
>          "volatile_capable":true,
>          "nr_targets":1
>        },
>        {
>          "decoder":"decoder3.5",
>          "resource":"0xf1d0000000",
>          "size":"256.00 MiB (268.44 MB)",
>          "interleave_ways":1,
>          "accelmem_capable":true,
>          "nr_targets":1
>        }
>      ]
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   cxl/filter.c |    6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/cxl/filter.c b/cxl/filter.c
> index 90b13be79d9c..6e8d42165205 100644
> --- a/cxl/filter.c
> +++ b/cxl/filter.c
> @@ -661,6 +661,12 @@ util_cxl_decoder_filter_by_region(struct cxl_decoder *decoder,
>   	if (!__ident)
>   		return decoder;
>   
> +	/* root decoders filter by children */
> +	cxl_region_foreach(decoder, region)
> +		if (util_cxl_region_filter(region, __ident))
> +			return decoder;
> +
> +	/* switch and endpoint decoders have a 1:1 association with a region */
>   	region = cxl_decoder_get_region(decoder);
>   	if (!region)
>   		return NULL;
> 
> 

[PATCH 3/4] test: Support test modules located in 'updates' instead of 'extra'

[Dan Williams]

Since kernel commit:

b74d7bb7ca24 ("kbuild: Modify default INSTALL_MOD_DIR from extra to updates")

...the kernel build process deposits the nfit_test can cxl_test modules in
/lib/modules/$KVER/updates. This is more widely supported across multiple
distributions as a default override for modules that ship in their native
directory.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/core.c b/test/core.c
index 5d1aa23723f1..a354f41dcba0 100644
--- a/test/core.c
+++ b/test/core.c
@@ -209,7 +209,7 @@ retry:
 			break;
 		}
 
-		if (!strstr(path, "/extra/")) {
+		if (!strstr(path, "/extra/") && !strstr(path, "/updates/")) {
 			log_err(&log_ctx, "%s.ko: appears to be production version: %s\n",
 					name, path);
 			break;

Re: [PATCH 3/4] test: Support test modules located in 'updates' instead of 'extra'

[Dave Jiang]

On 4/24/23 12:59 PM, Dan Williams wrote:
> Since kernel commit:
> 
> b74d7bb7ca24 ("kbuild: Modify default INSTALL_MOD_DIR from extra to updates")
> 
> ...the kernel build process deposits the nfit_test can cxl_test modules in

s/can/and/ ?

> /lib/modules/$KVER/updates. This is more widely supported across multiple
> distributions as a default override for modules that ship in their native
> directory.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   test/core.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/core.c b/test/core.c
> index 5d1aa23723f1..a354f41dcba0 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -209,7 +209,7 @@ retry:
>   			break;
>   		}
>   
> -		if (!strstr(path, "/extra/")) {
> +		if (!strstr(path, "/extra/") && !strstr(path, "/updates/")) {
>   			log_err(&log_ctx, "%s.ko: appears to be production version: %s\n",
>   					name, path);
>   			break;
> 
> 

[PATCH 4/4] test: Fix dangling pointer warning

[Dan Williams]

gcc (13.0.1 20230421 (Red Hat 13.0.1-0)) complains:

../test/libndctl.c: In function ‘check_commands’:
../test/libndctl.c:2313:20: warning: storing the address of local variable
‘__check_dimm_cmds’ in ‘check_cmds’ [-Wdangling-poiter=]

...fix it by showing the compiler that the local setting does not escape
the function.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/libndctl.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/libndctl.c b/test/libndctl.c
index 51245cf4ea98..858110c4dbc1 100644
--- a/test/libndctl.c
+++ b/test/libndctl.c
@@ -2322,7 +2322,8 @@ static int check_commands(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
 					ndctl_bus_get_provider(bus),
 					ndctl_dimm_get_id(dimm),
 					ndctl_dimm_get_cmd_name(dimm, i));
-			return -ENXIO;
+			rc = -ENXIO;
+			break;
 		}
 
 		if (!check->check_fn)
@@ -2331,6 +2332,7 @@ static int check_commands(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
 		if (rc)
 			break;
 	}
+	check_cmds = NULL;
 
 	for (i = 0; i < ARRAY_SIZE(__check_dimm_cmds); i++) {
 		if (__check_dimm_cmds[i].cmd)

Re: [PATCH 4/4] test: Fix dangling pointer warning

[Dave Jiang]

On 4/24/23 12:59 PM, Dan Williams wrote:
> gcc (13.0.1 20230421 (Red Hat 13.0.1-0)) complains:
> 
> ../test/libndctl.c: In function ‘check_commands’:
> ../test/libndctl.c:2313:20: warning: storing the address of local variable
> ‘__check_dimm_cmds’ in ‘check_cmds’ [-Wdangling-poiter=]
> 
> ...fix it by showing the compiler that the local setting does not escape
> the function.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   test/libndctl.c |    4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/test/libndctl.c b/test/libndctl.c
> index 51245cf4ea98..858110c4dbc1 100644
> --- a/test/libndctl.c
> +++ b/test/libndctl.c
> @@ -2322,7 +2322,8 @@ static int check_commands(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
>   					ndctl_bus_get_provider(bus),
>   					ndctl_dimm_get_id(dimm),
>   					ndctl_dimm_get_cmd_name(dimm, i));
> -			return -ENXIO;
> +			rc = -ENXIO;
> +			break;
>   		}
>   
>   		if (!check->check_fn)
> @@ -2331,6 +2332,7 @@ static int check_commands(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
>   		if (rc)
>   			break;
>   	}
> +	check_cmds = NULL;
>   
>   	for (i = 0; i < ARRAY_SIZE(__check_dimm_cmds); i++) {
>   		if (__check_dimm_cmds[i].cmd)
> 
>