[ndctl PATCH v2 00/18] cxl-cli test and usability updates

[ndctl PATCH v2 00/18] cxl-cli test and usability updates

[Dan Williams]

Changes since v1 [1]:
- Clarify the changelog as to why emitting pmem_size on non-pmem capable
  device is confusing, and clean up the man page to remove stale example
  of "{pmem,ram}_size: 0" (Alison)
- Update the man page for cxl create-region to make it clear that the
  tool maybe able to select the possible memdevs, but it may not still
  require a manual specification of the ordering until the tool can do
  that automatically. (Alison)
- Support specification of the region order in either individual args, a
  comma separated list, or space separated list. (Vishal)
- Fixup count of the target list in the presence of list args. (Vishal)
- Fixup test/cxl-topology.sh to use consistent 'math' context for comparisons
  (Vishal)
- Include some more documentation fixups and list enhancements

[1]: http://lore.kernel.org/r/166777840496.1238089.5601286140872803173.stgit@dwillia2-xfh.jf.intel.com

---
The v6.1 kernel picks up new cxl_test infrastructure for a switch
attached to a single-port host-bridge. While extending the region
creation and topology tests for that change, some additional updates
were identified. The main one is the ability to elide the "ways"
"memdevs" arguments to 'cxl create-region'. Those parameters are now
derived by the result of a topology walk under the given root decoder.
I.e.:

    cxl create-region -d decoder3.4

...will internally perform a:

    cxl list -M -d decoder3.4

...operation and use those memdevs as region targets.

This also updates 'create-region' size detection to be maximum free
extent in the decoder, or the available capacity in the target memdevs,
whichever is smaller.

Some miscellaneous fixes and updates are included as well.

---

Dan Williams (18):
      ndctl/test: Move firmware-update.sh to the 'destructive' set
      ndctl/test: Add kernel backtrace detection to some dax tests
      ndctl/clang-format: Move minimum version to 6
      ndctl/clang-format: Fix space after for_each macros
      cxl/list: Always attempt to collect child objects
      cxl/list: Add a 'firmware_node' alias
      cxl/list: Add parent_dport attribute to port listings
      cxl/list: Skip emitting pmem_size when it is zero
      cxl/filter: Return json-c topology
      cxl/list: Record cxl objects in json objects
      cxl/region: Make ways an integer argument
      cxl/region: Make granularity an integer argument
      cxl/region: Use cxl_filter_walk() to gather create-region targets
      cxl/region: Trim region size by max available extent
      cxl/Documentation: Fix whitespace typos in create-region man page
      cxl/region: Autoselect memdevs for create-region
      cxl/test: Extend cxl-topology.sh for a single root-port host-bridge
      cxl/test: Test single-port host-bridge region creation


 .clang-format                           |   38 +--
 Documentation/cxl/cxl-create-region.txt |   12 +
 Documentation/cxl/cxl-list.txt          |    5 
 cxl/filter.c                            |   36 +--
 cxl/filter.h                            |   22 ++
 cxl/json.c                              |   57 ++++
 cxl/lib/libcxl.c                        |   69 +++++
 cxl/lib/libcxl.sym                      |    7 +
 cxl/lib/private.h                       |    4 
 cxl/libcxl.h                            |    3 
 cxl/list.c                              |    7 -
 cxl/region.c                            |  406 ++++++++++++++++++++-----------
 test/common                             |   10 +
 test/cxl-create-region.sh               |   28 ++
 test/cxl-region-sysfs.sh                |    4 
 test/cxl-topology.sh                    |   53 ++--
 test/dax.sh                             |    2 
 test/daxdev-errors.sh                   |    2 
 test/meson.build                        |    2 
 test/multi-dax.sh                       |    2 
 util/util.h                             |    9 +
 21 files changed, 537 insertions(+), 241 deletions(-)

base-commit: 1d4dbf6ff6eb988864d154792aaa098a2b11a244

[ndctl PATCH v2 01/18] ndctl/test: Move firmware-update.sh to the 'destructive' set

[Dan Williams]

The firmware update test attempts a system-suspend test which may break
systems that have a broken driver, or otherwise are not prepared to support
suspend.

