[ndctl PATCH] cxl/test: replace a bad root decoder usage in cxl-xor-region.sh

[ndctl PATCH] cxl/test: replace a bad root decoder usage in cxl-xor-region.sh

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

The 4-way XOR region as defined in this test uses a root decoder that
is created using an improperly defined CFMWS. The problem with the
CFMWS is that Host Bridges repeat in the target list like this:
{ 0, 1, 0, 1 }.

Stop using that root decoder and create a 4-way XOR region using an
x2 root decoder that supports XOR arithmetic.

The test passes prior to this patch but there is an interleave check [1]
introduced in the CXL region driver that will expose the bad interleave
this test creates via dev_dbg() messages like this:

[ ] cxl_core:cxl_region_attach:1808: cxl decoder17.0: Test cxl_calc_interleave_pos(): fail test_pos:4 cxled->pos:2
[ ] cxl_core:cxl_region_attach:1808: cxl decoder18.0: Test cxl_calc_interleave_pos(): fail test_pos:5 cxled->pos:3

Note that the CFMWS's are defined in the kernel cxl-test module, so a
kernel patch removing the bad CFMWS will also need to be merged, but
that cleanup can follow this patch. Also note that the bad CFMWS is not
used in the default cxl-test environment. It is only visible when the
cxl-test module is loaded using the param interleave_arithmetic=1. It is
a special config that provides the XOR math CFMWS's for this test.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a3e00c964fb943934af916f48f0dd43b5110c866

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

Vishal - I'm hoping you will merge this in ndctl v79 even though the
exposure with the kernel doesn't happen until kernel 6.7. This way
users of cxl-test are not learning to ignore the interleave calc
warnings.

Also, hopefully I have not introduced any new shell issues, but
I know this unit test has existing warnings. Can we do a shell
cleanup in a follow-on patchset across the CXL tests?
(and not last minute for your ndctl release)


 test/cxl-xor-region.sh | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/test/cxl-xor-region.sh b/test/cxl-xor-region.sh
