Re: [RFC PATCH] tools/testing/cxl: Support multi-decoder shared-dport topology

[Alison Schofield]

On Thu, May 21, 2026 at 06:29:22PM -0700, Alison Schofield wrote:
> On Thu, May 21, 2026 at 04:48:06PM +0800, Richard Cheng wrote:
> > Add "multi_decoder" module param to the cxl_test emulator. When enabled,
> > mock_init_hdm_decoder() addtionally programs decoder X.1 on cxl_mem.0
> > and cxl_mem.4 as a second auto-region carved from the next
> > mock_auto_region_size of HPA in the same CFMWS window.
> > The result is 2 committed switch decoders that share the same
> > downstream-port target_map[].
> > 
> > This is inspired by [1] so the emulator can detect and test for scenario
> > like that.
> > 
> > [1]: https://lore.kernel.org/all/20260108101324.509667-1-rrichter@xxxxxxx/
> > Signed-off-by: Richard Cheng <icheng@xxxxxxxxxx>
> > ---
> 
> Hi Richard,
> 
> Welcome to cxl-test and thanks for the patch and the explanation.
> Before we add another module param and hardcoded auto-region, I
> want to walk through what's already available and see if it covers
> your need.
> 
> The fix for `3e8aaacdad4f` ("cxl/port: Fix target list setup for
> multiple decoders sharing the same dport") can be duplicated with
> the user create region path using cxl-test default topology as is,
> and that can be replayed as an auto region.
> 
> I've appended a script to demonstrate it. FWIW, I didn't know
> this for sure, and was wondering if the auto and user paths might
> diverge somewhere in this config, but they didn't. I was able to
> watch it fail without Roberts fix, then pass with it applied.
> 
> ICYMI there is a test case in cxl-topology.sh that detects the
> failure pattern but not a config to verify it like you point
> out here.
> 
> Your implemenation did make me aware of a gap in the region replay
> capability. On real HW, BIOS programs the decoders, then the OS walks
> the dports, and update_decoder_targets() is what populates target[]
> from target_map[]. But cxl-test bypasses that ordering. In cxl-test,
> mock_init_hdm_decoder() writes cxlsd->target[] itself on replay,
> which IIUC is why your patch called cxl_port_update_decoder_targets()
> from the mock to reach Robert's line. Lifting that call into the mock
> unconditionally, so replay goes through the same dport-enumeration
> path as real HW would widen what replay actually exercises. I'm not
> sure how we get there, but if you'd have interest and time to take
> a look at that, patches welcomed.

Hi Richard,

I've been working in this space, using region replay and found what
I said above started off true about the bypass, but the bypass does
not actually apply to region replay, only the default auto region.

In real HW ordering-
BIOS programs decoder and writes target_map[] and then the OS walks
dports and update_decoder_targets() reads that target_map[] and
populates cxlsd->target[].  

In cxl-test, the place we would need to fix to be more 'like' real
hardware is the initial mock auto region that gets set up on module load.
It is the path that takes a shortcut by writing cxlds->target[i] = dport
directly and setting target_map[i].

The cxl-test replay_path actually already matches real HW ordering. Target
lists are populated from the target_map[] during dport enumeration.

I still think there is a fix in there. The setup of that auto region
needs to set target_map[], but it needs to stop setting cxlsd->target[]
and let that happen 'naturally' when the dport enumeration happens.
There could be some ordering issue with when dport enumeration fires,
so could get more interesting than that.

It would be a good fixup to make the auto-region more 'authentic',
ie force it through more of the actual auto region driver paths.

-- Alison




> 
> Does the user-create followed by region replay flow cover your need? 
> 
> -- Alison
> From 0fcdebae75d307036d83f8b7be971db305b83660 Mon Sep 17 00:00:00 2001
> From: Alison Schofield <alison.schofield@xxxxxxxxx>
> Date: Thu, 21 May 2026 17:39:51 -0700
> Subject: [PATCH] WIP test/cxl-multi-decoder-shared-dport.sh
> 
> Signed-off-by: Alison Schofield <alison.schofield@xxxxxxxxx>
> ---
>  test/cxl-multi-decoder-shared-dport.sh | 139 +++++++++++++++++++++++++
>  test/meson.build                       |   2 +
>  2 files changed, 141 insertions(+)
>  create mode 100755 test/cxl-multi-decoder-shared-dport.sh
> 
> diff --git a/test/cxl-multi-decoder-shared-dport.sh b/test/cxl-multi-decoder-shared-dport.sh
> new file mode 100755
> index 000000000000..5f15b23f3a27
> --- /dev/null
> +++ b/test/cxl-multi-decoder-shared-dport.sh
> @@ -0,0 +1,139 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2026 Intel Corporation. All rights reserved.
> +
> +. $(dirname $0)/common
> +
> +rc=77
> +
> +set -ex
> +
> +trap 'err $LINENO' ERR
> +
> +check_prereq "jq"
> +
> +modprobe -r cxl_test
> +modprobe cxl_test
> +
> +rc=1
> +
> +# Regression test for kernel commit 3e8aaacdad4f ("cxl/port: Fix target
> +# list setup for multiple decoders sharing the same dport").
> +#
> +# Create two user-space regions on the same root decoder, claiming the
> +# same memdev set. Topology routes both regions through the same switch
> +# downstream ports, so each switch port ends up with two committed
> +# decoders sharing the same dport. cxl_test enumerates dports per
> +# memdev attach, which fires cxl_port_update_decoder_targets() against
> +# the already-committed decoders and lands on the line the kernel
> +# commit fixes. A pre-3e8aaacdad4f kernel stops after the first match
> +# and leaves the second decoder's cxlsd->target[] empty (nr_targets=0).
> +#
> +# replay_regions exercises the same shape after a cxl_acpi unbind/bind,
> +# so the regression is checked on both the user-create and replay
> +# paths.
> +
> +destroy_regions() {
> +	$CXL destroy-region -f -b cxl_test all
> +}
> +
> +# Pick the volatile-capable, 2-target root decoder. The cxl_test
> +# topology routes its 4 targets via 2 switches (HB0/sw0, HB0/sw1),
> +# 2 memdevs per switch. Both regions below claim all 4 memdevs, so
> +# they share the same switch dports by topology.
> +find_x4_ram_decoder() {
> +	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
> +		select(.volatile_capable == true) |
> +		select(.nr_targets == 2) |
> +		.decoder")
> +
> +	[[ $decoder ]] || err "$LINENO"
> +
> +	# Decoder window must hold two regions
> +	# Each region size is half the window.
> +	decoder_size=$($CXL list -d "$decoder" | jq -r '.[0].size')
> +	[[ -n "$decoder_size" && "$decoder_size" != "null" ]] || err "$LINENO"
> +	region_size=$((decoder_size / 2))
> +
> +	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
> +		.targets | .[] | select(.position == 0) | .target")
> +	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
> +		.targets | .[] | select(.position == 1) | .target")
> +
> +	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].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"
> +}
> +
> +create_region() {
> +	local size="$1"
> +	local region
> +
> +	region=$($CXL create-region -d "$decoder" -m -s "$size" "$memdevs" |
> +		jq -r ".region")
> +
> +	if [[ -z "$region" || "$region" == "null" ]]; then
> +		echo "create-region failed for $decoder"
> +		err "$LINENO"
> +	fi
> +	echo "$region"
> +}
> +
> +# Similar to cxl-topology.sh - test_switch_decoder_target_enumeration()
> +# Counts switch ports where >=2 decoders are programmed (one with
> +# targets, one or more without). After both regions are committed,
> +# every switch port we use should have >=2 decoders programmed.
> +check_shared_dport_enumeration() {
> +	local label="$1"
> +	local issues
> +
> +	issues=$($CXL list -b cxl_test -vvv | jq '
> +		[.. | objects | select(.depth == 2 and has("decoders:" + .port))] |
> +		map({
> +			nr_dports: .nr_dports,
> +			with_targets: ([to_entries[] |
> +				select(.key | startswith("decoders:")) |
> +				.value[] | select(has("mode") == false and
> +				.nr_targets > 0)] | length),
> +			without_targets: ([to_entries[] |
> +				select(.key | startswith("decoders:")) |
> +				.value[] | select(has("mode") == false and
> +				.nr_targets == 0)] | length)
> +		}) |
> +		map(select(.nr_dports > 0 and
> +			   .with_targets >= 1 and
> +			   .without_targets >= 1)) |
> +		length')
> +
> +	if ((issues != 0)); then
> +		echo "$label: $issues switch port(s) with incomplete target enumeration"
> +		echo "(>=1 decoder with targets, >=1 with nr_targets=0)"
> +		return 1
> +	fi
> +	echo "$label: switch decoder target enumeration OK"
> +	return 0
> +}
> +
> +# Start clean so we own all RAM capacity.
> +destroy_regions
> +
> +find_x4_ram_decoder
> +
> +region_a=$(create_region "$region_size")
> +region_b=$(create_region "$region_size")
> +
> +check_shared_dport_enumeration "after user-create" || err "$LINENO"
> +
> +replay_regions || err "$LINENO"
> +
> +check_shared_dport_enumeration "after replay" || err "$LINENO"
> +
> +# Destroy in reverse DPA order
> +$CXL destroy-region -f -b cxl_test "$region_b"
> +$CXL destroy-region -f -b cxl_test "$region_a"
> +
> +check_dmesg "$LINENO"
> +
> +modprobe -r cxl_test
> diff --git a/test/meson.build b/test/meson.build
> index e0e2193bfd51..4abad21102a7 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -171,6 +171,7 @@ cxl_translate = find_program('cxl-translate.sh')
>  cxl_elc = find_program('cxl-elc.sh')
>  cxl_dax_hmem = find_program('cxl-dax-hmem.sh')
>  cxl_region_replay = find_program('cxl-region-replay.sh')
> +cxl_multi_decoder_shared_dport = find_program('cxl-multi-decoder-shared-dport.sh')
>  
>  tests = [
>    [ 'libndctl',               libndctl,		  'ndctl' ],
> @@ -207,6 +208,7 @@ tests = [
>    [ 'cxl-elc.sh',             cxl_elc,            'cxl'   ],
>    [ 'cxl-dax-hmem.sh',        cxl_dax_hmem,       'cxl'   ],
>    [ 'cxl-region-replay.sh',   cxl_region_replay,  'cxl'   ],
> +  [ 'cxl-multi-decoder-shared-dport.sh', cxl_multi_decoder_shared_dport, 'cxl' ],
>  ]
>  
>  if get_option('destructive').enabled()
> -- 
> 2.37.3
> 
> 
> 
> 
> 
> 
> 
> >  tools/testing/cxl/test/cxl.c | 88 +++++++++++++++++++++++++++++++++---
> >  1 file changed, 81 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> > index 418669927fb0..929bbee471db 100644
> > --- a/tools/testing/cxl/test/cxl.c
> > +++ b/tools/testing/cxl/test/cxl.c
> > @@ -17,6 +17,7 @@
> >  static int interleave_arithmetic;
> >  static bool extended_linear_cache;
> >  static bool fail_autoassemble;
> > +static bool multi_decoder;
> >  
> >  #define FAKE_QTG_ID	42
> >  
> > @@ -1041,6 +1042,17 @@ static void default_mock_decoder(struct cxl_decoder *cxld)
> >  	WARN_ON_ONCE(!cxld_registry_new(cxld));
> >  }
> >  
> > +static int decoder_by_id(struct device *dev, const void *data)
> > +{
> > +	int target_id = (int)(uintptr_t)data;
> > +	struct cxl_decoder *cxld;
> > +
> > +	if (!is_switch_decoder(dev))
> > +		return 0;
> > +	cxld = to_cxl_decoder(dev);
> > +	return cxld->id == target_id;
> > +}
> > +
> >  static int first_decoder(struct device *dev, const void *data)
> >  {
> >  	struct cxl_decoder *cxld;
> > @@ -1079,6 +1091,8 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> >  	struct cxl_memdev *cxlmd;
> >  	struct cxl_dport *dport;
> >  	struct device *dev;
> > +	int max_decoder_id;
> > +	int region_decoder_id = 0;
> >  	bool hb0 = false;
> >  	u64 base;
> >  	int i;
> > @@ -1129,9 +1143,16 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> >  	 * assignment those devices are named cxl_mem.0, and cxl_mem.4.
> >  	 *
> >  	 * See 'cxl list -BMPu -m cxl_mem.0,cxl_mem.4'
> > +	 *
> > +	 * When multi_decoder is enabled, additionally program decoder
> > +	 * X.1 of the same endpoints as a second auto-region that shares
> > +	 * the switch's downstream ports with the first region, so that
> > +	 * the same dport ends up in target_map[] of two committed switch
> > +	 * decoders simultaneously.
> >  	 */
> > +	max_decoder_id = multi_decoder ? 1 : 0;
> >  	if (!is_endpoint_decoder(&cxld->dev) || !hb0 || pdev->id % 4 ||
> > -	    pdev->id > 4 || cxld->id > 0) {
> > +	    pdev->id > 4 || cxld->id > max_decoder_id) {
> >  		default_mock_decoder(cxld);
> >  		return false;
> >  	}
> > @@ -1142,9 +1163,14 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> >  		return false;
> >  	}
> >  
> > +	region_decoder_id = cxld->id;
> > +
> >  	base = window->base_hpa;
> >  	if (extended_linear_cache)
> >  		base += mock_auto_region_size;
> > +	/* Place the second auto-region right after the first. */
> > +	if (region_decoder_id == 1)
> > +		base += mock_auto_region_size;
> >  	cxld->hpa_range = (struct range) {
> >  		.start = base,
> >  		.end = base + mock_auto_region_size - 1,
> > @@ -1156,7 +1182,9 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> >  	cxld->flags = CXL_DECODER_F_ENABLE;
> >  	cxled->state = CXL_DECODER_STATE_AUTO;
> >  	port->commit_end = cxld->id;
> > -	devm_cxl_dpa_reserve(cxled, 0,
> > +	devm_cxl_dpa_reserve(cxled,
> > +			     region_decoder_id *
> > +				 (mock_auto_region_size / 2),
> >  			     mock_auto_region_size / cxld->interleave_ways, 0);
> >  	cxld->commit = mock_decoder_commit;
> >  	cxld->reset = mock_decoder_reset;
> > @@ -1165,12 +1193,23 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> >  	/*
> >  	 * Now that endpoint decoder is set up, walk up the hierarchy
> >  	 * and setup the switch and root port decoders targeting @cxlmd.
> > +	 *
> > +	 * For region 0 use switch-decoder slot 0 (selected by
> > +	 * first_decoder()); for region 1 use slot 1 so that the two
> > +	 * regions exercise the multi-decoder / shared-dport scenario.
> >  	 */
> >  	iter = port;
> >  	for (i = 0; i < 2; i++) {
> >  		dport = iter->parent_dport;
> >  		iter = dport->port;
> > -		dev = device_find_child(&iter->dev, NULL, first_decoder);
> > +		if (region_decoder_id == 0) {
> > +			dev = device_find_child(&iter->dev, NULL,
> > +						first_decoder);
> > +		} else {
> > +			dev = device_find_child(&iter->dev,
> > +						(void *)(uintptr_t)region_decoder_id,
> > +						decoder_by_id);
> > +		}
> >  		/*
> >  		 * Ancestor ports are guaranteed to be enumerated before
> >  		 * @port, and all ports have at least one decoder.
> > @@ -1179,23 +1218,36 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> >  			continue;
> >  
> >  		cxlsd = to_cxl_switch_decoder(dev);
> > +		/*
> > +		 * Region 0 (decoder.0) keeps the historical shortcut of
> > +		 * stamping target[] directly so single-region setup is
> > +		 * unaffected by dport-add ordering quirks of cxl_test.
> > +		 *
> > +		 * Region 1 (decoder.1, only programmed when
> > +		 * multi_decoder=1) intentionally leaves target[]
> > +		 * to update_decoder_targets() at dport-add time.
> > +		 */
> >  		if (i == 0) {
> >  			/* put cxl_mem.4 second in the decode order */
> >  			if (pdev->id == 4) {
> > -				cxlsd->target[1] = dport;
> > +				if (region_decoder_id == 0)
> > +					cxlsd->target[1] = dport;
> >  				cxlsd->cxld.target_map[1] = dport->port_id;
> >  			} else {
> > -				cxlsd->target[0] = dport;
> > +				if (region_decoder_id == 0)
> > +					cxlsd->target[0] = dport;
> >  				cxlsd->cxld.target_map[0] = dport->port_id;
> >  			}
> >  		} else {
> > -			cxlsd->target[0] = dport;
> > +			if (region_decoder_id == 0)
> > +				cxlsd->target[0] = dport;
> >  			cxlsd->cxld.target_map[0] = dport->port_id;
> >  		}
> >  		cxld = &cxlsd->cxld;
> >  		cxld->target_type = CXL_DECODER_HOSTONLYMEM;
> >  		cxld->flags = CXL_DECODER_F_ENABLE;
> > -		iter->commit_end = 0;
> > +		if (iter->commit_end < region_decoder_id)
> > +			iter->commit_end = region_decoder_id;
> >  		/*
> >  		 * Switch targets 2 endpoints, while host bridge targets
> >  		 * one root port
> > @@ -1212,6 +1264,26 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
> >  		cxld->commit = mock_decoder_commit;
> >  		cxld->reset = mock_decoder_reset;
> >  
> > +		/*
> > +		 * For the second region only target_map[] is set above,
> > +		 * not cxlsd->target[]. On real hardware target[] gets
> > +		 * populated when dports are added (BIOS-programmed
> > +		 * decoders are read first, then dports get enumerated).
> > +		 * cxl_test's order is reversed: dports were added long
> > +		 * before this point, so re-fire the update here against
> > +		 * each existing dport to mimic the real-hardware
> > +		 * ordering and exercise update_decoder_targets() with
> > +		 * pre-programmed decoders.
> > +		 */
> > +		if (region_decoder_id == 1) {
> > +			struct cxl_dport *existing;
> > +			unsigned long index;
> > +
> > +			xa_for_each(&iter->dports, index, existing)
> > +				cxl_port_update_decoder_targets(iter,
> > +								existing);
> > +		}
> > +
> >  		cxld_registry_update(cxld);
> >  		put_device(dev);
> >  	}
> > @@ -2049,6 +2121,8 @@ module_param(extended_linear_cache, bool, 0444);
> >  MODULE_PARM_DESC(extended_linear_cache, "Enable extended linear cache support");
> >  module_param(fail_autoassemble, bool, 0444);
> >  MODULE_PARM_DESC(fail_autoassemble, "Simulate missing member of an auto-region");
> > +module_param(multi_decoder, bool, 0444);
> > +MODULE_PARM_DESC(multi_decoder, "Auto-program a 2nd decoder per endpoint sharing switch dports");
> >  module_init(cxl_test_init);
> >  module_exit(cxl_test_exit);
> >  MODULE_LICENSE("GPL v2");
> > -- 
> > 2.43.0
> > 
>