Link: https://github.com/pmem/ndctl/issues/221
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/meson.build |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/meson.build b/test/meson.build
index 5953c286d13f..c31d8eac66c5 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -170,7 +170,6 @@ tests = [
   [ 'btt-errors.sh',          btt_errors,	  'ndctl' ],
   [ 'hugetlb',                hugetlb,		  'ndctl' ],
   [ 'btt-pad-compat.sh',      btt_pad_compat,	  'ndctl' ],
-  [ 'firmware-update.sh',     firmware_update,	  'ndctl' ],
   [ 'ack-shutdown-count-set', ack_shutdown_count, 'ndctl' ],
   [ 'rescan-partitions.sh',   rescan_partitions,  'ndctl' ],
   [ 'inject-smart.sh',        inject_smart,	  'ndctl' ],
@@ -196,6 +195,7 @@ if get_option('destructive').enabled()
   mmap_test = find_program('mmap.sh')
 
   tests += [
+    [ 'firmware-update.sh',     firmware_update,	  'ndctl' ],
     [ 'pmem-ns',           pmem_ns,	   'ndctl' ],
     [ 'sub-section.sh',    sub_section,	   'dax'   ],
     [ 'dax-dev',           dax_dev,	   'dax'   ],

Re: [ndctl PATCH v2 01/18] ndctl/test: Move firmware-update.sh to the 'destructive' set

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:28:03PM -0800, Dan Williams wrote:
> The firmware update test attempts a system-suspend test which may break
> systems that have a broken driver, or otherwise are not prepared to support
> suspend.
> 

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> Link: https://github.com/pmem/ndctl/issues/221
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  test/meson.build |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/meson.build b/test/meson.build
> index 5953c286d13f..c31d8eac66c5 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -170,7 +170,6 @@ tests = [
>    [ 'btt-errors.sh',          btt_errors,	  'ndctl' ],
>    [ 'hugetlb',                hugetlb,		  'ndctl' ],
>    [ 'btt-pad-compat.sh',      btt_pad_compat,	  'ndctl' ],
> -  [ 'firmware-update.sh',     firmware_update,	  'ndctl' ],
>    [ 'ack-shutdown-count-set', ack_shutdown_count, 'ndctl' ],
>    [ 'rescan-partitions.sh',   rescan_partitions,  'ndctl' ],
>    [ 'inject-smart.sh',        inject_smart,	  'ndctl' ],
> @@ -196,6 +195,7 @@ if get_option('destructive').enabled()
>    mmap_test = find_program('mmap.sh')
>  
>    tests += [
> +    [ 'firmware-update.sh',     firmware_update,	  'ndctl' ],
>      [ 'pmem-ns',           pmem_ns,	   'ndctl' ],
>      [ 'sub-section.sh',    sub_section,	   'dax'   ],
>      [ 'dax-dev',           dax_dev,	   'dax'   ],
> 

[ndctl PATCH v2 02/18] ndctl/test: Add kernel backtrace detection to some dax tests

[Dan Williams]

It is useful to fail a test if it triggers a backtrace. Generalize the
mechanism from test/cxl-topology.sh and add it to tests that want
to validate clean kernel logs.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/common              |   10 ++++++++++
 test/cxl-region-sysfs.sh |    4 +---
 test/cxl-topology.sh     |    5 +----
 test/dax.sh              |    2 ++
 test/daxdev-errors.sh    |    2 ++
 test/multi-dax.sh        |    2 ++
 6 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/test/common b/test/common
index 65615cc09a3e..44cc352f6009 100644
--- a/test/common
+++ b/test/common
@@ -132,3 +132,13 @@ json2var()
 {
 	sed -e "s/[{}\",]//g; s/\[//g; s/\]//g; s/:/=/g"
 }
+
+# check_dmesg
+# $1: line number where this is called
+check_dmesg()
+{
+	# validate no WARN or lockdep report during the run
+	log=$(journalctl -r -k --since "-$((SECONDS+1))s")
+	grep -q "Call Trace" <<< $log && err $1
+	true
+}
diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
index 63186b60dfec..e128406cd8c8 100644
--- a/test/cxl-region-sysfs.sh
+++ b/test/cxl-region-sysfs.sh
@@ -164,8 +164,6 @@ readarray -t endpoint < <($CXL free-dpa -t pmem ${mem[*]} |
 			  jq -r ".[] | .decoder.decoder")
 echo "$region released ${#endpoint[@]} targets: ${endpoint[@]}"
 
-# validate no WARN or lockdep report during the run
-log=$(journalctl -r -k --since "-$((SECONDS+1))s")
-grep -q "Call Trace" <<< $log && err "$LINENO"
+check_dmesg "$LINENO"
 
 modprobe -r cxl_test
diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
index f7e390d22680..1f15d29f0600 100644
--- a/test/cxl-topology.sh
+++ b/test/cxl-topology.sh
@@ -169,9 +169,6 @@ done
 # validate that the bus can be disabled without issue
 $CXL disable-bus $root -f
 
-
-# validate no WARN or lockdep report during the run
-log=$(journalctl -r -k --since "-$((SECONDS+1))s")
-grep -q "Call Trace" <<< $log && err "$LINENO"
+check_dmesg "$LINENO"
 
 modprobe -r cxl_test
diff --git a/test/dax.sh b/test/dax.sh
index bb9848b10ecc..3ffbc8079eba 100755
--- a/test/dax.sh
+++ b/test/dax.sh
@@ -118,4 +118,6 @@ else
 	run_xfs
 fi
 
+check_dmesg "$LINENO"
+
 exit 0
diff --git a/test/daxdev-errors.sh b/test/daxdev-errors.sh
index 7f79718113d0..84ef93499acf 100755
--- a/test/daxdev-errors.sh
+++ b/test/daxdev-errors.sh
@@ -71,6 +71,8 @@ if read sector len < /sys/bus/platform/devices/nfit_test.0/$busdev/$region/badbl
 fi
 [ -n "$sector" ] && echo "fail: $LINENO" && exit 1
 
+check_dmesg "$LINENO"
+
 _cleanup
 
 exit 0
diff --git a/test/multi-dax.sh b/test/multi-dax.sh
index 04070adb18e4..d471e1c96b5e 100755
--- a/test/multi-dax.sh
+++ b/test/multi-dax.sh
@@ -28,6 +28,8 @@ chardev1=$(echo $json | jq ". | select(.mode == \"devdax\") | .daxregion.devices
 json=$($NDCTL create-namespace -b $NFIT_TEST_BUS0 -r $region -t pmem -m devdax -a $ALIGN_SIZE -s 16M)
 chardev2=$(echo $json | jq ". | select(.mode == \"devdax\") | .daxregion.devices[0].chardev")
 
+check_dmesg "$LINENO"
+
 _cleanup
 
 exit 0

[ndctl PATCH v2 03/18] ndctl/clang-format: Move minimum version to 6

[Dan Williams]

Follow the kernel change that did the same:

sed -i 's/^\(\s*\)#\(\S*\s\+\S*\) # Unknown to clang-format.*/\1\2/' .clang-format

commit 96232c7d4f84 ("clang-format: Update to clang-format >= 6")

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 .clang-format |   36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/.clang-format b/.clang-format
index b6169e15097c..f372823c3248 100644
--- a/.clang-format
+++ b/.clang-format
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 #
-# clang-format configuration file. Intended for clang-format >= 4.
+# clang-format configuration file. Intended for clang-format >= 6.
 # Copied from Linux's .clang-format
 #
 # For more information, see:
@@ -13,7 +13,7 @@ AccessModifierOffset: -4
 AlignAfterOpenBracket: Align
 AlignConsecutiveAssignments: false
 AlignConsecutiveDeclarations: false
-#AlignEscapedNewlines: Left # Unknown to clang-format-4.0
+AlignEscapedNewlines: Left
 AlignOperands: true
 AlignTrailingComments: false
 AllowAllParametersOfDeclarationOnNextLine: false
@@ -37,24 +37,24 @@ BraceWrapping:
   AfterObjCDeclaration: false
   AfterStruct: false
   AfterUnion: false
-  #AfterExternBlock: false # Unknown to clang-format-5.0
+  AfterExternBlock: false
   BeforeCatch: false
   BeforeElse: false
   IndentBraces: false
-  #SplitEmptyFunction: true # Unknown to clang-format-4.0
-  #SplitEmptyRecord: true # Unknown to clang-format-4.0
-  #SplitEmptyNamespace: true # Unknown to clang-format-4.0
+  SplitEmptyFunction: true
+  SplitEmptyRecord: true
+  SplitEmptyNamespace: true
 BreakBeforeBinaryOperators: None
 BreakBeforeBraces: Custom
-#BreakBeforeInheritanceComma: false # Unknown to clang-format-4.0
+BreakBeforeInheritanceComma: false
 BreakBeforeTernaryOperators: false
 BreakConstructorInitializersBeforeComma: false
-#BreakConstructorInitializers: BeforeComma # Unknown to clang-format-4.0
+BreakConstructorInitializers: BeforeComma
 BreakAfterJavaFieldAnnotations: false
 BreakStringLiterals: false
 ColumnLimit: 80
 CommentPragmas: '^ IWYU pragma:'
-#CompactNamespaces: false # Unknown to clang-format-4.0
+CompactNamespaces: false
 ConstructorInitializerAllOnOneLineOrOnePerLine: false
 ConstructorInitializerIndentWidth: 8
 ContinuationIndentWidth: 8
@@ -62,7 +62,7 @@ Cpp11BracedListStyle: false
 DerivePointerAlignment: false
 DisableFormat: false
 ExperimentalAutoDetectBinPacking: false
-#FixNamespaceComments: false # Unknown to clang-format-4.0
+FixNamespaceComments: false
 
 # Taken from:
 # while read -r sym; do
@@ -118,13 +118,13 @@ ForEachMacros:
   - 'ndctl_region_foreach'
   - 'udev_list_entry_foreach'
 
-#IncludeBlocks: Preserve # Unknown to clang-format-5.0
+IncludeBlocks: Preserve
 IncludeCategories:
   - Regex: '.*'
     Priority: 1
 IncludeIsMainRegex: '(Test)?$'
 IndentCaseLabels: false
-#IndentPPDirectives: None # Unknown to clang-format-5.0
+IndentPPDirectives: None
 IndentWidth: 8
 IndentWrappedFunctionNames: false
 JavaScriptQuotes: Leave
@@ -134,13 +134,13 @@ MacroBlockBegin: ''
 MacroBlockEnd: ''
 MaxEmptyLinesToKeep: 1
 NamespaceIndentation: None
-#ObjCBinPackProtocolList: Auto # Unknown to clang-format-5.0
+ObjCBinPackProtocolList: Auto
 ObjCBlockIndentWidth: 8
 ObjCSpaceAfterProperty: true
 ObjCSpaceBeforeProtocolList: true
 
 # Taken from git's rules
-#PenaltyBreakAssignment: 10 # Unknown to clang-format-4.0
+PenaltyBreakAssignment: 10
 PenaltyBreakBeforeFirstCallParameter: 30
 PenaltyBreakComment: 10
 PenaltyBreakFirstLessLess: 0
@@ -151,14 +151,14 @@ PenaltyReturnTypeOnItsOwnLine: 60
 PointerAlignment: Right
 ReflowComments: false
 SortIncludes: false
-#SortUsingDeclarations: false # Unknown to clang-format-4.0
+SortUsingDeclarations: false
 SpaceAfterCStyleCast: false
 SpaceAfterTemplateKeyword: true
 SpaceBeforeAssignmentOperators: true
-#SpaceBeforeCtorInitializerColon: true # Unknown to clang-format-5.0
-#SpaceBeforeInheritanceColon: true # Unknown to clang-format-5.0
+SpaceBeforeCtorInitializerColon: true
+SpaceBeforeInheritanceColon: true
 SpaceBeforeParens: ControlStatements
-#SpaceBeforeRangeBasedForLoopColon: true # Unknown to clang-format-5.0
+SpaceBeforeRangeBasedForLoopColon: true
 SpaceInEmptyParentheses: false
 SpacesBeforeTrailingComments: 1
 SpacesInAngles: false

[ndctl PATCH v2 04/18] ndctl/clang-format: Fix space after for_each macros

[Dan Williams]

Copy the approach taken in the kernel via:

commit 781121a7f6d1 ("clang-format: Fix space after for_each macros")

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 .clang-format |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.clang-format b/.clang-format
index f372823c3248..448b7e7211ae 100644
--- a/.clang-format
+++ b/.clang-format
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 #
-# clang-format configuration file. Intended for clang-format >= 6.
+# clang-format configuration file. Intended for clang-format >= 11.
 # Copied from Linux's .clang-format
 #
 # For more information, see:
@@ -157,7 +157,7 @@ SpaceAfterTemplateKeyword: true
 SpaceBeforeAssignmentOperators: true
 SpaceBeforeCtorInitializerColon: true
 SpaceBeforeInheritanceColon: true
-SpaceBeforeParens: ControlStatements
+SpaceBeforeParens: ControlStatementsExceptForEachMacros
 SpaceBeforeRangeBasedForLoopColon: true
 SpaceInEmptyParentheses: false
 SpacesBeforeTrailingComments: 1

Re: [ndctl PATCH v2 04/18] ndctl/clang-format: Fix space after for_each macros

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:28:21PM -0800, Dan Williams wrote:
> Copy the approach taken in the kernel via:
> 
> commit 781121a7f6d1 ("clang-format: Fix space after for_each macros")

On a related note - 'cxl_mapping_foreach' seems to be missing from
.clang-format. Perhaps it is in another patch I haven't seen yet.

This patch -
Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  .clang-format |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/.clang-format b/.clang-format
> index f372823c3248..448b7e7211ae 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  #
> -# clang-format configuration file. Intended for clang-format >= 6.
> +# clang-format configuration file. Intended for clang-format >= 11.
>  # Copied from Linux's .clang-format
>  #
>  # For more information, see:
> @@ -157,7 +157,7 @@ SpaceAfterTemplateKeyword: true
>  SpaceBeforeAssignmentOperators: true
>  SpaceBeforeCtorInitializerColon: true
>  SpaceBeforeInheritanceColon: true
> -SpaceBeforeParens: ControlStatements
> +SpaceBeforeParens: ControlStatementsExceptForEachMacros
>  SpaceBeforeRangeBasedForLoopColon: true
>  SpaceInEmptyParentheses: false
>  SpacesBeforeTrailingComments: 1
> 

Re: [ndctl PATCH v2 04/18] ndctl/clang-format: Fix space after for_each macros

[Verma, Vishal L]

On Fri, 2022-12-09 at 09:22 -0800, Alison Schofield wrote:
> On Thu, Dec 08, 2022 at 01:28:21PM -0800, Dan Williams wrote:
> > Copy the approach taken in the kernel via:
> > 
> > commit 781121a7f6d1 ("clang-format: Fix space after for_each
> > macros")
> 
> On a related note - 'cxl_mapping_foreach' seems to be missing from
> .clang-format. Perhaps it is in another patch I haven't seen yet.

Ah good catch - I think that's my miss - I'll send out a patch for that
separately.

> 
> This patch -
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> 
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  .clang-format |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/.clang-format b/.clang-format
> > index f372823c3248..448b7e7211ae 100644
> > --- a/.clang-format
> > +++ b/.clang-format
> > @@ -1,6 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  #
> > -# clang-format configuration file. Intended for clang-format >= 6.
> > +# clang-format configuration file. Intended for clang-format >= 11.
> >  # Copied from Linux's .clang-format
> >  #
> >  # For more information, see:
> > @@ -157,7 +157,7 @@ SpaceAfterTemplateKeyword: true
> >  SpaceBeforeAssignmentOperators: true
> >  SpaceBeforeCtorInitializerColon: true
> >  SpaceBeforeInheritanceColon: true
> > -SpaceBeforeParens: ControlStatements
> > +SpaceBeforeParens: ControlStatementsExceptForEachMacros
> >  SpaceBeforeRangeBasedForLoopColon: true
> >  SpaceInEmptyParentheses: false
> >  SpacesBeforeTrailingComments: 1
> > 

[ndctl PATCH v2 05/18] cxl/list: Always attempt to collect child objects

[Dan Williams]

The evolution of the hierarchical listing left warts like the following:

	if (p->memdevs && !p->ports && !p->endpoints) {
		jchilddevs = json_object_new_array();

...whereby it tried to avoid creating a container for child devices if
another container deeper in the hierarchy might supersede the upper-level
containers. I.e. if endpoints are included in the listing then there will
be nothing to report at the bus level. The protection is unnnecessary
because cond_add_put_array_suffix() already handles the case of dropping
empty containers when a lower level container subsumes all the objects.

Moreover, it's a broken check when adding objects at new levels of the
topology. CXL devices attached to an RCH cause memdevs to appear directly
beneath a bus object, and not an intervening port. So in preparation for
that change, delete all the unnecessary special casing for "jchildobj"
container creation.

Reported-by: Vishal Verma <vishal.l.verma@intel.com>
Fixes: 41d6769393f4 ("cxl/list: Move enabled memdevs underneath their endpoint")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/filter.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cxl/filter.c b/cxl/filter.c
index 56c659965891..040e7deefb3e 100644
--- a/cxl/filter.c
+++ b/cxl/filter.c
@@ -971,7 +971,7 @@ walk_child_ports(struct cxl_port *parent_port, struct cxl_filter_params *p,
 				continue;
 			}
 
-			if (p->memdevs && !p->endpoints) {
+			if (p->memdevs) {
 				jchilddevs = json_object_new_array();
 				if (!jchilddevs) {
 					err(p,
@@ -1151,7 +1151,7 @@ int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *p)
 				}
 			}
 
-			if (p->memdevs && !p->ports && !p->endpoints) {
+			if (p->memdevs) {
 				jchilddevs = json_object_new_array();
 				if (!jchilddevs) {
 					err(p,
@@ -1169,7 +1169,7 @@ int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *p)
 					continue;
 				}
 			}
-			if (p->regions && !p->decoders) {
+			if (p->regions) {
 				jchildregions = json_object_new_array();
 				if (!jchildregions) {
 					err(p,

[ndctl PATCH v2 06/18] cxl/list: Add a 'firmware_node' alias

[Dan Williams]

In preparation for the kernel switching from ACPI0016 devices as the dport
identifier to the corresponding host-bridge as the identifier, add support
for listing the dport firmware_node as an alias, and determining when the
dev_path is the phys_path for a dport or a target.

The code paths that depend on phys_path like cxl_target_maps_memdev() and
cxl_dport_maps_memdev() already have appropriate fallbacks to dev_path.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/json.c         |   20 +++++++++++++++++---
 cxl/lib/libcxl.c   |   31 +++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym |    6 ++++++
 cxl/lib/private.h  |    2 ++
 cxl/libcxl.h       |    2 ++
 5 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/cxl/json.c b/cxl/json.c
index 63c17519aba1..5cff532acb13 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -384,7 +384,7 @@ void util_cxl_dports_append_json(struct json_object *jport,
 
 	cxl_dport_foreach(port, dport) {
 		struct json_object *jdport;
-		const char *phys_node;
+		const char *phys_node, *fw_node;
 
 		if (!util_cxl_dport_filter_by_memdev(dport, ident, serial))
 			continue;
@@ -404,6 +404,13 @@ void util_cxl_dports_append_json(struct json_object *jport,
 				json_object_object_add(jdport, "alias", jobj);
 		}
 
+		fw_node = cxl_dport_get_firmware_node(dport);
+		if (fw_node) {
+			jobj = json_object_new_string(fw_node);
+			if (jobj)
+				json_object_object_add(jdport, "alias", jobj);
+		}
+
 		val = cxl_dport_get_id(dport);
 		jobj = util_json_object_hex(val, flags);
 		if (jobj)
@@ -711,9 +718,9 @@ void util_cxl_targets_append_json(struct json_object *jdecoder,
 		return;
 
 	cxl_target_foreach(decoder, target) {
-		struct json_object *jtarget;
-		const char *phys_node;
 		const char *devname;
+		struct json_object *jtarget;
+		const char *phys_node, *fw_node;
 
 		if (!util_cxl_target_filter_by_memdev(target, ident, serial))
 			continue;
@@ -734,6 +741,13 @@ void util_cxl_targets_append_json(struct json_object *jdecoder,
 				json_object_object_add(jtarget, "alias", jobj);
 		}
 
+		fw_node = cxl_target_get_firmware_node(target);
+		if (fw_node) {
+			jobj = json_object_new_string(fw_node);
+			if (jobj)
+				json_object_object_add(jtarget, "alias", jobj);
+		}
+
 		val = cxl_target_get_position(target);
 		jobj = json_object_new_int(val);
 		if (jobj)
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index e8c5d4444dd0..a69e31bc8a6e 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -77,6 +77,7 @@ static void free_target(struct cxl_target *target, struct list_head *head)
 		list_del_from(head, &target->list);
 	free(target->dev_path);
 	free(target->phys_path);
+	free(target->fw_path);
 	free(target);
 }
 
@@ -134,6 +135,7 @@ static void free_dport(struct cxl_dport *dport, struct list_head *head)
 	free(dport->dev_buf);
 	free(dport->dev_path);
 	free(dport->phys_path);
+	free(dport->fw_path);
 	free(dport);
 }
 
@@ -1856,6 +1858,15 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 		dbg(ctx, "%s: target%ld %s phys_path: %s\n", devname, i,
 		    target->dev_path,
 		    target->phys_path ? target->phys_path : "none");
+
+		sprintf(port->dev_buf, "%s/dport%d/firmware_node", port->dev_path, did);
+		target->fw_path = realpath(port->dev_buf, NULL);
+		dbg(ctx, "%s: target%ld %s fw_path: %s\n", devname, i,
+		    target->dev_path,
+		    target->fw_path ? target->fw_path : "none");
+
+		if (!target->phys_path && target->fw_path)
+			target->phys_path = strdup(target->dev_path);
 		list_add(&decoder->targets, &target->list);
 	}
 
@@ -2288,6 +2299,13 @@ CXL_EXPORT const char *cxl_target_get_physical_node(struct cxl_target *target)
 	return devpath_to_devname(target->phys_path);
 }
 
+CXL_EXPORT const char *cxl_target_get_firmware_node(struct cxl_target *target)
+{
+	if (!target->fw_path)
+		return NULL;
+	return devpath_to_devname(target->fw_path);
+}
+
 CXL_EXPORT struct cxl_target *
 cxl_decoder_get_target_by_memdev(struct cxl_decoder *decoder,
 				 struct cxl_memdev *memdev)
@@ -2569,6 +2587,12 @@ static void *add_cxl_dport(void *parent, int id, const char *cxldport_base)
 	sprintf(dport->dev_buf, "%s/physical_node", cxldport_base);
 	dport->phys_path = realpath(dport->dev_buf, NULL);
 
+	sprintf(dport->dev_buf, "%s/firmware_node", cxldport_base);
+	dport->fw_path = realpath(dport->dev_buf, NULL);
+
+	if (!dport->phys_path && dport->fw_path)
+		dport->phys_path = strdup(dport->dev_path);
+
 	cxl_dport_foreach(port, dport_dup)
 		if (dport_dup->id == dport->id) {
 			free_dport(dport, NULL);
@@ -2629,6 +2653,13 @@ CXL_EXPORT const char *cxl_dport_get_physical_node(struct cxl_dport *dport)
 	return devpath_to_devname(dport->phys_path);
 }
 
+CXL_EXPORT const char *cxl_dport_get_firmware_node(struct cxl_dport *dport)
+{
+	if (!dport->fw_path)
+		return NULL;
+	return devpath_to_devname(dport->fw_path);
+}
+
 CXL_EXPORT int cxl_dport_get_id(struct cxl_dport *dport)
 {
 	return dport->id;
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 8bb91e05638b..490ed1fda5d3 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -217,3 +217,9 @@ global:
 	cxl_decoder_get_max_available_extent;
 	cxl_decoder_get_region;
 } LIBCXL_2;
+
+LIBCXL_4 {
+global:
+	cxl_target_get_firmware_node;
+	cxl_dport_get_firmware_node;
+} LIBCXL_3;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index 437eadeb670a..e378339ec353 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -45,6 +45,7 @@ struct cxl_dport {
 	size_t buf_len;
 	char *dev_path;
 	char *phys_path;
+	char *fw_path;
 	struct cxl_port *port;
 	struct list_node list;
 };
@@ -93,6 +94,7 @@ struct cxl_target {
 	struct cxl_decoder *decoder;
 	char *dev_path;
 	char *phys_path;
+	char *fw_path;
 	int id, position;
 };
 
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 9fe4e99263dd..1e0076908901 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -116,6 +116,7 @@ struct cxl_dport *cxl_dport_get_first(struct cxl_port *port);
 struct cxl_dport *cxl_dport_get_next(struct cxl_dport *dport);
 const char *cxl_dport_get_devname(struct cxl_dport *dport);
 const char *cxl_dport_get_physical_node(struct cxl_dport *dport);
+const char *cxl_dport_get_firmware_node(struct cxl_dport *dport);
 struct cxl_port *cxl_dport_get_port(struct cxl_dport *dport);
 int cxl_dport_get_id(struct cxl_dport *dport);
 bool cxl_dport_maps_memdev(struct cxl_dport *dport, struct cxl_memdev *memdev);
@@ -225,6 +226,7 @@ const char *cxl_target_get_devname(struct cxl_target *target);
 bool cxl_target_maps_memdev(struct cxl_target *target,
 			    struct cxl_memdev *memdev);
 const char *cxl_target_get_physical_node(struct cxl_target *target);
+const char *cxl_target_get_firmware_node(struct cxl_target *target);
 
 #define cxl_target_foreach(decoder, target)                                    \
 	for (target = cxl_target_get_first(decoder); target != NULL;           \

[ndctl PATCH v2 07/18] cxl/list: Add parent_dport attribute to port listings

[Dan Williams]

---
 cxl/json.c         |    8 ++++++++
 cxl/lib/libcxl.c   |   38 ++++++++++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym |    1 +
 cxl/lib/private.h  |    2 ++
 cxl/libcxl.h       |    1 +
 5 files changed, 50 insertions(+)

diff --git a/cxl/json.c b/cxl/json.c
index 5cff532acb13..2f3639ede2f8 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -783,6 +783,14 @@ static struct json_object *__util_cxl_port_to_json(struct cxl_port *port,
 	if (jobj)
 		json_object_object_add(jport, "host", jobj);
 
+	if (cxl_port_get_parent_dport(port)) {
+		struct cxl_dport *dport = cxl_port_get_parent_dport(port);
+
+		jobj = json_object_new_string(cxl_dport_get_devname(dport));
+		if (jobj)
+			json_object_object_add(jport, "parent_dport", jobj);
+	}
+
 	jobj = json_object_new_int(cxl_port_get_depth(port));
 	if (jobj)
 		json_object_object_add(jport, "depth", jobj);
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index a69e31bc8a6e..9475d0e51f8c 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -162,6 +162,7 @@ static void __free_port(struct cxl_port *port, struct list_head *head)
 	free(port->dev_buf);
 	free(port->dev_path);
 	free(port->uport);
+	free(port->parent_dport_path);
 }
 
 static void free_port(struct cxl_port *port, struct list_head *head)
@@ -1488,6 +1489,20 @@ static int cxl_port_init(struct cxl_port *port, struct cxl_port *parent_port,
 	if (!port->uport)
 		goto err;
 
+	/*
+	 * CXL root devices have no parents and level 1 ports are both
+	 * CXL root targets and hosts of the next level, so:
+	 *     parent_dport == uport
+	 * ...at depth == 1
+	 */
+	if (port->depth > 1) {
+		rc = snprintf(port->dev_buf, port->buf_len, "%s/parent_dport",
+			      cxlport_base);
+		if (rc >= port->buf_len)
+			goto err;
+		port->parent_dport_path = realpath(port->dev_buf, NULL);
+	}
+
 	sprintf(path, "%s/modalias", cxlport_base);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		port->module = util_modalias_to_module(ctx, buf);
@@ -2465,6 +2480,29 @@ CXL_EXPORT const char *cxl_port_get_host(struct cxl_port *port)
 	return devpath_to_devname(port->uport);
 }
 
+CXL_EXPORT struct cxl_dport *cxl_port_get_parent_dport(struct cxl_port *port)
+{
+	struct cxl_port *parent;
+	struct cxl_dport *dport;
+	const char *name;
+
+	if (port->parent_dport)
+		return port->parent_dport;
+
+	if (!port->parent_dport_path)
+		return NULL;
+
+	parent = cxl_port_get_parent(port);
+	name = devpath_to_devname(port->parent_dport_path);
+	cxl_dport_foreach(parent, dport)
+		if (strcmp(cxl_dport_get_devname(dport), name) == 0) {
+			port->parent_dport = dport;
+			return dport;
+		}
+
+	return NULL;
+}
+
 CXL_EXPORT bool cxl_port_hosts_memdev(struct cxl_port *port,
 				      struct cxl_memdev *memdev)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 490ed1fda5d3..cc5c1d543484 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -222,4 +222,5 @@ LIBCXL_4 {
 global:
 	cxl_target_get_firmware_node;
 	cxl_dport_get_firmware_node;
+	cxl_port_get_parent_dport;
 } LIBCXL_3;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index e378339ec353..da2fce33cb07 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -62,6 +62,8 @@ struct cxl_port {
 	size_t buf_len;
 	char *dev_path;
 	char *uport;
+	char *parent_dport_path;
+	struct cxl_dport *parent_dport;
 	int ports_init;
 	int endpoints_init;
 	int decoders_init;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 1e0076908901..8d75330886f8 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -96,6 +96,7 @@ bool cxl_port_is_endpoint(struct cxl_port *port);
 struct cxl_endpoint *cxl_port_to_endpoint(struct cxl_port *port);
 struct cxl_bus *cxl_port_get_bus(struct cxl_port *port);
 const char *cxl_port_get_host(struct cxl_port *port);
+struct cxl_dport *cxl_port_get_parent_dport(struct cxl_port *port);
 bool cxl_port_hosts_memdev(struct cxl_port *port, struct cxl_memdev *memdev);
 int cxl_port_get_nr_dports(struct cxl_port *port);
 int cxl_port_disable_invalidate(struct cxl_port *port);

RE: [ndctl PATCH v2 07/18] cxl/list: Add parent_dport attribute to port listings

[Dan Williams]

Dan Williams wrote:
> 
> ---
>  cxl/json.c         |    8 ++++++++
>  cxl/lib/libcxl.c   |   38 ++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym |    1 +
>  cxl/lib/private.h  |    2 ++
>  cxl/libcxl.h       |    1 +
>  5 files changed, 50 insertions(+)

I should not have sent this out. It was sitting in my development tree
for other work, but should have been held back until the kernel support
landed.

Drop this for now as the kernel support will not arrive until v6.3:

http://lore.kernel.org/r/167124082375.1626103.6047000000121298560.stgit@dwillia2-xfh.jf.intel.com

Re: [ndctl PATCH v2 07/18] cxl/list: Add parent_dport attribute to port listings

[Verma, Vishal L]

On Fri, 2022-12-16 at 17:36 -0800, Dan Williams wrote:
> Dan Williams wrote:
> > 
> > ---
> >  cxl/json.c         |    8 ++++++++
> >  cxl/lib/libcxl.c   |   38 ++++++++++++++++++++++++++++++++++++++
> >  cxl/lib/libcxl.sym |    1 +
> >  cxl/lib/private.h  |    2 ++
> >  cxl/libcxl.h       |    1 +
> >  5 files changed, 50 insertions(+)
> 
> I should not have sent this out. It was sitting in my development
> tree
> for other work, but should have been held back until the kernel
> support
> landed.
> 
> Drop this for now as the kernel support will not arrive until v6.3:
> 
> http://lore.kernel.org/r/167124082375.1626103.6047000000121298560.stgit@dwillia2-xfh.jf.intel.com

Ah it has made it out to pending already.. shall I just revert it?

[ndctl PATCH v2 08/18] cxl/list: Skip emitting pmem_size when it is zero

[Dan Williams]

The typical case is that CXL devices are pure ram devices. Only emit
capacity sizes when they are non-zero to avoid confusion around whether
pmem is available via partitioning or not.

The confusion being that a user may assign more meaning to the zero size
value than it actually deserves. A zero value for either pmem or ram,
doesn't indicate the devices capability for either mode.  Use the -I
option to cxl list to include paritition info in the memdev listing.
That will explicitly show the ram and pmem capabilities of the device.

Do the same for ram_size on the odd case that someone builds a pure pmem
device.

Cc: Alison Schofield <alison.schofield@intel.com>
[alison: clarify changelog]
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/cxl/cxl-list.txt |    5 -----
 cxl/json.c                     |   20 +++++++++++++-------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
index 14a2b4bb5c2a..56229abcb053 100644
--- a/Documentation/cxl/cxl-list.txt
+++ b/Documentation/cxl/cxl-list.txt
@@ -70,7 +70,6 @@ configured.
 {
   "memdev":"mem0",
   "pmem_size":"256.00 MiB (268.44 MB)",
-  "ram_size":0,
   "serial":"0",
   "host":"0000:35:00.0"
 }
@@ -88,7 +87,6 @@ EXAMPLE
   {
     "memdev":"mem0",
     "pmem_size":268435456,
-    "ram_size":0,
     "serial":0,
     "host":"0000:35:00.0"
   }
@@ -101,7 +99,6 @@ EXAMPLE
       {
         "memdev":"mem0",
         "pmem_size":"256.00 MiB (268.44 MB)",
-        "ram_size":0,
         "serial":"0"
       }
     ]
@@ -129,7 +126,6 @@ OPTIONS
   {
     "memdev":"mem0",
     "pmem_size":268435456,
-    "ram_size":0,
     "serial":0
   },
   {
@@ -204,7 +200,6 @@ OPTIONS
 [
   {
     "memdev":"mem0",
-    "pmem_size":0,
     "ram_size":273535729664,
     "partition_info":{
       "total_size":273535729664,
diff --git a/cxl/json.c b/cxl/json.c
index 2f3639ede2f8..292e8428ccee 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -305,7 +305,7 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 {
 	const char *devname = cxl_memdev_get_devname(memdev);
 	struct json_object *jdev, *jobj;
-	unsigned long long serial;
+	unsigned long long serial, size;
 	int numa_node;
 
 	jdev = json_object_new_object();
@@ -316,13 +316,19 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 	if (jobj)
 		json_object_object_add(jdev, "memdev", jobj);
 
-	jobj = util_json_object_size(cxl_memdev_get_pmem_size(memdev), flags);
-	if (jobj)
-		json_object_object_add(jdev, "pmem_size", jobj);
+	size = cxl_memdev_get_pmem_size(memdev);
+	if (size) {
+		jobj = util_json_object_size(size, flags);
+		if (jobj)
+			json_object_object_add(jdev, "pmem_size", jobj);
+	}
 
-	jobj = util_json_object_size(cxl_memdev_get_ram_size(memdev), flags);
-	if (jobj)
-		json_object_object_add(jdev, "ram_size", jobj);
+	size = cxl_memdev_get_ram_size(memdev);
+	if (size) {
+		jobj = util_json_object_size(size, flags);
+		if (jobj)
+			json_object_object_add(jdev, "ram_size", jobj);
+	}
 
 	if (flags & UTIL_JSON_HEALTH) {
 		jobj = util_cxl_memdev_health_to_json(memdev, flags);

Re: [ndctl PATCH v2 08/18] cxl/list: Skip emitting pmem_size when it is zero

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:28:45PM -0800, Dan Williams wrote:
> The typical case is that CXL devices are pure ram devices. Only emit
> capacity sizes when they are non-zero to avoid confusion around whether
> pmem is available via partitioning or not.
> 
> The confusion being that a user may assign more meaning to the zero size
> value than it actually deserves. A zero value for either pmem or ram,
> doesn't indicate the devices capability for either mode.  Use the -I
> option to cxl list to include paritition info in the memdev listing.
> That will explicitly show the ram and pmem capabilities of the device.
> 
> Do the same for ram_size on the odd case that someone builds a pure pmem
> device.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> 
> Cc: Alison Schofield <alison.schofield@intel.com>
> [alison: clarify changelog]
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  Documentation/cxl/cxl-list.txt |    5 -----
>  cxl/json.c                     |   20 +++++++++++++-------
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
> index 14a2b4bb5c2a..56229abcb053 100644
> --- a/Documentation/cxl/cxl-list.txt
> +++ b/Documentation/cxl/cxl-list.txt
> @@ -70,7 +70,6 @@ configured.
>  {
>    "memdev":"mem0",
>    "pmem_size":"256.00 MiB (268.44 MB)",
> -  "ram_size":0,
>    "serial":"0",
>    "host":"0000:35:00.0"
>  }
> @@ -88,7 +87,6 @@ EXAMPLE
>    {
>      "memdev":"mem0",
>      "pmem_size":268435456,
> -    "ram_size":0,
>      "serial":0,
>      "host":"0000:35:00.0"
>    }
> @@ -101,7 +99,6 @@ EXAMPLE
>        {
>          "memdev":"mem0",
>          "pmem_size":"256.00 MiB (268.44 MB)",
> -        "ram_size":0,
>          "serial":"0"
>        }
>      ]
> @@ -129,7 +126,6 @@ OPTIONS
>    {
>      "memdev":"mem0",
>      "pmem_size":268435456,
> -    "ram_size":0,
>      "serial":0
>    },
>    {
> @@ -204,7 +200,6 @@ OPTIONS
>  [
>    {
>      "memdev":"mem0",
> -    "pmem_size":0,
>      "ram_size":273535729664,
>      "partition_info":{
>        "total_size":273535729664,
> diff --git a/cxl/json.c b/cxl/json.c
> index 2f3639ede2f8..292e8428ccee 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -305,7 +305,7 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  {
>  	const char *devname = cxl_memdev_get_devname(memdev);
>  	struct json_object *jdev, *jobj;
> -	unsigned long long serial;
> +	unsigned long long serial, size;
>  	int numa_node;
>  
>  	jdev = json_object_new_object();
> @@ -316,13 +316,19 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  	if (jobj)
>  		json_object_object_add(jdev, "memdev", jobj);
>  
> -	jobj = util_json_object_size(cxl_memdev_get_pmem_size(memdev), flags);
> -	if (jobj)
> -		json_object_object_add(jdev, "pmem_size", jobj);
> +	size = cxl_memdev_get_pmem_size(memdev);
> +	if (size) {
> +		jobj = util_json_object_size(size, flags);
> +		if (jobj)
> +			json_object_object_add(jdev, "pmem_size", jobj);
> +	}
>  
> -	jobj = util_json_object_size(cxl_memdev_get_ram_size(memdev), flags);
> -	if (jobj)
> -		json_object_object_add(jdev, "ram_size", jobj);
> +	size = cxl_memdev_get_ram_size(memdev);
> +	if (size) {
> +		jobj = util_json_object_size(size, flags);
> +		if (jobj)
> +			json_object_object_add(jdev, "ram_size", jobj);
> +	}
>  
>  	if (flags & UTIL_JSON_HEALTH) {
>  		jobj = util_cxl_memdev_health_to_json(memdev, flags);
> 

[ndctl PATCH v2 09/18] cxl/filter: Return json-c topology

[Dan Williams]

In preparation for cxl_filter_walk() to be used to collect and publish cxl
objects for other utilities, return the resulting json_object directly.
Move the responsibility of freeing and optionally printing the object to
the caller.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/filter.c |   30 ++++++------------------------
 cxl/filter.h |   22 +++++++++++++++++++++-
 cxl/list.c   |    7 ++++++-
 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/cxl/filter.c b/cxl/filter.c
index 040e7deefb3e..8499450ded01 100644
--- a/cxl/filter.c
+++ b/cxl/filter.c
@@ -672,23 +672,6 @@ util_cxl_decoder_filter_by_region(struct cxl_decoder *decoder,
 	return decoder;
 }
 
-static unsigned long params_to_flags(struct cxl_filter_params *param)
-{
-	unsigned long flags = 0;
-
-	if (param->idle)
-		flags |= UTIL_JSON_IDLE;
-	if (param->human)
-		flags |= UTIL_JSON_HUMAN;
-	if (param->health)
-		flags |= UTIL_JSON_HEALTH;
-	if (param->targets)
-		flags |= UTIL_JSON_TARGETS;
-	if (param->partition)
-		flags |= UTIL_JSON_PARTITION;
-	return flags;
-}
-
 static void splice_array(struct cxl_filter_params *p, struct json_object *jobjs,
 			 struct json_object *platform,
 			 const char *container_name, bool do_container)
@@ -1027,11 +1010,12 @@ walk_children:
 	}
 }
 
-int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *p)
+struct json_object *cxl_filter_walk(struct cxl_ctx *ctx,
+				    struct cxl_filter_params *p)
 {
 	struct json_object *jdevs = NULL, *jbuses = NULL, *jports = NULL;
 	struct json_object *jplatform = json_object_new_array();
-	unsigned long flags = params_to_flags(p);
+	unsigned long flags = cxl_filter_to_flags(p);
 	struct json_object *jportdecoders = NULL;
 	struct json_object *jbusdecoders = NULL;
 	struct json_object *jepdecoders = NULL;
@@ -1044,7 +1028,7 @@ int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *p)
 
 	if (!jplatform) {
 		dbg(p, "platform object allocation failure\n");
-		return -ENOMEM;
+		return NULL;
 	}
 
 	janondevs = json_object_new_array();
@@ -1232,9 +1216,7 @@ walk_children:
 		     top_level_objs > 1);
 	splice_array(p, jregions, jplatform, "regions", top_level_objs > 1);
 
-	util_display_json_array(stdout, jplatform, flags);
-
-	return 0;
+	return jplatform;
 err:
 	json_object_put(janondevs);
 	json_object_put(jbuses);
@@ -1246,5 +1228,5 @@ err:
 	json_object_put(jepdecoders);
 	json_object_put(jregions);
 	json_object_put(jplatform);
-	return -ENOMEM;
+	return NULL;
 }
diff --git a/cxl/filter.h b/cxl/filter.h
index 256df49c3d0c..2bda6ddd77ca 100644
--- a/cxl/filter.h
+++ b/cxl/filter.h
@@ -5,6 +5,7 @@
 
 #include <stdbool.h>
 #include <util/log.h>
+#include <util/json.h>
 
 struct cxl_filter_params {
 	const char *memdev_filter;
@@ -59,6 +60,25 @@ struct cxl_dport *util_cxl_dport_filter_by_memdev(struct cxl_dport *dport,
 						  const char *serial);
 struct cxl_decoder *util_cxl_decoder_filter(struct cxl_decoder *decoder,
 					    const char *__ident);
-int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *param);
+struct json_object *cxl_filter_walk(struct cxl_ctx *ctx,
+				    struct cxl_filter_params *param);
+
+static inline unsigned long cxl_filter_to_flags(struct cxl_filter_params *param)
+{
+	unsigned long flags = 0;
+
+	if (param->idle)
+		flags |= UTIL_JSON_IDLE;
+	if (param->human)
+		flags |= UTIL_JSON_HUMAN;
+	if (param->health)
+		flags |= UTIL_JSON_HEALTH;
+	if (param->targets)
+		flags |= UTIL_JSON_TARGETS;
+	if (param->partition)
+		flags |= UTIL_JSON_PARTITION;
+	return flags;
+}
+
 bool cxl_filter_has(const char *needle, const char *__filter);
 #endif /* _CXL_UTIL_FILTER_H_ */
diff --git a/cxl/list.c b/cxl/list.c
index 8c48fbbaaec3..2026de2b548b 100644
--- a/cxl/list.c
+++ b/cxl/list.c
@@ -72,6 +72,7 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
 		"cxl list [<options>]",
 		NULL
 	};
+	struct json_object *jtopology;
 	int i;
 
 	argc = parse_options(argc, argv, options, u, 0);
@@ -140,5 +141,9 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
 		param.endpoints = true;
 
 	dbg(&param, "walk topology\n");
-	return cxl_filter_walk(ctx, &param);
+	jtopology = cxl_filter_walk(ctx, &param);
+	if (!jtopology)
+		return -ENOMEM;
+	util_display_json_array(stdout, jtopology, cxl_filter_to_flags(&param));
+	return 0;
 }

Re: [ndctl PATCH v2 09/18] cxl/filter: Return json-c topology

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:28:50PM -0800, Dan Williams wrote:
> In preparation for cxl_filter_walk() to be used to collect and publish cxl
> objects for other utilities, return the resulting json_object directly.
> Move the responsibility of freeing and optionally printing the object to
> the caller.

Tested-by: Alison Schofield <alison.schofield@intel.com>

> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  cxl/filter.c |   30 ++++++------------------------
>  cxl/filter.h |   22 +++++++++++++++++++++-
>  cxl/list.c   |    7 ++++++-
>  3 files changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/cxl/filter.c b/cxl/filter.c
> index 040e7deefb3e..8499450ded01 100644
> --- a/cxl/filter.c
> +++ b/cxl/filter.c
> @@ -672,23 +672,6 @@ util_cxl_decoder_filter_by_region(struct cxl_decoder *decoder,
>  	return decoder;
>  }
>  
> -static unsigned long params_to_flags(struct cxl_filter_params *param)
> -{
> -	unsigned long flags = 0;
> -
> -	if (param->idle)
> -		flags |= UTIL_JSON_IDLE;
> -	if (param->human)
> -		flags |= UTIL_JSON_HUMAN;
> -	if (param->health)
> -		flags |= UTIL_JSON_HEALTH;
> -	if (param->targets)
> -		flags |= UTIL_JSON_TARGETS;
> -	if (param->partition)
> -		flags |= UTIL_JSON_PARTITION;
> -	return flags;
> -}
> -
>  static void splice_array(struct cxl_filter_params *p, struct json_object *jobjs,
>  			 struct json_object *platform,
>  			 const char *container_name, bool do_container)
> @@ -1027,11 +1010,12 @@ walk_children:
>  	}
>  }
>  
> -int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *p)
> +struct json_object *cxl_filter_walk(struct cxl_ctx *ctx,
> +				    struct cxl_filter_params *p)
>  {
>  	struct json_object *jdevs = NULL, *jbuses = NULL, *jports = NULL;
>  	struct json_object *jplatform = json_object_new_array();
> -	unsigned long flags = params_to_flags(p);
> +	unsigned long flags = cxl_filter_to_flags(p);
>  	struct json_object *jportdecoders = NULL;
>  	struct json_object *jbusdecoders = NULL;
>  	struct json_object *jepdecoders = NULL;
> @@ -1044,7 +1028,7 @@ int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *p)
>  
>  	if (!jplatform) {
>  		dbg(p, "platform object allocation failure\n");
> -		return -ENOMEM;
> +		return NULL;
>  	}
>  
>  	janondevs = json_object_new_array();
> @@ -1232,9 +1216,7 @@ walk_children:
>  		     top_level_objs > 1);
>  	splice_array(p, jregions, jplatform, "regions", top_level_objs > 1);
>  
> -	util_display_json_array(stdout, jplatform, flags);
> -
> -	return 0;
> +	return jplatform;
>  err:
>  	json_object_put(janondevs);
>  	json_object_put(jbuses);
> @@ -1246,5 +1228,5 @@ err:
>  	json_object_put(jepdecoders);
>  	json_object_put(jregions);
>  	json_object_put(jplatform);
> -	return -ENOMEM;
> +	return NULL;
>  }
> diff --git a/cxl/filter.h b/cxl/filter.h
> index 256df49c3d0c..2bda6ddd77ca 100644
> --- a/cxl/filter.h
> +++ b/cxl/filter.h
> @@ -5,6 +5,7 @@
>  
>  #include <stdbool.h>
>  #include <util/log.h>
> +#include <util/json.h>
>  
>  struct cxl_filter_params {
>  	const char *memdev_filter;
> @@ -59,6 +60,25 @@ struct cxl_dport *util_cxl_dport_filter_by_memdev(struct cxl_dport *dport,
>  						  const char *serial);
>  struct cxl_decoder *util_cxl_decoder_filter(struct cxl_decoder *decoder,
>  					    const char *__ident);
> -int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *param);
> +struct json_object *cxl_filter_walk(struct cxl_ctx *ctx,
> +				    struct cxl_filter_params *param);
> +
> +static inline unsigned long cxl_filter_to_flags(struct cxl_filter_params *param)
> +{
> +	unsigned long flags = 0;
> +
> +	if (param->idle)
> +		flags |= UTIL_JSON_IDLE;
> +	if (param->human)
> +		flags |= UTIL_JSON_HUMAN;
> +	if (param->health)
> +		flags |= UTIL_JSON_HEALTH;
> +	if (param->targets)
> +		flags |= UTIL_JSON_TARGETS;
> +	if (param->partition)
> +		flags |= UTIL_JSON_PARTITION;
> +	return flags;
> +}
> +
>  bool cxl_filter_has(const char *needle, const char *__filter);
>  #endif /* _CXL_UTIL_FILTER_H_ */
> diff --git a/cxl/list.c b/cxl/list.c
> index 8c48fbbaaec3..2026de2b548b 100644
> --- a/cxl/list.c
> +++ b/cxl/list.c
> @@ -72,6 +72,7 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
>  		"cxl list [<options>]",
>  		NULL
>  	};
> +	struct json_object *jtopology;
>  	int i;
>  
>  	argc = parse_options(argc, argv, options, u, 0);
> @@ -140,5 +141,9 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
>  		param.endpoints = true;
>  
>  	dbg(&param, "walk topology\n");
> -	return cxl_filter_walk(ctx, &param);
> +	jtopology = cxl_filter_walk(ctx, &param);
> +	if (!jtopology)
> +		return -ENOMEM;
> +	util_display_json_array(stdout, jtopology, cxl_filter_to_flags(&param));
> +	return 0;
>  }
> 

[ndctl PATCH v2 10/18] cxl/list: Record cxl objects in json objects

[Dan Williams]

In preparation for reusing 'cxl list' object selection in other utilities,
like 'cxl create-region', record the associated cxl object in the json
object. For example, enable 'cxl create-region -d decoderX.Y' to lookup the
memdevs that result from 'cxl list -M -d decoderX.Y'.

This sets up future design decisions for code that wants to walk the
topology. It can either open-code its own object walk, or get the json-c
representation of a query and use that. Unless the use case knows exactly
the object it wants it is likely more powerful to specify a
cxl_filter_walk() query and then walk the topology result.

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

diff --git a/cxl/json.c b/cxl/json.c
index 292e8428ccee..844bc089a4b7 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -365,6 +365,8 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 		if (jobj)
 			json_object_object_add(jdev, "partition_info", jobj);
 	}
+
+	json_object_set_userdata(jdev, memdev, NULL);
 	return jdev;
 }
 
@@ -423,6 +425,7 @@ void util_cxl_dports_append_json(struct json_object *jport,
 			json_object_object_add(jdport, "id", jobj);
 
 		json_object_array_add(jdports, jdport);
+		json_object_set_userdata(jdport, dport, NULL);
 	}
 
 	json_object_object_add(jport, "dports", jdports);
@@ -446,6 +449,7 @@ struct json_object *util_cxl_bus_to_json(struct cxl_bus *bus,
 	if (jobj)
 		json_object_object_add(jbus, "provider", jobj);
 
+	json_object_set_userdata(jbus, bus, NULL);
 	return jbus;
 }
 
@@ -570,6 +574,7 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 					       jobj);
 	}
 
+	json_object_set_userdata(jdecoder, decoder, NULL);
 	return jdecoder;
 }
 
@@ -628,6 +633,7 @@ void util_cxl_mappings_append_json(struct json_object *jregion,
 			json_object_object_add(jmapping, "decoder", jobj);
 
 		json_object_array_add(jmappings, jmapping);
+		json_object_set_userdata(jmapping, mapping, NULL);
 	}
 
 	json_object_object_add(jregion, "mappings", jmappings);
@@ -693,6 +699,7 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
 
 	util_cxl_mappings_append_json(jregion, region, flags);
 
+	json_object_set_userdata(jregion, region, NULL);
 	return jregion;
 }
 
@@ -765,6 +772,7 @@ void util_cxl_targets_append_json(struct json_object *jdecoder,
 			json_object_object_add(jtarget, "id", jobj);
 
 		json_object_array_add(jtargets, jtarget);
+		json_object_set_userdata(jtarget, target, NULL);
 	}
 
 	json_object_object_add(jdecoder, "targets", jtargets);
@@ -807,6 +815,7 @@ static struct json_object *__util_cxl_port_to_json(struct cxl_port *port,
 			json_object_object_add(jport, "state", jobj);
 	}
 
+	json_object_set_userdata(jport, port, NULL);
 	return jport;
 }
 

Re: [ndctl PATCH v2 10/18] cxl/list: Record cxl objects in json objects

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:28:57PM -0800, Dan Williams wrote:
> In preparation for reusing 'cxl list' object selection in other utilities,
> like 'cxl create-region', record the associated cxl object in the json
> object. For example, enable 'cxl create-region -d decoderX.Y' to lookup the
> memdevs that result from 'cxl list -M -d decoderX.Y'.
> 
> This sets up future design decisions for code that wants to walk the
> topology. It can either open-code its own object walk, or get the json-c
> representation of a query and use that. Unless the use case knows exactly
> the object it wants it is likely more powerful to specify a
> cxl_filter_walk() query and then walk the topology result.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  cxl/json.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index 292e8428ccee..844bc089a4b7 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -365,6 +365,8 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		if (jobj)
>  			json_object_object_add(jdev, "partition_info", jobj);
>  	}
> +
> +	json_object_set_userdata(jdev, memdev, NULL);
>  	return jdev;
>  }
>  
> @@ -423,6 +425,7 @@ void util_cxl_dports_append_json(struct json_object *jport,
>  			json_object_object_add(jdport, "id", jobj);
>  
>  		json_object_array_add(jdports, jdport);
> +		json_object_set_userdata(jdport, dport, NULL);
>  	}
>  
>  	json_object_object_add(jport, "dports", jdports);
> @@ -446,6 +449,7 @@ struct json_object *util_cxl_bus_to_json(struct cxl_bus *bus,
>  	if (jobj)
>  		json_object_object_add(jbus, "provider", jobj);
>  
> +	json_object_set_userdata(jbus, bus, NULL);
>  	return jbus;
>  }
>  
> @@ -570,6 +574,7 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
>  					       jobj);
>  	}
>  
> +	json_object_set_userdata(jdecoder, decoder, NULL);
>  	return jdecoder;
>  }
>  
> @@ -628,6 +633,7 @@ void util_cxl_mappings_append_json(struct json_object *jregion,
>  			json_object_object_add(jmapping, "decoder", jobj);
>  
>  		json_object_array_add(jmappings, jmapping);
> +		json_object_set_userdata(jmapping, mapping, NULL);
>  	}
>  
>  	json_object_object_add(jregion, "mappings", jmappings);
> @@ -693,6 +699,7 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>  
>  	util_cxl_mappings_append_json(jregion, region, flags);
>  
> +	json_object_set_userdata(jregion, region, NULL);
>  	return jregion;
>  }
>  
> @@ -765,6 +772,7 @@ void util_cxl_targets_append_json(struct json_object *jdecoder,
>  			json_object_object_add(jtarget, "id", jobj);
>  
>  		json_object_array_add(jtargets, jtarget);
> +		json_object_set_userdata(jtarget, target, NULL);
>  	}
>  
>  	json_object_object_add(jdecoder, "targets", jtargets);
> @@ -807,6 +815,7 @@ static struct json_object *__util_cxl_port_to_json(struct cxl_port *port,
>  			json_object_object_add(jport, "state", jobj);
>  	}
>  
> +	json_object_set_userdata(jport, port, NULL);
>  	return jport;
>  }
>  
> 