index 1962327bd00a..117e7a4bba61 100644
--- a/test/cxl-xor-region.sh
+++ b/test/cxl-xor-region.sh
@@ -68,10 +68,10 @@ setup_x2()
 
 setup_x4()
 {
-        # find x4 decoder
+        # find an x2 decoder
         decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
           select(.pmem_capable == true) |
-          select(.nr_targets == 4) |
+          select(.nr_targets == 2) |
           .decoder")
 
         # Find a memdev for each host-bridge interleave position
@@ -79,14 +79,10 @@ setup_x4()
             .targets | .[] | select(.position == 0) | .target")
         port_dev1=$($CXL list -T -d $decoder | jq -r ".[] |
             .targets | .[] | select(.position == 1) | .target")
-        port_dev2=$($CXL list -T -d $decoder | jq -r ".[] |
-            .targets | .[] | select(.position == 2) | .target")
-        port_dev3=$($CXL list -T -d $decoder | jq -r ".[] |
-            .targets | .[] | select(.position == 3) | .target")
         mem0=$($CXL list -M -p $port_dev0 | jq -r ".[0].memdev")
-        mem1=$($CXL list -M -p $port_dev1 | jq -r ".[1].memdev")
-        mem2=$($CXL list -M -p $port_dev2 | jq -r ".[2].memdev")
-        mem3=$($CXL list -M -p $port_dev3 | jq -r ".[3].memdev")
+        mem1=$($CXL list -M -p $port_dev1 | jq -r ".[0].memdev")
+        mem2=$($CXL list -M -p $port_dev0 | jq -r ".[1].memdev")
+        mem3=$($CXL list -M -p $port_dev1 | jq -r ".[1].memdev")
         memdevs="$mem0 $mem1 $mem2 $mem3"
 }
 

base-commit: a871e6153b11fe63780b37cdcb1eb347b296095c
-- 
2.37.3

Re: [ndctl PATCH] cxl/test: replace a bad root decoder usage in cxl-xor-region.sh

[Alison Schofield]

On Tue, Nov 21, 2023 at 06:57:53PM -0800, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> The 4-way XOR region as defined in this test uses a root decoder that
> is created using an improperly defined CFMWS. The problem with the
> CFMWS is that Host Bridges repeat in the target list like this:
> { 0, 1, 0, 1 }.
> 
> Stop using that root decoder and create a 4-way XOR region using an
> x2 root decoder that supports XOR arithmetic.
> 
> The test passes prior to this patch but there is an interleave check [1]
> introduced in the CXL region driver that will expose the bad interleave
> this test creates via dev_dbg() messages like this:
> 
> [ ] cxl_core:cxl_region_attach:1808: cxl decoder17.0: Test cxl_calc_interleave_pos(): fail test_pos:4 cxled->pos:2
> [ ] cxl_core:cxl_region_attach:1808: cxl decoder18.0: Test cxl_calc_interleave_pos(): fail test_pos:5 cxled->pos:3
> 
> Note that the CFMWS's are defined in the kernel cxl-test module, so a
> kernel patch removing the bad CFMWS will also need to be merged, but
> that cleanup can follow this patch. Also note that the bad CFMWS is not
> used in the default cxl-test environment. It is only visible when the
> cxl-test module is loaded using the param interleave_arithmetic=1. It is
> a special config that provides the XOR math CFMWS's for this test.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a3e00c964fb943934af916f48f0dd43b5110c866
> 


Fixes: 05486f8bf154 ("cxl/test: add cxl_xor_region test")


> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> 
> Vishal - I'm hoping you will merge this in ndctl v79 even though the
> exposure with the kernel doesn't happen until kernel 6.7. This way
> users of cxl-test are not learning to ignore the interleave calc
> warnings.
> 
> Also, hopefully I have not introduced any new shell issues, but
> I know this unit test has existing warnings. Can we do a shell
> cleanup in a follow-on patchset across the CXL tests?
> (and not last minute for your ndctl release)
> 
> 
>  test/cxl-xor-region.sh | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 

snip

> 

Re: [ndctl PATCH] cxl/test: replace a bad root decoder usage in cxl-xor-region.sh

[Verma, Vishal L]

On Tue, 2023-11-21 at 18:57 -0800, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> The 4-way XOR region as defined in this test uses a root decoder that
> is created using an improperly defined CFMWS. The problem with the
> CFMWS is that Host Bridges repeat in the target list like this:
> { 0, 1, 0, 1 }.
> 
> Stop using that root decoder and create a 4-way XOR region using an
> x2 root decoder that supports XOR arithmetic.
> 
> The test passes prior to this patch but there is an interleave check [1]
> introduced in the CXL region driver that will expose the bad interleave
> this test creates via dev_dbg() messages like this:
> 
> [ ] cxl_core:cxl_region_attach:1808: cxl decoder17.0: Test cxl_calc_interleave_pos(): fail test_pos:4 cxled->pos:2
> [ ] cxl_core:cxl_region_attach:1808: cxl decoder18.0: Test cxl_calc_interleave_pos(): fail test_pos:5 cxled->pos:3
> 
> Note that the CFMWS's are defined in the kernel cxl-test module, so a
> kernel patch removing the bad CFMWS will also need to be merged, but
> that cleanup can follow this patch. Also note that the bad CFMWS is not
> used in the default cxl-test environment. It is only visible when the
> cxl-test module is loaded using the param interleave_arithmetic=1. It is
> a special config that provides the XOR math CFMWS's for this test.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a3e00c964fb943934af916f48f0dd43b5110c866
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> 
> Vishal - I'm hoping you will merge this in ndctl v79 even though the
> exposure with the kernel doesn't happen until kernel 6.7. This way
> users of cxl-test are not learning to ignore the interleave calc
> warnings.

Yep makes sense - I'm also thinking that since we're at -rc2, v79 can
just be the v6.7 equivalent release (and I'll include the sanitize
patches too since that's the only other pending v6.7 specific thing.

> 
> Also, hopefully I have not introduced any new shell issues, but
> I know this unit test has existing warnings. Can we do a shell
> cleanup in a follow-on patchset across the CXL tests?
> (and not last minute for your ndctl release)

Agreed, there are a few shellcheck complaints that have snuck in over
time, fixing these as a separate cleanup is definitely the way to go.

> 
>  test/cxl-xor-region.sh | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>