diff mbox series

[2/2] cxl/test: Replace an illegal CFMWS definition with a useful x3 CFMWS

Message ID d8bbae3ea5dc59bd4972236fe06adb5ec85e5c21.1707891715.git.alison.schofield@intel.com
State New, archived
Headers show
Series XOR 3-6-12 HB Interleave Calc Repair | expand

Commit Message

Alison Schofield Feb. 14, 2024, 7:13 a.m. UTC
From: Alison Schofield <alison.schofield@intel.com>

cxl/test has a small set (3) of CFMWS's that become available when
cxl_test is loaded with this module option: interleave_arithmetic=1.
That gives users of cxl_test access to root decoders that use XOR Math
for interleave arithmetic rather than the default Modulo.

The existing x4 CFMWS definition is an invalid configuration because
it repeats Host Bridges in the interleave set. A patch to remove its
usage in the CXL unit test cxl-xor-region.sh is in flight.

Coincidentally, there is another CFMWS definition queued for addition
here and a patch to use it in the CXL unit test is also in flight.
Rather than tear this CFMWS out and adjust the indexes only to follow
it with another patch to introduce a new CFMWS, do a switch in place.

Transform the illegal x4 CFMWS definition into a useful x3 version.

Fixes: 7a7e6edfca85 ("tools/testing/cxl: Add XOR Math support to cxl_test")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

I realize this switcheroo is atypical for a patch. It seemed to be a
cleaner repair, especially when trying to push this cxl/test change
with the acpi driver patch in this set. Say the word and I'll split.

Also, I'm not looking to put back a 4-way HB interleave at the moment,
because it's not a unique case (similar to 2-way in the calc) and also
because I have a patchset in the works that supports all the HB interleave
configs. This sets focus is repairing, not enhancing.


 tools/testing/cxl/test/cxl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Dan Williams Feb. 29, 2024, 6:59 p.m. UTC | #1
alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> cxl/test has a small set (3) of CFMWS's that become available when
> cxl_test is loaded with this module option: interleave_arithmetic=1.
> That gives users of cxl_test access to root decoders that use XOR Math
> for interleave arithmetic rather than the default Modulo.
> 
> The existing x4 CFMWS definition is an invalid configuration because
> it repeats Host Bridges in the interleave set. A patch to remove its
> usage in the CXL unit test cxl-xor-region.sh is in flight.

The commentary about "in flight" has me confused... posted patches that
I have missed, or work-in-progress patches on your workstation?

> Coincidentally, there is another CFMWS definition queued for addition
> here and a patch to use it in the CXL unit test is also in flight.
> Rather than tear this CFMWS out and adjust the indexes only to follow
> it with another patch to introduce a new CFMWS, do a switch in place.

It sounds like you are saying to squash 2 patches into one, but unless I
am missing someting only you knew there were 2 patches to begin with.]

> Transform the illegal x4 CFMWS definition into a useful x3 version.

Maybe just drop the commentary about in-flight and just describe the end
state.

> Fixes: 7a7e6edfca85 ("tools/testing/cxl: Add XOR Math support to cxl_test")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> 
> I realize this switcheroo is atypical for a patch. It seemed to be a
> cleaner repair, especially when trying to push this cxl/test change
> with the acpi driver patch in this set. Say the word and I'll split.
> 
> Also, I'm not looking to put back a 4-way HB interleave at the moment,
> because it's not a unique case (similar to 2-way in the calc) and also
> because I have a patchset in the works that supports all the HB interleave
> configs. This sets focus is repairing, not enhancing.

It's cxl_test, the rules are more relaxed so I think it is fine to say
"x4 window is dead, long live the new x3 window".

>  tools/testing/cxl/test/cxl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index a3cdbb2be038..e7e4afabc97a 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -329,14 +329,14 @@ static struct {
>  				.length = sizeof(mock_cedt.cfmws8),
>  			},
>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
> -			.interleave_ways = 2,
> +			.interleave_ways = 8,
>  			.granularity = 0,
>  			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = 0,
> -			.window_size = SZ_256M * 16UL,
> +			.window_size = SZ_256M * 12UL,
>  		},
> -		.target = { 0, 1, 0, 1, },
> +		.target = { 0, 1, 2 },

I tried this patch out and was surprised that none of the existing tests
noticed this change. That at least makes it easier to coordinate, but
was surprising.
diff mbox series

Patch

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index a3cdbb2be038..e7e4afabc97a 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -329,14 +329,14 @@  static struct {
 				.length = sizeof(mock_cedt.cfmws8),
 			},
 			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
-			.interleave_ways = 2,
+			.interleave_ways = 8,
 			.granularity = 0,
 			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = 0,
-			.window_size = SZ_256M * 16UL,
+			.window_size = SZ_256M * 12UL,
 		},
-		.target = { 0, 1, 0, 1, },
+		.target = { 0, 1, 2 },
 	},
 	.cxims0 = {
 		.cxims = {