[ndctl PATCH v2 11/18] cxl/region: Make ways an integer argument

[Dan Williams]

Since --ways does not take a unit value like --size, just make it an
integer argument directly and skip the hand coded conversion.

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

diff --git a/cxl/region.c b/cxl/region.c
index 334fcc291de7..494da5139c05 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -21,21 +21,23 @@
 static struct region_params {
 	const char *bus;
 	const char *size;
-	const char *ways;
 	const char *granularity;
 	const char *type;
 	const char *root_decoder;
 	const char *region;
+	int ways;
 	bool memdevs;
 	bool force;
 	bool human;
 	bool debug;
-} param;
+} param = {
+	.ways = INT_MAX,
+};
 
 struct parsed_params {
 	u64 size;
 	u64 ep_min_size;
-	unsigned int ways;
+	int ways;
 	unsigned int granularity;
 	const char **targets;
 	int num_targets;
@@ -63,9 +65,8 @@ OPT_BOOLEAN(0, "debug", &param.debug, "turn on debug")
 OPT_STRING('s', "size", &param.size, \
 	   "size in bytes or with a K/M/G etc. suffix", \
 	   "total size desired for the resulting region."), \
-OPT_STRING('w', "ways", &param.ways, \
-	   "number of interleave ways", \
-	   "number of memdevs participating in the regions interleave set"), \
+OPT_INTEGER('w', "ways", &param.ways, \
+	    "number of memdevs participating in the regions interleave set"), \
 OPT_STRING('g', "granularity", \
 	   &param.granularity, "interleave granularity", \
 	   "granularity of the interleave set"), \
@@ -126,15 +127,11 @@ static int parse_create_options(int argc, const char **argv,
 		}
 	}
 
-	if (param.ways) {
-		unsigned long ways = strtoul(param.ways, NULL, 0);
-
-		if (ways == ULONG_MAX || (int)ways <= 0) {
-			log_err(&rl, "Invalid interleave ways: %s\n",
-				param.ways);
-			return -EINVAL;
-		}
-		p->ways = ways;
+	if (param.ways <= 0) {
+		log_err(&rl, "Invalid interleave ways: %d\n", param.ways);
+		return -EINVAL;
+	} else if (param.ways < INT_MAX) {
+		p->ways = param.ways;
 	} else if (argc) {
 		p->ways = argc;
 	} else {
@@ -155,13 +152,13 @@ static int parse_create_options(int argc, const char **argv,
 	}
 
 
-	if (argc > (int)p->ways) {
+	if (argc > p->ways) {
 		for (i = p->ways; i < argc; i++)
 			log_err(&rl, "extra argument: %s\n", p->targets[i]);
 		return -EINVAL;
 	}
 
-	if (argc < (int)p->ways) {
+	if (argc < p->ways) {
 		log_err(&rl,
 			"too few target arguments (%d) for interleave ways (%u)\n",
 			argc, p->ways);
@@ -253,7 +250,7 @@ static bool validate_memdev(struct cxl_memdev *memdev, const char *target,
 
 static int validate_config_memdevs(struct cxl_ctx *ctx, struct parsed_params *p)
 {
-	unsigned int i, matched = 0;
+	int i, matched = 0;
 
 	for (i = 0; i < p->ways; i++) {
 		struct cxl_memdev *memdev;
@@ -393,7 +390,8 @@ static int cxl_region_determine_granularity(struct cxl_region *region,
 					    struct parsed_params *p)
 {
 	const char *devname = cxl_region_get_devname(region);
-	unsigned int granularity, ways;
+	unsigned int granularity;
+	int ways;
 
 	/* Default granularity will be the root decoder's granularity */
 	granularity = cxl_decoder_get_interleave_granularity(p->root_decoder);
@@ -408,7 +406,7 @@ static int cxl_region_determine_granularity(struct cxl_region *region,
 		return granularity;
 
 	ways = cxl_decoder_get_interleave_ways(p->root_decoder);
-	if (ways == 0 || ways == UINT_MAX) {
+	if (ways == 0 || ways == -1) {
 		log_err(&rl, "%s: unable to determine root decoder ways\n",
 			devname);
 		return -ENXIO;
@@ -436,12 +434,11 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 {
 	unsigned long flags = UTIL_JSON_TARGETS;
 	struct json_object *jregion;
-	unsigned int i, granularity;
 	struct cxl_region *region;
+	int i, rc, granularity;
 	u64 size, max_extent;
 	const char *devname;
 	uuid_t uuid;
-	int rc;
 
 	rc = create_region_validate_config(ctx, p);
 	if (rc)

Re: [ndctl PATCH v2 11/18] cxl/region: Make ways an integer argument

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:29:03PM -0800, Dan Williams wrote:
> Since --ways does not take a unit value like --size, just make it an
> integer argument directly and skip the hand coded conversion.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  cxl/region.c |   41 +++++++++++++++++++----------------------
>  1 file changed, 19 insertions(+), 22 deletions(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 334fcc291de7..494da5139c05 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -21,21 +21,23 @@
>  static struct region_params {
>  	const char *bus;
>  	const char *size;
> -	const char *ways;
>  	const char *granularity;
>  	const char *type;
>  	const char *root_decoder;
>  	const char *region;
> +	int ways;
>  	bool memdevs;
>  	bool force;
>  	bool human;
>  	bool debug;
> -} param;
> +} param = {
> +	.ways = INT_MAX,
> +};
>  
>  struct parsed_params {
>  	u64 size;
>  	u64 ep_min_size;
> -	unsigned int ways;
> +	int ways;
>  	unsigned int granularity;
>  	const char **targets;
>  	int num_targets;
> @@ -63,9 +65,8 @@ OPT_BOOLEAN(0, "debug", &param.debug, "turn on debug")
>  OPT_STRING('s', "size", &param.size, \
>  	   "size in bytes or with a K/M/G etc. suffix", \
>  	   "total size desired for the resulting region."), \
> -OPT_STRING('w', "ways", &param.ways, \
> -	   "number of interleave ways", \
> -	   "number of memdevs participating in the regions interleave set"), \
> +OPT_INTEGER('w', "ways", &param.ways, \
> +	    "number of memdevs participating in the regions interleave set"), \
>  OPT_STRING('g', "granularity", \
>  	   &param.granularity, "interleave granularity", \
>  	   "granularity of the interleave set"), \
> @@ -126,15 +127,11 @@ static int parse_create_options(int argc, const char **argv,
>  		}
>  	}
>  
> -	if (param.ways) {
> -		unsigned long ways = strtoul(param.ways, NULL, 0);
> -
> -		if (ways == ULONG_MAX || (int)ways <= 0) {
> -			log_err(&rl, "Invalid interleave ways: %s\n",
> -				param.ways);
> -			return -EINVAL;
> -		}
> -		p->ways = ways;
> +	if (param.ways <= 0) {
> +		log_err(&rl, "Invalid interleave ways: %d\n", param.ways);
> +		return -EINVAL;
> +	} else if (param.ways < INT_MAX) {
> +		p->ways = param.ways;
>  	} else if (argc) {
>  		p->ways = argc;
>  	} else {
> @@ -155,13 +152,13 @@ static int parse_create_options(int argc, const char **argv,
>  	}
>  
>  
> -	if (argc > (int)p->ways) {
> +	if (argc > p->ways) {
>  		for (i = p->ways; i < argc; i++)
>  			log_err(&rl, "extra argument: %s\n", p->targets[i]);
>  		return -EINVAL;
>  	}
>  
> -	if (argc < (int)p->ways) {
> +	if (argc < p->ways) {
>  		log_err(&rl,
>  			"too few target arguments (%d) for interleave ways (%u)\n",
>  			argc, p->ways);
> @@ -253,7 +250,7 @@ static bool validate_memdev(struct cxl_memdev *memdev, const char *target,
>  
>  static int validate_config_memdevs(struct cxl_ctx *ctx, struct parsed_params *p)
>  {
> -	unsigned int i, matched = 0;
> +	int i, matched = 0;
>  
>  	for (i = 0; i < p->ways; i++) {
>  		struct cxl_memdev *memdev;
> @@ -393,7 +390,8 @@ static int cxl_region_determine_granularity(struct cxl_region *region,
>  					    struct parsed_params *p)
>  {
>  	const char *devname = cxl_region_get_devname(region);
> -	unsigned int granularity, ways;
> +	unsigned int granularity;
> +	int ways;
>  
>  	/* Default granularity will be the root decoder's granularity */
>  	granularity = cxl_decoder_get_interleave_granularity(p->root_decoder);
> @@ -408,7 +406,7 @@ static int cxl_region_determine_granularity(struct cxl_region *region,
>  		return granularity;
>  
>  	ways = cxl_decoder_get_interleave_ways(p->root_decoder);
> -	if (ways == 0 || ways == UINT_MAX) {
> +	if (ways == 0 || ways == -1) {
>  		log_err(&rl, "%s: unable to determine root decoder ways\n",
>  			devname);
>  		return -ENXIO;
> @@ -436,12 +434,11 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  {
>  	unsigned long flags = UTIL_JSON_TARGETS;
>  	struct json_object *jregion;
> -	unsigned int i, granularity;
>  	struct cxl_region *region;
> +	int i, rc, granularity;
>  	u64 size, max_extent;
>  	const char *devname;
>  	uuid_t uuid;
> -	int rc;
>  
>  	rc = create_region_validate_config(ctx, p);
>  	if (rc)
> 

[ndctl PATCH v2 12/18] cxl/region: Make granularity an integer argument

[Dan Williams]

Since --granularity does not take a unit value like --size, just make it an
integer argument directly and skip the hand coded conversion.

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

diff --git a/cxl/region.c b/cxl/region.c
index 494da5139c05..c6d7d1a973a8 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -21,24 +21,25 @@
 static struct region_params {
 	const char *bus;
 	const char *size;
-	const char *granularity;
 	const char *type;
 	const char *root_decoder;
 	const char *region;
 	int ways;
+	int granularity;
 	bool memdevs;
 	bool force;
 	bool human;
 	bool debug;
 } param = {
 	.ways = INT_MAX,
+	.granularity = INT_MAX,
 };
 
 struct parsed_params {
 	u64 size;
 	u64 ep_min_size;
 	int ways;
-	unsigned int granularity;
+	int granularity;
 	const char **targets;
 	int num_targets;
 	struct cxl_decoder *root_decoder;
@@ -67,9 +68,8 @@ OPT_STRING('s', "size", &param.size, \
 	   "total size desired for the resulting region."), \
 OPT_INTEGER('w', "ways", &param.ways, \
 	    "number of memdevs participating in the regions interleave set"), \
-OPT_STRING('g', "granularity", \
-	   &param.granularity, "interleave granularity", \
-	   "granularity of the interleave set"), \
+OPT_INTEGER('g', "granularity", &param.granularity,  \
+	    "granularity of the interleave set"), \
 OPT_STRING('t', "type", &param.type, \
 	   "region type", "region type - 'pmem' or 'ram'"), \
 OPT_BOOLEAN('m', "memdevs", &param.memdevs, \
@@ -140,18 +140,15 @@ static int parse_create_options(int argc, const char **argv,
 		return -EINVAL;
 	}
 
-	if (param.granularity) {
-		unsigned long granularity = strtoul(param.granularity, NULL, 0);
-
-		if (granularity == ULONG_MAX || (int)granularity <= 0) {
-			log_err(&rl, "Invalid interleave granularity: %s\n",
+	if (param.granularity < INT_MAX) {
+		if (param.granularity <= 0) {
+			log_err(&rl, "Invalid interleave granularity: %d\n",
 				param.granularity);
 			return -EINVAL;
 		}
-		p->granularity = granularity;
+		p->granularity = param.granularity;
 	}
 
-
 	if (argc > p->ways) {
 		for (i = p->ways; i < argc; i++)
 			log_err(&rl, "extra argument: %s\n", p->targets[i]);
@@ -390,12 +387,11 @@ static int cxl_region_determine_granularity(struct cxl_region *region,
 					    struct parsed_params *p)
 {
 	const char *devname = cxl_region_get_devname(region);
-	unsigned int granularity;
-	int ways;
+	int granularity, ways;
 
 	/* Default granularity will be the root decoder's granularity */
 	granularity = cxl_decoder_get_interleave_granularity(p->root_decoder);
-	if (granularity == 0 || granularity == UINT_MAX) {
+	if (granularity == 0 || granularity == -1) {
 		log_err(&rl, "%s: unable to determine root decoder granularity\n",
 			devname);
 		return -ENXIO;

Re: [ndctl PATCH v2 12/18] cxl/region: Make granularity an integer argument

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:29:08PM -0800, Dan Williams wrote:
> Since --granularity does not take a unit value like --size, just make it an
> integer argument directly and skip the hand coded conversion.
> 

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  cxl/region.c |   26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 494da5139c05..c6d7d1a973a8 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -21,24 +21,25 @@
>  static struct region_params {
>  	const char *bus;
>  	const char *size;
> -	const char *granularity;
>  	const char *type;
>  	const char *root_decoder;
>  	const char *region;
>  	int ways;
> +	int granularity;
>  	bool memdevs;
>  	bool force;
>  	bool human;
>  	bool debug;
>  } param = {
>  	.ways = INT_MAX,
> +	.granularity = INT_MAX,
>  };
>  
>  struct parsed_params {
>  	u64 size;
>  	u64 ep_min_size;
>  	int ways;
> -	unsigned int granularity;
> +	int granularity;
>  	const char **targets;
>  	int num_targets;
>  	struct cxl_decoder *root_decoder;
> @@ -67,9 +68,8 @@ OPT_STRING('s', "size", &param.size, \
>  	   "total size desired for the resulting region."), \
>  OPT_INTEGER('w', "ways", &param.ways, \
>  	    "number of memdevs participating in the regions interleave set"), \
> -OPT_STRING('g', "granularity", \
> -	   &param.granularity, "interleave granularity", \
> -	   "granularity of the interleave set"), \
> +OPT_INTEGER('g', "granularity", &param.granularity,  \
> +	    "granularity of the interleave set"), \
>  OPT_STRING('t', "type", &param.type, \
>  	   "region type", "region type - 'pmem' or 'ram'"), \
>  OPT_BOOLEAN('m', "memdevs", &param.memdevs, \
> @@ -140,18 +140,15 @@ static int parse_create_options(int argc, const char **argv,
>  		return -EINVAL;
>  	}
>  
> -	if (param.granularity) {
> -		unsigned long granularity = strtoul(param.granularity, NULL, 0);
> -
> -		if (granularity == ULONG_MAX || (int)granularity <= 0) {
> -			log_err(&rl, "Invalid interleave granularity: %s\n",
> +	if (param.granularity < INT_MAX) {
> +		if (param.granularity <= 0) {
> +			log_err(&rl, "Invalid interleave granularity: %d\n",
>  				param.granularity);
>  			return -EINVAL;
>  		}
> -		p->granularity = granularity;
> +		p->granularity = param.granularity;
>  	}
>  
> -
>  	if (argc > p->ways) {
>  		for (i = p->ways; i < argc; i++)
>  			log_err(&rl, "extra argument: %s\n", p->targets[i]);
> @@ -390,12 +387,11 @@ static int cxl_region_determine_granularity(struct cxl_region *region,
>  					    struct parsed_params *p)
>  {
>  	const char *devname = cxl_region_get_devname(region);
> -	unsigned int granularity;
> -	int ways;
> +	int granularity, ways;
>  
>  	/* Default granularity will be the root decoder's granularity */
>  	granularity = cxl_decoder_get_interleave_granularity(p->root_decoder);
> -	if (granularity == 0 || granularity == UINT_MAX) {
> +	if (granularity == 0 || granularity == -1) {
>  		log_err(&rl, "%s: unable to determine root decoder granularity\n",
>  			devname);
>  		return -ENXIO;
> 

[ndctl PATCH v2 13/18] cxl/region: Use cxl_filter_walk() to gather create-region targets

[Dan Williams]

The core of 'cxl list' knows, among other things, how to filter memdevs by
their connectivity to a root decoder, enabled status, and how to identify
memdevs by name, id, serial number. Use the fact that the json-c object
array returned by cxl_filter_walk() also includes the corresponding libcxl
objects to populate and validate the memdev target list for 'cxl
create-region'.

With this in place a default set of memdev targets can be derived from the
specified root decoder, and the connectivity is validated by the same logic
that prepares the hierarchical json topology. The argument list becomes as
tolerant of different id formats as 'cxl list'. For example "mem9" and "9"
are equivalent. Comma and space separated lists are also allowed, e.g.
"mem9,mem10".

Note that 'cxl list' order groups memdevs by port, later this will need to
augmented with a sort implementation that orders memdevs by a topology
compatible decode order.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/region.c |  334 +++++++++++++++++++++++++++++++++++++++-------------------
 util/util.h  |    9 ++
 2 files changed, 234 insertions(+), 109 deletions(-)

diff --git a/cxl/region.c b/cxl/region.c
index c6d7d1a973a8..36ebc8e5210f 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -40,8 +40,10 @@ struct parsed_params {
 	u64 ep_min_size;
 	int ways;
 	int granularity;
-	const char **targets;
-	int num_targets;
+	struct json_object *memdevs;
+	int num_memdevs;
+	int argc;
+	const char **argv;
 	struct cxl_decoder *root_decoder;
 	enum cxl_decoder_mode mode;
 };
@@ -99,16 +101,190 @@ static const struct option destroy_options[] = {
 	OPT_END(),
 };
 
-static int parse_create_options(int argc, const char **argv,
-				struct parsed_params *p)
+/*
+ * Convert an array of strings that can be a mixture of single items, a
+ * command separted list, or a space separated list, into a flattened
+ * comma-separated string. That single string can then be used as a
+ * filter argument to cxl_filter_walk(), or an ordering constraint for
+ * json_object_array_sort()
+ *
+ * On entry @count is the number of elements in the strings array, on
+ * exit, @count is the number of elements in the csv.
+ */
+static const char *to_csv(int *count, const char **strings)
 {
-	int i;
+	ssize_t len = 0, cursor = 0;
+	char *csv, *list, *save;
+	int i, new_count = 0;
+	const char *arg;
+
+	if (!*count)
+		return NULL;
+
+	for (i = 0; i < *count; i++) {
+		/*
+		 * An entry in strings may itself by a space or comma
+		 * separated list, so decompose that for the final csv
+		 */
+		list = strdup(strings[i]);
+		if (!list)
+			return NULL;
+
+		for (arg = strtok_r(list, which_sep(list), &save); arg;
+		     arg = strtok_r(NULL, which_sep(list), &save)) {
+			len += strlen(arg);
+			new_count++;
+		}
+
+		free(list);
+
+	}
+
+	len += new_count + 1;
+	csv = calloc(1, len);
+	if (!csv)
+		return NULL;
+	for (i = 0; i < *count; i++) {
+		list = strdup(strings[i]);
+		if (!list)
+			return NULL;
+
+		for (arg = strtok_r(list, which_sep(list), &save); arg;
+		     arg = strtok_r(NULL, which_sep(list), &save)) {
+			cursor += snprintf(csv + cursor, len - cursor, "%s%s",
+					   arg, i + 1 < new_count ? "," : "");
+			if (cursor >= len) {
+				csv[len] = 0;
+				break;
+			}
+		}
 
+		free(list);
+	}
+	*count = new_count;
+	return csv;
+}
+
+static struct sort_context {
+	const char *csv;
+} sort_context;
+
+static int memdev_filter_pos(struct json_object *jobj, const char *_csv)
+{
+	struct cxl_memdev *memdev = json_object_get_userdata(jobj);
+	char *csv, *save;
+	const char *arg;
+	int pos;
+
+	csv = strdup(_csv);
+	if (!csv)
+		return -1;
+
+	for (pos = 0, arg = strtok_r(csv, which_sep(csv), &save); arg;
+	     arg = strtok_r(NULL, which_sep(csv), &save), pos++)
+		if (util_cxl_memdev_filter(memdev, arg, NULL))
+			return pos;
+	free(csv);
+
+	return pos;
+}
+
+static int memdev_sort(const void *a, const void *b)
+{
+	struct json_object **a_obj, **b_obj;
+	int a_pos, b_pos;
+
+	a_obj = (struct json_object **) a;
+	b_obj = (struct json_object **) b;
+
+	a_pos = memdev_filter_pos(*a_obj, sort_context.csv);
+	b_pos = memdev_filter_pos(*b_obj, sort_context.csv);
+
+	if (a_pos < 0 || b_pos < 0)
+		return 0;
+
+	return a_pos - b_pos;
+}
+
+static struct json_object *collect_memdevs(struct cxl_ctx *ctx,
+					   const char *decoder, int *count,
+					   const char **mems)
+{
+	const char *csv = to_csv(count, mems);
+	struct cxl_filter_params filter_params = {
+		.decoder_filter = decoder,
+		.memdevs = true,
+		.memdev_filter = csv,
+	};
+	struct json_object *jmemdevs;
+
+	jmemdevs = cxl_filter_walk(ctx, &filter_params);
+
+	if (!jmemdevs) {
+		log_err(&rl, "failed to retrieve memdevs\n");
+		goto out;
+	}
+
+	if (json_object_array_length(jmemdevs) == 0) {
+		log_err(&rl,
+			"no active memdevs found: decoder: %s filter: %s\n",
+			decoder, csv ? csv : "none");
+		json_object_put(jmemdevs);
+		jmemdevs = NULL;
+		goto out;
+	}
+
+	if (csv) {
+		sort_context.csv = csv,
+		json_object_array_sort(jmemdevs, memdev_sort);
+	}
+out:
+	free((void *)csv);
+	return jmemdevs;
+}
+
+static bool validate_ways(struct parsed_params *p, int count)
+{
+	/*
+	 * Validate interleave ways against targets found in the topology. If
+	 * the targets were specified, then non-default param.ways must equal
+	 * that number of targets.
+	 */
+	if (p->ways > p->num_memdevs || (count && p->ways != p->num_memdevs)) {
+		log_err(&rl,
+			"Interleave ways %d is %s than number of memdevs %s: %d\n",
+			p->ways, p->ways > p->num_memdevs ? "greater" : "less",
+			count ? "specified" : "found", p->num_memdevs);
+		return false;
+	}
+	return true;
+}
+
+static int parse_create_options(struct cxl_ctx *ctx, int count,
+				const char **mems, struct parsed_params *p)
+{
 	if (!param.root_decoder) {
 		log_err(&rl, "no root decoder specified\n");
 		return -EINVAL;
 	}
 
+	/*
+	 * For all practical purposes, -m is the default target type, but
+	 * hold off on actively making that decision until a second target
+	 * option is available.
+	 */
+	if (!param.memdevs) {
+		log_err(&rl,
+			"must specify option for target object types (-m)\n");
+		return -EINVAL;
+	}
+
+	/* Collect the valid memdevs relative to the given root decoder */
+	p->memdevs = collect_memdevs(ctx, param.root_decoder, &count, mems);
+	if (!p->memdevs)
+		return -ENXIO;
+	p->num_memdevs = json_object_array_length(p->memdevs);
+
 	if (param.type) {
 		p->mode = cxl_decoder_mode_from_ident(param.type);
 		if (p->mode == CXL_DECODER_MODE_NONE) {
@@ -132,8 +308,12 @@ static int parse_create_options(int argc, const char **argv,
 		return -EINVAL;
 	} else if (param.ways < INT_MAX) {
 		p->ways = param.ways;
-	} else if (argc) {
-		p->ways = argc;
+		if (!validate_ways(p, count))
+			return -EINVAL;
+	} else if (count) {
+		p->ways = count;
+		if (!validate_ways(p, count))
+			return -EINVAL;
 	} else {
 		log_err(&rl,
 			"couldn't determine interleave ways from options or arguments\n");
@@ -149,19 +329,6 @@ static int parse_create_options(int argc, const char **argv,
 		p->granularity = param.granularity;
 	}
 
-	if (argc > p->ways) {
-		for (i = p->ways; i < argc; i++)
-			log_err(&rl, "extra argument: %s\n", p->targets[i]);
-		return -EINVAL;
-	}
-
-	if (argc < p->ways) {
-		log_err(&rl,
-			"too few target arguments (%d) for interleave ways (%u)\n",
-			argc, p->ways);
-		return -EINVAL;
-	}
-
 	if (p->size && p->ways) {
 		if (p->size % p->ways) {
 			log_err(&rl,
@@ -171,17 +338,6 @@ static int parse_create_options(int argc, const char **argv,
 		}
 	}
 
-	/*
-	 * For all practical purposes, -m is the default target type, but
-	 * hold off on actively making that decision until a second target
-	 * option is available.
-	 */
-	if (!param.memdevs) {
-		log_err(&rl,
-			"must specify option for target object types (-m)\n");
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
@@ -196,8 +352,8 @@ static int parse_region_options(int argc, const char **argv,
 	};
 
 	argc = parse_options(argc, argv, options, u, 0);
-	p->targets = argv;
-	p->num_targets = argc;
+	p->argc = argc;
+	p->argv = argv;
 
 	if (param.debug) {
 		cxl_set_log_priority(ctx, LOG_DEBUG);
@@ -207,62 +363,27 @@ static int parse_region_options(int argc, const char **argv,
 
 	switch(action) {
 	case ACTION_CREATE:
-		return parse_create_options(argc, argv, p);
+		return parse_create_options(ctx, argc, argv, p);
 	default:
 		return 0;
 	}
 }
 
-/**
- * validate_memdev() - match memdev with the target provided,
- *                     and determine its size contribution
- * @memdev: cxl_memdev being tested for a match against the named target
- * @target: target memdev
- * @p:      params structure
- *
- * This is called for each memdev in the system, and only returns 'true' if
- * the memdev name matches the target argument being tested. Additionally,
- * it sets an ep_min_size attribute that always contains the size of the
- * smallest target in the provided list. This is used during the automatic
- * size determination later, to ensure that all targets contribute equally
- * to the region in case of unevenly sized memdevs.
- */
-static bool validate_memdev(struct cxl_memdev *memdev, const char *target,
-			    struct parsed_params *p)
-{
-	const char *devname = cxl_memdev_get_devname(memdev);
-	u64 size;
-
-	if (strcmp(devname, target) != 0)
-		return false;
-
-	size = cxl_memdev_get_pmem_size(memdev);
-	if (!p->ep_min_size)
-		p->ep_min_size = size;
-	else
-		p->ep_min_size = min(p->ep_min_size, size);
-
-	return true;
-}
-
-static int validate_config_memdevs(struct cxl_ctx *ctx, struct parsed_params *p)
+static void collect_minsize(struct cxl_ctx *ctx, struct parsed_params *p)
 {
-	int i, matched = 0;
+	int i;
 
 	for (i = 0; i < p->ways; i++) {
-		struct cxl_memdev *memdev;
+		struct json_object *jobj =
+			json_object_array_get_idx(p->memdevs, i);
+		struct cxl_memdev *memdev = json_object_get_userdata(jobj);
+		u64 size = cxl_memdev_get_pmem_size(memdev);
 
-		cxl_memdev_foreach(ctx, memdev)
-			if (validate_memdev(memdev, p->targets[i], p))
-				matched++;
-	}
-	if (matched != p->ways) {
-		log_err(&rl,
-			"one or more memdevs not found in CXL topology\n");
-		return -ENXIO;
+		if (!p->ep_min_size)
+			p->ep_min_size = size;
+		else
+			p->ep_min_size = min(p->ep_min_size, size);
 	}
-
-	return 0;
 }
 
 static int validate_decoder(struct cxl_decoder *decoder,
@@ -330,26 +451,18 @@ found:
 	if (rc)
 		return rc;
 
-	return validate_config_memdevs(ctx, p);
+	collect_minsize(ctx, p);
+	return 0;
 }
 
-static struct cxl_decoder *
-cxl_memdev_target_find_decoder(struct cxl_ctx *ctx, const char *memdev_name)
+static struct cxl_decoder *cxl_memdev_find_decoder(struct cxl_memdev *memdev)
 {
-	struct cxl_endpoint *ep = NULL;
+	const char *memdev_name = cxl_memdev_get_devname(memdev);
 	struct cxl_decoder *decoder;
-	struct cxl_memdev *memdev;
+	struct cxl_endpoint *ep;
 	struct cxl_port *port;
 
-	cxl_memdev_foreach(ctx, memdev) {
-		const char *devname = cxl_memdev_get_devname(memdev);
-
-		if (strcmp(devname, memdev_name) != 0)
-			continue;
-
-		ep = cxl_memdev_get_endpoint(memdev);
-	}
-
+	ep = cxl_memdev_get_endpoint(memdev);
 	if (!ep) {
 		log_err(&rl, "could not get an endpoint for %s\n",
 			memdev_name);
@@ -379,7 +492,7 @@ do { \
 				prefix##_get_devname(dev), \
 				strerror(abs(__rc))); \
 		rc = __rc; \
-		goto err_delete; \
+		goto out; \
 	} \
 } while (0)
 
@@ -478,7 +591,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 
 	rc = cxl_region_determine_granularity(region, p);
 	if (rc < 0)
-		goto err_delete;
+		goto out;
 	granularity = rc;
 
 	uuid_generate(uuid);
@@ -488,12 +601,15 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 	try(cxl_region, set_size, region, size);
 
 	for (i = 0; i < p->ways; i++) {
-		struct cxl_decoder *ep_decoder = NULL;
+		struct cxl_decoder *ep_decoder;
+		struct json_object *jobj =
+			json_object_array_get_idx(p->memdevs, i);
+		struct cxl_memdev *memdev = json_object_get_userdata(jobj);
 
-		ep_decoder = cxl_memdev_target_find_decoder(ctx, p->targets[i]);
+		ep_decoder = cxl_memdev_find_decoder(memdev);
 		if (!ep_decoder) {
 			rc = -ENXIO;
-			goto err_delete;
+			goto out;
 		}
 		if (cxl_decoder_get_mode(ep_decoder) != p->mode) {
 			/*
@@ -508,8 +624,8 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 		rc = cxl_region_set_target(region, i, ep_decoder);
 		if (rc) {
 			log_err(&rl, "%s: failed to set target%d to %s\n",
-				devname, i, p->targets[i]);
-			goto err_delete;
+				devname, i, cxl_memdev_get_devname(memdev));
+			goto out;
 		}
 	}
 
@@ -517,14 +633,14 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 	if (rc) {
 		log_err(&rl, "%s: failed to commit decode: %s\n", devname,
 			strerror(-rc));
-		goto err_delete;
+		goto out;
 	}
 
 	rc = cxl_region_enable(region);
 	if (rc) {
 		log_err(&rl, "%s: failed to enable: %s\n", devname,
 			strerror(-rc));
-		goto err_delete;
+		goto out;
 	}
 	*count = 1;
 
@@ -535,10 +651,10 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 		printf("%s\n", json_object_to_json_string_ext(jregion,
 					JSON_C_TO_STRING_PRETTY));
 
-	return 0;
-
-err_delete:
-	cxl_region_delete(region);
+out:
+	json_object_put(p->memdevs);
+	if (rc)
+		cxl_region_delete(region);
 	return rc;
 }
 
@@ -630,8 +746,8 @@ static int decoder_region_action(struct parsed_params *p,
 	cxl_region_foreach_safe (decoder, region, _r) {
 		int i, match = 0;
 
-		for (i = 0; i < p->num_targets; i++) {
-			if (util_cxl_region_filter(region, p->targets[i])) {
+		for (i = 0; i < p->argc; i++) {
+			if (util_cxl_region_filter(region, p->argv[i])) {
 				match = 1;
 				break;
 			}
@@ -665,7 +781,7 @@ static int region_action(int argc, const char **argv, struct cxl_ctx *ctx,
 		return rc;
 
 	if (action == ACTION_CREATE)
-		return create_region(ctx, count, p);
+		rc = create_region(ctx, count, p);
 
 	cxl_bus_foreach(ctx, bus) {
 		struct cxl_decoder *decoder;
diff --git a/util/util.h b/util/util.h
index b2b4ae6503aa..58db06530c37 100644
--- a/util/util.h
+++ b/util/util.h
@@ -79,6 +79,15 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
         return strncmp(str, prefix, len) ? NULL : str + len;
 }
 
+static inline const char *which_sep(const char *filter)
+{
+	if (strchr(filter, ' '))
+		return " ";
+	if (strchr(filter, ','))
+		return ",";
+	return " ";
+}
+
 static inline int is_absolute_path(const char *path)
 {
 	return path[0] == '/';

[ndctl PATCH v2 14/18] cxl/region: Trim region size by max available extent

[Dan Williams]

When a size is not specified, limit the size to either the available DPA
capacity, or the max available extent in the root decoder, whichever is
smaller.

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

diff --git a/cxl/region.c b/cxl/region.c
index 36ebc8e5210f..286c358f1a34 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -544,6 +544,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 	unsigned long flags = UTIL_JSON_TARGETS;
 	struct json_object *jregion;
 	struct cxl_region *region;
+	bool default_size = true;
 	int i, rc, granularity;
 	u64 size, max_extent;
 	const char *devname;
@@ -555,6 +556,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 
 	if (p->size) {
 		size = p->size;
+		default_size = false;
 	} else if (p->ep_min_size) {
 		size = p->ep_min_size * p->ways;
 	} else {
@@ -567,13 +569,16 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 			cxl_decoder_get_devname(p->root_decoder));
 		return -EINVAL;
 	}
-	if (size > max_extent) {
+	if (!default_size && size > max_extent) {
 		log_err(&rl,
 			"%s: region size %#lx exceeds max available space\n",
 			cxl_decoder_get_devname(p->root_decoder), size);
 		return -ENOSPC;
 	}
 
+	if (size > max_extent)
+		size = ALIGN_DOWN(max_extent, SZ_256M * p->ways);
+
 	if (p->mode == CXL_DECODER_MODE_PMEM) {
 		region = cxl_decoder_create_pmem_region(p->root_decoder);
 		if (!region) {

Re: [ndctl PATCH v2 14/18] cxl/region: Trim region size by max available extent

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:29:20PM -0800, Dan Williams wrote:
> When a size is not specified, limit the size to either the available DPA
> capacity, or the max available extent in the root decoder, whichever is
> smaller.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  cxl/region.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 36ebc8e5210f..286c358f1a34 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -544,6 +544,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  	unsigned long flags = UTIL_JSON_TARGETS;
>  	struct json_object *jregion;
>  	struct cxl_region *region;
> +	bool default_size = true;
>  	int i, rc, granularity;
>  	u64 size, max_extent;
>  	const char *devname;
> @@ -555,6 +556,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  
>  	if (p->size) {
>  		size = p->size;
> +		default_size = false;
>  	} else if (p->ep_min_size) {
>  		size = p->ep_min_size * p->ways;
>  	} else {
> @@ -567,13 +569,16 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  			cxl_decoder_get_devname(p->root_decoder));
>  		return -EINVAL;
>  	}
> -	if (size > max_extent) {
> +	if (!default_size && size > max_extent) {
>  		log_err(&rl,
>  			"%s: region size %#lx exceeds max available space\n",
>  			cxl_decoder_get_devname(p->root_decoder), size);
>  		return -ENOSPC;
>  	}
>  
> +	if (size > max_extent)
> +		size = ALIGN_DOWN(max_extent, SZ_256M * p->ways);
> +
>  	if (p->mode == CXL_DECODER_MODE_PMEM) {
>  		region = cxl_decoder_create_pmem_region(p->root_decoder);
>  		if (!region) {
> 

[ndctl PATCH v2 15/18] cxl/Documentation: Fix whitespace typos in create-region man page

[Dan Williams]

---
 Documentation/cxl/cxl-create-region.txt |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
index 6b740d5b8d96..e0e6818cfdd1 100644
--- a/Documentation/cxl/cxl-create-region.txt
+++ b/Documentation/cxl/cxl-create-region.txt
@@ -28,7 +28,7 @@ be emitted on stderr.
 EXAMPLE
 -------
 ----
-#cxl create - region - m - d decoder0 .1 - w 2 - g 1024 mem0 mem1
+#cxl create-region -m -d decoder0.1 -w 2 -g 1024 mem0 mem1
 {
   "region":"region0",
   "resource":"0xc90000000",

Re: [ndctl PATCH v2 15/18] cxl/Documentation: Fix whitespace typos in create-region man page

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:29:26PM -0800, Dan Williams wrote:

Missing SOB and commit log

The diff - 
Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> 
> ---
>  Documentation/cxl/cxl-create-region.txt |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
> index 6b740d5b8d96..e0e6818cfdd1 100644
> --- a/Documentation/cxl/cxl-create-region.txt
> +++ b/Documentation/cxl/cxl-create-region.txt
> @@ -28,7 +28,7 @@ be emitted on stderr.
>  EXAMPLE
>  -------
>  ----
> -#cxl create - region - m - d decoder0 .1 - w 2 - g 1024 mem0 mem1
> +#cxl create-region -m -d decoder0.1 -w 2 -g 1024 mem0 mem1
>  {
>    "region":"region0",
>    "resource":"0xc90000000",
> 

Re: [ndctl PATCH v2 15/18] cxl/Documentation: Fix whitespace typos in create-region man page

[Dan Williams]

Alison Schofield wrote:
> On Thu, Dec 08, 2022 at 01:29:26PM -0800, Dan Williams wrote:
> 
> Missing SOB and commit log

Whoops, moved too fast and broke things.

Vishal, feel free to add:

---
The 'create-region' man page had some typos when it was first created.
Clean them up per syntax expectations.

Fixes: 21b089025178 ("cxl: add a 'create-region' command")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---

...or let me know if you want a resend.

Re: [ndctl PATCH v2 15/18] cxl/Documentation: Fix whitespace typos in create-region man page

[Verma, Vishal L]

On Fri, 2022-12-09 at 10:06 -0800, Dan Williams wrote:
> Alison Schofield wrote:
> > On Thu, Dec 08, 2022 at 01:29:26PM -0800, Dan Williams wrote:
> > 
> > Missing SOB and commit log
> 
> Whoops, moved too fast and broke things.
> 
> Vishal, feel free to add:

No problem, done. Patch 7 was missing these too, but I've fixed that up
as well. This series is now in pushed to pending.

> 
> ---
> The 'create-region' man page had some typos when it was first created.
> Clean them up per syntax expectations.
> 
> Fixes: 21b089025178 ("cxl: add a 'create-region' command")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> 
> ...or let me know if you want a resend.

[ndctl PATCH v2 16/18] cxl/region: Autoselect memdevs for create-region

[Dan Williams]

Now that parse_create_region() uses cxl_filter_walk() to gather memdevs
use that as the target list in case no target list is provided. In other
words the result of "cxl list -M -d $decoder" returns all the potential
memdevs that can comprise a region under $decoder, so just go ahead and
try to use that as the target list by default.

Note though that the order of devices returned by cxl_filter_walk() may
not be a suitable region creation order. So this porcelain helps for
simple topologies, but needs a follow-on patch to sort the memdevs by
valid region order, and/or discover cases where deviceA or deviceB can
be in the region, but not both.

Outside of those cases:

   cxl create-region -d decoderX.Y

...is sufficient to create a region.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/cxl/cxl-create-region.txt |   10 ++++++----
 cxl/region.c                            |   16 ++++++++--------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
index e0e6818cfdd1..286779eff9ed 100644
--- a/Documentation/cxl/cxl-create-region.txt
+++ b/Documentation/cxl/cxl-create-region.txt
@@ -53,16 +53,18 @@ OPTIONS
 -------
 <target(s)>::
 The CXL targets that should be used to form the region. The number of
-'target' arguments must match the '--ways' option (if provided). The
-targets are memdev names such as 'mem0', 'mem1' etc.
+'target' arguments must match the '--ways' option (if provided).
 
 include::bus-option.txt[]
 
 -m::
 --memdevs::
 	Indicate that the non-option arguments for 'target(s)' refer to memdev
-	names. Currently this is the only option supported, and must be
-	specified.
+	device names. If this option is omitted and no targets are specified
+	then create-region uses the equivalent of 'cxl list -M -d $decoder'
+	internally as the target list. Note that depending on the topology, for
+	example with switches, the automatic target list ordering may not be
+	valid and manual specification of the target list is required.
 
 -s::
 --size=::
diff --git a/cxl/region.c b/cxl/region.c
index 286c358f1a34..15cac64a158c 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -269,10 +269,13 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
 	}
 
 	/*
-	 * For all practical purposes, -m is the default target type, but
-	 * hold off on actively making that decision until a second target
-	 * option is available.
+	 * For all practical purposes, -m is the default target type, but hold
+	 * off on actively making that decision until a second target option is
+	 * available. Unless there are no arguments then just assume memdevs.
 	 */
+	if (!count)
+		param.memdevs = true;
+
 	if (!param.memdevs) {
 		log_err(&rl,
 			"must specify option for target object types (-m)\n");
@@ -314,11 +317,8 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
 		p->ways = count;
 		if (!validate_ways(p, count))
 			return -EINVAL;
-	} else {
-		log_err(&rl,
-			"couldn't determine interleave ways from options or arguments\n");
-		return -EINVAL;
-	}
+	} else
+		p->ways = p->num_memdevs;
 
 	if (param.granularity < INT_MAX) {
 		if (param.granularity <= 0) {

Re: [ndctl PATCH v2 16/18] cxl/region: Autoselect memdevs for create-region

[Alison Schofield]

On Thu, Dec 08, 2022 at 01:29:32PM -0800, Dan Williams wrote:
> Now that parse_create_region() uses cxl_filter_walk() to gather memdevs
> use that as the target list in case no target list is provided. In other
> words the result of "cxl list -M -d $decoder" returns all the potential
> memdevs that can comprise a region under $decoder, so just go ahead and
> try to use that as the target list by default.
> 
> Note though that the order of devices returned by cxl_filter_walk() may
> not be a suitable region creation order. So this porcelain helps for
> simple topologies, but needs a follow-on patch to sort the memdevs by
> valid region order, and/or discover cases where deviceA or deviceB can
> be in the region, but not both.
> 
> Outside of those cases:
> 
>    cxl create-region -d decoderX.Y
> 
> ...is sufficient to create a region.
> 

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  Documentation/cxl/cxl-create-region.txt |   10 ++++++----
>  cxl/region.c                            |   16 ++++++++--------
>  2 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
> index e0e6818cfdd1..286779eff9ed 100644
> --- a/Documentation/cxl/cxl-create-region.txt
> +++ b/Documentation/cxl/cxl-create-region.txt
> @@ -53,16 +53,18 @@ OPTIONS
>  -------
>  <target(s)>::
>  The CXL targets that should be used to form the region. The number of
> -'target' arguments must match the '--ways' option (if provided). The
> -targets are memdev names such as 'mem0', 'mem1' etc.
> +'target' arguments must match the '--ways' option (if provided).
>  
>  include::bus-option.txt[]
>  
>  -m::
>  --memdevs::
>  	Indicate that the non-option arguments for 'target(s)' refer to memdev
> -	names. Currently this is the only option supported, and must be
> -	specified.
> +	device names. If this option is omitted and no targets are specified
> +	then create-region uses the equivalent of 'cxl list -M -d $decoder'
> +	internally as the target list. Note that depending on the topology, for
> +	example with switches, the automatic target list ordering may not be
> +	valid and manual specification of the target list is required.
>  
>  -s::
>  --size=::
> diff --git a/cxl/region.c b/cxl/region.c
> index 286c358f1a34..15cac64a158c 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -269,10 +269,13 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
>  	}
>  
>  	/*
> -	 * For all practical purposes, -m is the default target type, but
> -	 * hold off on actively making that decision until a second target
> -	 * option is available.
> +	 * For all practical purposes, -m is the default target type, but hold
> +	 * off on actively making that decision until a second target option is
> +	 * available. Unless there are no arguments then just assume memdevs.
>  	 */
> +	if (!count)
> +		param.memdevs = true;
> +
>  	if (!param.memdevs) {
>  		log_err(&rl,
>  			"must specify option for target object types (-m)\n");
> @@ -314,11 +317,8 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
>  		p->ways = count;
>  		if (!validate_ways(p, count))
>  			return -EINVAL;
> -	} else {
> -		log_err(&rl,
> -			"couldn't determine interleave ways from options or arguments\n");
> -		return -EINVAL;
> -	}
> +	} else
> +		p->ways = p->num_memdevs;
>  
>  	if (param.granularity < INT_MAX) {
>  		if (param.granularity <= 0) {
> 

[ndctl PATCH v2 17/18] cxl/test: Extend cxl-topology.sh for a single root-port host-bridge

[Dan Williams]

A recent extension of cxl_test adds 2 memory devices attached through a
switch to a single ported host-bridge to reproduce a bug report.

Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: http://lore.kernel.org/r/20221010172057.00001559@huawei.com
Tested-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/cxl-topology.sh |   48 +++++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
index 1f15d29f0600..362fffa6d539 100644
--- a/test/cxl-topology.sh
+++ b/test/cxl-topology.sh
@@ -29,27 +29,30 @@ count=$(jq "length" <<< $json)
 root=$(jq -r ".[] | .bus" <<< $json)
 
 
-# validate 2 host bridges under a root port
+# validate 2 or 3 host bridges under a root port
 port_sort="sort_by(.port | .[4:] | tonumber)"
 json=$($CXL list -b cxl_test -BP)
 count=$(jq ".[] | .[\"ports:$root\"] | length" <<< $json)
-((count == 2)) || err "$LINENO"
+((count == 2)) || ((count == 3)) || err "$LINENO"
+bridges=$count
 
 bridge[0]=$(jq -r ".[] | .[\"ports:$root\"] | $port_sort | .[0].port" <<< $json)
 bridge[1]=$(jq -r ".[] | .[\"ports:$root\"] | $port_sort | .[1].port" <<< $json)
+((bridges > 2)) && bridge[2]=$(jq -r ".[] | .[\"ports:$root\"] | $port_sort | .[2].port" <<< $json)
 
+# validate root ports per host bridge
+check_host_bridge()
+{
+	json=$($CXL list -b cxl_test -T -p $1)
+	count=$(jq ".[] | .dports | length" <<< $json)
+	((count == $2)) || err "$3"
+}
 
-# validate 2 root ports per host bridge
-json=$($CXL list -b cxl_test -T -p ${bridge[0]})
-count=$(jq ".[] | .dports | length" <<< $json)
-((count == 2)) || err "$LINENO"
-
-json=$($CXL list -b cxl_test -T -p ${bridge[1]})
-count=$(jq ".[] | .dports | length" <<< $json)
-((count == 2)) || err "$LINENO"
+check_host_bridge ${bridge[0]} 2 $LINENO
+check_host_bridge ${bridge[1]} 2 $LINENO
+((bridges > 2)) && check_host_bridge ${bridge[2]} 1 $LINENO
 
-
-# validate 2 switches per-root port
+# validate 2 switches per root-port
 json=$($CXL list -b cxl_test -P -p ${bridge[0]})
 count=$(jq ".[] | .[\"ports:${bridge[0]}\"] | length" <<< $json)
 ((count == 2)) || err "$LINENO"
@@ -65,9 +68,9 @@ switch[2]=$(jq -r ".[] | .[\"ports:${bridge[1]}\"] | $port_sort | .[0].host" <<<
 switch[3]=$(jq -r ".[] | .[\"ports:${bridge[1]}\"] | $port_sort | .[1].host" <<< $json)
 
 
-# validate the expected properties of the 4 root decoders
-# use the size of the first decoder to determine the cxl_test version /
-# properties
+# validate the expected properties of the 4 or 5 root decoders
+# use the size of the first decoder to determine the
+# cxl_test version / properties
 json=$($CXL list -b cxl_test -D -d root)
 port_id=${root:4}
 port_id_len=${#port_id}
@@ -103,12 +106,19 @@ count=$(jq "[ $decoder_sort | .[3] |
 	select(.nr_targets == 2) ] | length" <<< $json)
 ((count == 1)) || err "$LINENO"
 
+if (( bridges == 3 )); then
+	count=$(jq "[ $decoder_sort | .[4] |
+		select(.pmem_capable == true) |
+		select(.size == $decoder_base_size) |
+		select(.nr_targets == 1) ] | length" <<< $json)
+	((count == 1)) || err "$LINENO"
+fi
 
-# check that all 8 cxl_test memdevs are enabled by default and have a
+# check that all 8 or 10 cxl_test memdevs are enabled by default and have a
 # pmem size of 256M, or 1G
 json=$($CXL list -b cxl_test -M)
 count=$(jq "map(select(.pmem_size == $pmem_size)) | length" <<< $json)
-((count == 8)) || err "$LINENO"
+((bridges == 2 && count == 8 || bridges == 3 && count == 10)) || err "$LINENO"
 
 
 # check that switch ports disappear after all of their memdevs have been
@@ -151,8 +161,8 @@ do
 done
 
 
-# validate host bridge tear down
-for b in ${bridge[@]}
+# validate host bridge tear down for the first 2 bridges
+for b in ${bridge[0]} ${bridge[1]}
 do
 	$CXL disable-port $b -f
 	json=$($CXL list -M -i -p $b)

[ndctl PATCH v2 18/18] cxl/test: Test single-port host-bridge region creation

[Dan Williams]

The original port decoder programming algorithm in the kernel failed to
acommodate the corner case of a passthrough port connected to a fan-out
port. Use the 5th cxl_test decoder to regression test this scenario.

Reported-by: Bobo WL <lmw.bobo@gmail.com>
Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: http://lore.kernel.org/r/20221010172057.00001559@huawei.com
Tested-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/cxl-create-region.sh |   28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index 82aad3a7285a..47aed44848ab 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -110,6 +110,34 @@ create_subregions()
 	done
 }
 
+create_single()
+{
+	# the 5th cxl_test decoder is expected to target a single-port
+	# host-bridge. Older cxl_test implementations may not define it,
+	# so skip the test in that case.
+	decoder=$($CXL list -b cxl_test -D -d root |
+		  jq -r ".[4] |
+		  select(.pmem_capable == true) |
+		  select(.nr_targets == 1) |
+		  .decoder")
+
+        if [[ ! $decoder ]]; then
+                echo "no single-port host-bridge decoder found, skipping"
+                return
+        fi
+
+	region=$($CXL create-region -d "$decoder" | jq -r ".region")
+	if [[ ! $region ]]; then
+		echo "failed to create single-port host-bridge region"
+		err "$LINENO"
+	fi
+
+	destroy_regions "$region"
+}
+
+# test region creation on devices behind a single-port host-bridge
+create_single
+
 # test reading labels directly through cxl-cli
 readarray -t mems < <("$CXL" list -b cxl_test -M | jq -r '.[].memdev')