* Re: [PATCH 3/3] tools/testing/cxl: Document test configurations
2023-05-17 21:28 ` [PATCH 3/3] tools/testing/cxl: Document test configurations Ira Weiny
@ 2023-05-17 21:31 ` Dave Jiang
2023-05-18 9:50 ` Jonathan Cameron
2023-09-16 7:03 ` Dan Williams
2 siblings, 0 replies; 15+ messages in thread
From: Dave Jiang @ 2023-05-17 21:31 UTC (permalink / raw)
To: Ira Weiny, Dan Williams, Alison Schofield, Vishal Verma,
Ben Widawsky, Jonathan Cameron
Cc: linux-cxl, linux-kernel
On 5/17/23 2:28 PM, Ira Weiny wrote:
> The devices created, their relationship, and intended testing purpose is
> not extremely clear, especially for those unfamiliar with cxl-test.
>
> Document the purpose of each hierarchy. Add ASCII art to show the
> relationship of devices. Group the device declarations together based
> on the hierarchies.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Nice artwork :)
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> tools/testing/cxl/test/cxl.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index bf00dc52fe96..bd38a5fb60ae 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -23,6 +23,31 @@ static int interleave_arithmetic;
> #define NR_CXL_PORT_DECODERS 8
> #define NR_BRIDGES (NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + NR_CXL_RCH)
>
> +/*
> + * Interleave testing
> + *
> + * +---------------+ +---------------+
> + * | host_bridge[0]| | host_bridge[1]|
> + * +-/---------\---+ +--/---------\--+
> + * /- -\ /- -\
> + * /- -\ /- -\
> + * +-------------+ +-------------+ +-------------+ +-------------+
> + * |root_port[0] | |root_port[1] | |root_port[2] | |root_port[3] |
> + * +------|------+ +------|------+ +------|------+ +------|------+
> + * | | | |
> + * +-------|-------+ +-------|-------+ +-------|-------+ +-------|-------+
> + * |switch_uport[0]| |switch_uport[1]| |switch_uport[2]| |switch_uport[3]|
> + * +---|-------|---+ +---/-------|---+ +---/-------|---+ +---|-------\---+
> + * | \ / \ / \ / \
> + * +----|----++--|------++---------++----|----++---------++----|----++----|----++---------+
> + * |switch ||switch ||switch ||switch ||switch ||switch ||switch ||switch |
> + * |_dport[0]||_dport[1]||_dport[2]||_dport[3]||_dport[4]||_dport[5]||_dport[6]||_dport[7]|
> + * +----|----++--|------++----|----++----|----++----|----++----|----++----|----++----|----+
> + * | | | | | | | |
> + * +---|--+ +-|----+ +---|--+ +---|--+ +--|---+ +---|--+ +---|--+ +---|--+
> + * |mem[0]| |mem[1]| |mem[2]| |mem[3]| |mem[4]| |mem[5]| |mem[6]| |mem[7]|
> + * +------+ +------+ +------+ +------+ +------+ +------+ +------+ +------+
> + */
> static struct platform_device *cxl_acpi;
> static struct platform_device *cxl_host_bridge[NR_CXL_HOST_BRIDGES];
> #define NR_MULTI_ROOT (NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS)
> @@ -31,16 +56,51 @@ static struct platform_device *cxl_switch_uport[NR_MULTI_ROOT];
> #define NR_MEM_MULTI \
> (NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS * NR_CXL_SWITCH_PORTS)
> static struct platform_device *cxl_switch_dport[NR_MEM_MULTI];
> +struct platform_device *cxl_mem[NR_MEM_MULTI];
>
> +/*
> + * 1) Preconfigured region support (Simulated BIOS configured region)
> + * 2) 'Pass-through' decoder
> + *
> + * +---------------+
> + * | hb_single |
> + * +------|--------+
> + * |
> + * +------|--------+
> + * | root_single |
> + * +------|--------+
> + * |
> + * +----------|----------+
> + * | swu_single |
> + * +-----|-----------|---+
> + * | |
> + * +-----|-----+ +--|--------+
> + * |swd_single | | swd_single|
> + * +-----|-----+ +----|------+
> + * | |
> + * +------|-----+ +----|-------+
> + * |mem_single | |mem_single |
> + * +------------+ +------------+
> + */
> static struct platform_device *cxl_hb_single[NR_CXL_SINGLE_HOST];
> static struct platform_device *cxl_root_single[NR_CXL_SINGLE_HOST];
> static struct platform_device *cxl_swu_single[NR_CXL_SINGLE_HOST];
> #define NR_MEM_SINGLE (NR_CXL_SINGLE_HOST * NR_CXL_SWITCH_PORTS)
> static struct platform_device *cxl_swd_single[NR_MEM_SINGLE];
> -
> -struct platform_device *cxl_mem[NR_MEM_MULTI];
> struct platform_device *cxl_mem_single[NR_MEM_SINGLE];
>
> +/*
> + * 1) RCD
> + * 2) Type-2 (Accelerator)
> + *
> + * +-----+
> + * | rch |
> + * +--|--+
> + * |
> + * +-|--+
> + * |rcd |
> + * +----+
> + */
> static struct platform_device *cxl_rch[NR_CXL_RCH];
> static struct platform_device *cxl_rcd[NR_CXL_RCH];
>
> @@ -64,6 +124,17 @@ static inline bool is_single_bridge(struct device *dev)
> return false;
> }
>
> +/*
> + * +---------------+ +---------------+
> + * | host_bridge[0]| | host_bridge[1]|
> + * +---------------+ +---------------+
> + * +---------------+
> + * | hb_single | (host_bridge[2])
> + * +---------------+
> + * +-----+
> + * | rch | (host_bridge[3])
> + * +-----+
> + */
> static struct acpi_device acpi0017_mock;
> static struct acpi_device host_bridge[NR_BRIDGES] = {
> [0] = {
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] tools/testing/cxl: Document test configurations
2023-05-17 21:28 ` [PATCH 3/3] tools/testing/cxl: Document test configurations Ira Weiny
2023-05-17 21:31 ` Dave Jiang
@ 2023-05-18 9:50 ` Jonathan Cameron
2023-05-18 9:51 ` Jonathan Cameron
2023-05-18 14:36 ` Ira Weiny
2023-09-16 7:03 ` Dan Williams
2 siblings, 2 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-18 9:50 UTC (permalink / raw)
To: Ira Weiny
Cc: Dan Williams, Alison Schofield, Vishal Verma, Dave Jiang,
Ben Widawsky, linux-cxl, linux-kernel
On Wed, 17 May 2023 14:28:12 -0700
Ira Weiny <ira.weiny@intel.com> wrote:
> The devices created, their relationship, and intended testing purpose is
> not extremely clear, especially for those unfamiliar with cxl-test.
>
> Document the purpose of each hierarchy. Add ASCII art to show the
> relationship of devices. Group the device declarations together based
> on the hierarchies.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Trivial nitpicks below :)
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> tools/testing/cxl/test/cxl.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index bf00dc52fe96..bd38a5fb60ae 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -23,6 +23,31 @@ static int interleave_arithmetic;
> #define NR_CXL_PORT_DECODERS 8
> #define NR_BRIDGES (NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + NR_CXL_RCH)
>
> +/*
> + * Interleave testing
Doesn't include the cfmws, which will be tricky to draw, but maybe you could
add something to indicate they interleave over the two HB sometimes?
> + *
> + * +---------------+ +---------------+
> + * | host_bridge[0]| | host_bridge[1]|
> + * +-/---------\---+ +--/---------\--+
Text for host bridges is right aligned.
> + * /- -\ /- -\
> + * /- -\ /- -\
> + * +-------------+ +-------------+ +-------------+ +-------------+
> + * |root_port[0] | |root_port[1] | |root_port[2] | |root_port[3] |
> + * +------|------+ +------|------+ +------|------+ +------|------+
and root ports are left aligned.
I'd shrink both boxes so they are same as the switches below - or expand them to give
a space on either side of the text.
> + * | | | |
> + * +-------|-------+ +-------|-------+ +-------|-------+ +-------|-------+
> + * |switch_uport[0]| |switch_uport[1]| |switch_uport[2]| |switch_uport[3]|
> + * +---|-------|---+ +---/-------|---+ +---/-------|---+ +---|-------\---+
> + * | \ / \ / \ / \
> + * +----|----++--|------++---------++----|----++---------++----|----++----|----++---------+
> + * |switch ||switch ||switch ||switch ||switch ||switch ||switch ||switch |
> + * |_dport[0]||_dport[1]||_dport[2]||_dport[3]||_dport[4]||_dport[5]||_dport[6]||_dport[7]|
> + * +----|----++--|------++----|----++----|----++----|----++----|----++----|----++----|----+
> + * | | | | | | | |
> + * +---|--+ +-|----+ +---|--+ +---|--+ +--|---+ +---|--+ +---|--+ +---|--+
> + * |mem[0]| |mem[1]| |mem[2]| |mem[3]| |mem[4]| |mem[5]| |mem[6]| |mem[7]|
> + * +------+ +------+ +------+ +------+ +------+ +------+ +------+ +------+
> + */
> static struct platform_device *cxl_acpi;
> static struct platform_device *cxl_host_bridge[NR_CXL_HOST_BRIDGES];
> #define NR_MULTI_ROOT (NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS)
> @@ -31,16 +56,51 @@ static struct platform_device *cxl_switch_uport[NR_MULTI_ROOT];
> #define NR_MEM_MULTI \
> (NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS * NR_CXL_SWITCH_PORTS)
> static struct platform_device *cxl_switch_dport[NR_MEM_MULTI];
> +struct platform_device *cxl_mem[NR_MEM_MULTI];
>
> +/*
> + * 1) Preconfigured region support (Simulated BIOS configured region)
> + * 2) 'Pass-through' decoder
> + *
> + * +---------------+
> + * | hb_single |
> + * +------|--------+
> + * |
> + * +------|--------+
> + * | root_single |
> + * +------|--------+
> + * |
> + * +----------|----------+
> + * | swu_single |
> + * +-----|-----------|---+
> + * | |
> + * +-----|-----+ +--|--------+
> + * |swd_single | | swd_single|
> + * +-----|-----+ +----|------+
> + * | |
> + * +------|-----+ +----|-------+
> + * |mem_single | |mem_single |
> + * +------------+ +------------+
mem[0] etc? Also swd_single[0] etc?
For consistency with above.
> + */
> static struct platform_device *cxl_hb_single[NR_CXL_SINGLE_HOST];
> static struct platform_device *cxl_root_single[NR_CXL_SINGLE_HOST];
> static struct platform_device *cxl_swu_single[NR_CXL_SINGLE_HOST];
> #define NR_MEM_SINGLE (NR_CXL_SINGLE_HOST * NR_CXL_SWITCH_PORTS)
> static struct platform_device *cxl_swd_single[NR_MEM_SINGLE];
> -
> -struct platform_device *cxl_mem[NR_MEM_MULTI];
> struct platform_device *cxl_mem_single[NR_MEM_SINGLE];
>
> +/*
> + * 1) RCD
> + * 2) Type-2 (Accelerator)
> + *
> + * +-----+
> + * | rch |
> + * +--|--+
> + * |
> + * +-|--+
> + * |rcd |
> + * +----+
> + */
> static struct platform_device *cxl_rch[NR_CXL_RCH];
> static struct platform_device *cxl_rcd[NR_CXL_RCH];
>
> @@ -64,6 +124,17 @@ static inline bool is_single_bridge(struct device *dev)
> return false;
> }
>
> +/*
> + * +---------------+ +---------------+
> + * | host_bridge[0]| | host_bridge[1]|
> + * +---------------+ +---------------+
> + * +---------------+
> + * | hb_single | (host_bridge[2])
> + * +---------------+
> + * +-----+
> + * | rch | (host_bridge[3])
> + * +-----+
> + */
Not sure what this diagram is illustrating...
> static struct acpi_device acpi0017_mock;
> static struct acpi_device host_bridge[NR_BRIDGES] = {
> [0] = {
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] tools/testing/cxl: Document test configurations
2023-05-18 9:50 ` Jonathan Cameron
@ 2023-05-18 9:51 ` Jonathan Cameron
2023-05-18 14:36 ` Ira Weiny
1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-18 9:51 UTC (permalink / raw)
To: Ira Weiny
Cc: Dan Williams, Alison Schofield, Vishal Verma, Dave Jiang,
Ben Widawsky, linux-cxl, linux-kernel
On Thu, 18 May 2023 10:50:20 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> On Wed, 17 May 2023 14:28:12 -0700
> Ira Weiny <ira.weiny@intel.com> wrote:
>
> > The devices created, their relationship, and intended testing purpose is
> > not extremely clear, especially for those unfamiliar with cxl-test.
> >
> > Document the purpose of each hierarchy. Add ASCII art to show the
> > relationship of devices. Group the device declarations together based
> > on the hierarchies.
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>
> Trivial nitpicks below :)
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Actually scrub that RB - the last question on what the final diagram means
probably makes an RB inappropriate for now...
>
> > ---
> > tools/testing/cxl/test/cxl.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 73 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> > index bf00dc52fe96..bd38a5fb60ae 100644
> > --- a/tools/testing/cxl/test/cxl.c
> > +++ b/tools/testing/cxl/test/cxl.c
> > @@ -23,6 +23,31 @@ static int interleave_arithmetic;
> > #define NR_CXL_PORT_DECODERS 8
> > #define NR_BRIDGES (NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + NR_CXL_RCH)
> >
> > +/*
> > + * Interleave testing
>
> Doesn't include the cfmws, which will be tricky to draw, but maybe you could
> add something to indicate they interleave over the two HB sometimes?
>
> > + *
> > + * +---------------+ +---------------+
> > + * | host_bridge[0]| | host_bridge[1]|
> > + * +-/---------\---+ +--/---------\--+
> Text for host bridges is right aligned.
> > + * /- -\ /- -\
> > + * /- -\ /- -\
> > + * +-------------+ +-------------+ +-------------+ +-------------+
> > + * |root_port[0] | |root_port[1] | |root_port[2] | |root_port[3] |
> > + * +------|------+ +------|------+ +------|------+ +------|------+
> and root ports are left aligned.
> I'd shrink both boxes so they are same as the switches below - or expand them to give
> a space on either side of the text.
>
>
> > + * | | | |
> > + * +-------|-------+ +-------|-------+ +-------|-------+ +-------|-------+
> > + * |switch_uport[0]| |switch_uport[1]| |switch_uport[2]| |switch_uport[3]|
> > + * +---|-------|---+ +---/-------|---+ +---/-------|---+ +---|-------\---+
> > + * | \ / \ / \ / \
> > + * +----|----++--|------++---------++----|----++---------++----|----++----|----++---------+
> > + * |switch ||switch ||switch ||switch ||switch ||switch ||switch ||switch |
> > + * |_dport[0]||_dport[1]||_dport[2]||_dport[3]||_dport[4]||_dport[5]||_dport[6]||_dport[7]|
> > + * +----|----++--|------++----|----++----|----++----|----++----|----++----|----++----|----+
> > + * | | | | | | | |
> > + * +---|--+ +-|----+ +---|--+ +---|--+ +--|---+ +---|--+ +---|--+ +---|--+
> > + * |mem[0]| |mem[1]| |mem[2]| |mem[3]| |mem[4]| |mem[5]| |mem[6]| |mem[7]|
> > + * +------+ +------+ +------+ +------+ +------+ +------+ +------+ +------+
> > + */
> > static struct platform_device *cxl_acpi;
> > static struct platform_device *cxl_host_bridge[NR_CXL_HOST_BRIDGES];
> > #define NR_MULTI_ROOT (NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS)
> > @@ -31,16 +56,51 @@ static struct platform_device *cxl_switch_uport[NR_MULTI_ROOT];
> > #define NR_MEM_MULTI \
> > (NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS * NR_CXL_SWITCH_PORTS)
> > static struct platform_device *cxl_switch_dport[NR_MEM_MULTI];
> > +struct platform_device *cxl_mem[NR_MEM_MULTI];
> >
> > +/*
> > + * 1) Preconfigured region support (Simulated BIOS configured region)
> > + * 2) 'Pass-through' decoder
> > + *
> > + * +---------------+
> > + * | hb_single |
> > + * +------|--------+
> > + * |
> > + * +------|--------+
> > + * | root_single |
> > + * +------|--------+
> > + * |
> > + * +----------|----------+
> > + * | swu_single |
> > + * +-----|-----------|---+
> > + * | |
> > + * +-----|-----+ +--|--------+
> > + * |swd_single | | swd_single|
> > + * +-----|-----+ +----|------+
> > + * | |
> > + * +------|-----+ +----|-------+
> > + * |mem_single | |mem_single |
> > + * +------------+ +------------+
> mem[0] etc? Also swd_single[0] etc?
>
> For consistency with above.
>
> > + */
> > static struct platform_device *cxl_hb_single[NR_CXL_SINGLE_HOST];
> > static struct platform_device *cxl_root_single[NR_CXL_SINGLE_HOST];
> > static struct platform_device *cxl_swu_single[NR_CXL_SINGLE_HOST];
> > #define NR_MEM_SINGLE (NR_CXL_SINGLE_HOST * NR_CXL_SWITCH_PORTS)
> > static struct platform_device *cxl_swd_single[NR_MEM_SINGLE];
> > -
> > -struct platform_device *cxl_mem[NR_MEM_MULTI];
> > struct platform_device *cxl_mem_single[NR_MEM_SINGLE];
> >
> > +/*
> > + * 1) RCD
> > + * 2) Type-2 (Accelerator)
> > + *
> > + * +-----+
> > + * | rch |
> > + * +--|--+
> > + * |
> > + * +-|--+
> > + * |rcd |
> > + * +----+
> > + */
> > static struct platform_device *cxl_rch[NR_CXL_RCH];
> > static struct platform_device *cxl_rcd[NR_CXL_RCH];
> >
> > @@ -64,6 +124,17 @@ static inline bool is_single_bridge(struct device *dev)
> > return false;
> > }
> >
> > +/*
> > + * +---------------+ +---------------+
> > + * | host_bridge[0]| | host_bridge[1]|
> > + * +---------------+ +---------------+
> > + * +---------------+
> > + * | hb_single | (host_bridge[2])
> > + * +---------------+
> > + * +-----+
> > + * | rch | (host_bridge[3])
> > + * +-----+
> > + */
>
> Not sure what this diagram is illustrating...
>
> > static struct acpi_device acpi0017_mock;
> > static struct acpi_device host_bridge[NR_BRIDGES] = {
> > [0] = {
> >
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] tools/testing/cxl: Document test configurations
2023-05-18 9:50 ` Jonathan Cameron
2023-05-18 9:51 ` Jonathan Cameron
@ 2023-05-18 14:36 ` Ira Weiny
1 sibling, 0 replies; 15+ messages in thread
From: Ira Weiny @ 2023-05-18 14:36 UTC (permalink / raw)
To: Jonathan Cameron, Ira Weiny
Cc: Dan Williams, Alison Schofield, Vishal Verma, Dave Jiang,
Ben Widawsky, linux-cxl, linux-kernel
Jonathan Cameron wrote:
> On Wed, 17 May 2023 14:28:12 -0700
> Ira Weiny <ira.weiny@intel.com> wrote:
>
[snip]
> > ---
> > tools/testing/cxl/test/cxl.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 73 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> > index bf00dc52fe96..bd38a5fb60ae 100644
> > --- a/tools/testing/cxl/test/cxl.c
> > +++ b/tools/testing/cxl/test/cxl.c
> > @@ -23,6 +23,31 @@ static int interleave_arithmetic;
> > #define NR_CXL_PORT_DECODERS 8
> > #define NR_BRIDGES (NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + NR_CXL_RCH)
> >
> > +/*
> > + * Interleave testing
>
> Doesn't include the cfmws, which will be tricky to draw, but maybe you could
> add something to indicate they interleave over the two HB sometimes?
I was mainly looking to document the devices below. Because they are all
'platform_device' and they are assigned type in the code which made things
a bit harder for me to follow when I was going through it the other day.
>
> > + *
> > + * +---------------+ +---------------+
> > + * | host_bridge[0]| | host_bridge[1]|
> > + * +-/---------\---+ +--/---------\--+
> Text for host bridges is right aligned.
Ah true. I used an online ascii editor for these. :-D So I did not pay
any attention when I copied pasted.
> > + * /- -\ /- -\
> > + * /- -\ /- -\
> > + * +-------------+ +-------------+ +-------------+ +-------------+
> > + * |root_port[0] | |root_port[1] | |root_port[2] | |root_port[3] |
> > + * +------|------+ +------|------+ +------|------+ +------|------+
> and root ports are left aligned.
> I'd shrink both boxes so they are same as the switches below - or expand them to give
> a space on either side of the text.
Done.
> >
> > +/*
> > + * 1) Preconfigured region support (Simulated BIOS configured region)
> > + * 2) 'Pass-through' decoder
> > + *
> > + * +---------------+
> > + * | hb_single |
> > + * +------|--------+
> > + * |
> > + * +------|--------+
> > + * | root_single |
> > + * +------|--------+
> > + * |
> > + * +----------|----------+
> > + * | swu_single |
> > + * +-----|-----------|---+
> > + * | |
> > + * +-----|-----+ +--|--------+
> > + * |swd_single | | swd_single|
> > + * +-----|-----+ +----|------+
> > + * | |
> > + * +------|-----+ +----|-------+
> > + * |mem_single | |mem_single |
> > + * +------------+ +------------+
> mem[0] etc? Also swd_single[0] etc?
>
> For consistency with above.
>
Actually mem_single[0,1]. yea swd_single[0,1].
> >
> > +/*
> > + * +---------------+ +---------------+
> > + * | host_bridge[0]| | host_bridge[1]|
> > + * +---------------+ +---------------+
> > + * +---------------+
> > + * | hb_single | (host_bridge[2])
> > + * +---------------+
> > + * +-----+
> > + * | rch | (host_bridge[3])
> > + * +-----+
> > + */
>
> Not sure what this diagram is illustrating...
Just showing how the acpi_devices array below ties in with the above
diagrams. Mainly that their is not a 1:1 corelation between
cxl_host_bridge[] and host_bridge[]. That index 2 and 3 are other
platform devices as shown.
I could probably make that equivalency note in the diagrams above where
hb_single and rch are defined/documented.
Let me do that.
Ira
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 3/3] tools/testing/cxl: Document test configurations
2023-05-17 21:28 ` [PATCH 3/3] tools/testing/cxl: Document test configurations Ira Weiny
2023-05-17 21:31 ` Dave Jiang
2023-05-18 9:50 ` Jonathan Cameron
@ 2023-09-16 7:03 ` Dan Williams
2023-09-18 17:31 ` Ira Weiny
2 siblings, 1 reply; 15+ messages in thread
From: Dan Williams @ 2023-09-16 7:03 UTC (permalink / raw)
To: Ira Weiny, Dan Williams, Alison Schofield, Vishal Verma,
Dave Jiang, Ben Widawsky, Jonathan Cameron
Cc: linux-cxl, linux-kernel, Ira Weiny
Ira Weiny wrote:
> The devices created, their relationship, and intended testing purpose is
> not extremely clear, especially for those unfamiliar with cxl-test.
>
> Document the purpose of each hierarchy. Add ASCII art to show the
> relationship of devices. Group the device declarations together based
> on the hierarchies.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
> tools/testing/cxl/test/cxl.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index bf00dc52fe96..bd38a5fb60ae 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -23,6 +23,31 @@ static int interleave_arithmetic;
> #define NR_CXL_PORT_DECODERS 8
> #define NR_BRIDGES (NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + NR_CXL_RCH)
>
> +/*
> + * Interleave testing
> + *
> + * +---------------+ +---------------+
> + * | host_bridge[0]| | host_bridge[1]|
> + * +-/---------\---+ +--/---------\--+
> + * /- -\ /- -\
> + * /- -\ /- -\
> + * +-------------+ +-------------+ +-------------+ +-------------+
> + * |root_port[0] | |root_port[1] | |root_port[2] | |root_port[3] |
> + * +------|------+ +------|------+ +------|------+ +------|------+
> + * | | | |
> + * +-------|-------+ +-------|-------+ +-------|-------+ +-------|-------+
> + * |switch_uport[0]| |switch_uport[1]| |switch_uport[2]| |switch_uport[3]|
> + * +---|-------|---+ +---/-------|---+ +---/-------|---+ +---|-------\---+
> + * | \ / \ / \ / \
> + * +----|----++--|------++---------++----|----++---------++----|----++----|----++---------+
> + * |switch ||switch ||switch ||switch ||switch ||switch ||switch ||switch |
> + * |_dport[0]||_dport[1]||_dport[2]||_dport[3]||_dport[4]||_dport[5]||_dport[6]||_dport[7]|
> + * +----|----++--|------++----|----++----|----++----|----++----|----++----|----++----|----+
> + * | | | | | | | |
> + * +---|--+ +-|----+ +---|--+ +---|--+ +--|---+ +---|--+ +---|--+ +---|--+
> + * |mem[0]| |mem[1]| |mem[2]| |mem[3]| |mem[4]| |mem[5]| |mem[6]| |mem[7]|
> + * +------+ +------+ +------+ +------+ +------+ +------+ +------+ +------+
> + */
Circling back to merge this I realize that the numbering is off. For
example a snippet from "cxl list -BPT -b cxl_test"
"ports:root3":[
{
"port":"port5",
"host":"cxl_host_bridge.1",
"depth":1,
"nr_dports":2,
"dports":[
{
"dport":"cxl_root_port.1",
"id":1
},
{
"dport":"cxl_root_port.3",
"id":3
}
],
This is due to the modulo math at setup time. I only noticed this
because I wanted a diagram to refer to when doing some recent
extensions.
I wonder if we could just use "cxl list" to maintain this diagram, or
maybe circle back and use this to keep an image up to date on a web page
somewhere:
https://lore.kernel.org/linux-cxl/cover.1660895649.git.sunfishho12@gmail.com/
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH 3/3] tools/testing/cxl: Document test configurations
2023-09-16 7:03 ` Dan Williams
@ 2023-09-18 17:31 ` Ira Weiny
0 siblings, 0 replies; 15+ messages in thread
From: Ira Weiny @ 2023-09-18 17:31 UTC (permalink / raw)
To: Dan Williams, Ira Weiny, Alison Schofield, Vishal Verma,
Dave Jiang, Ben Widawsky, Jonathan Cameron, sunfishho12@gmail.com
Cc: linux-cxl, linux-kernel, Ira Weiny
+Matthew Ho
Dan Williams wrote:
> Ira Weiny wrote:
> > The devices created, their relationship, and intended testing purpose is
> > not extremely clear, especially for those unfamiliar with cxl-test.
> >
> > Document the purpose of each hierarchy. Add ASCII art to show the
> > relationship of devices. Group the device declarations together based
> > on the hierarchies.
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> > tools/testing/cxl/test/cxl.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 73 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> > index bf00dc52fe96..bd38a5fb60ae 100644
> > --- a/tools/testing/cxl/test/cxl.c
> > +++ b/tools/testing/cxl/test/cxl.c
> > @@ -23,6 +23,31 @@ static int interleave_arithmetic;
> > #define NR_CXL_PORT_DECODERS 8
> > #define NR_BRIDGES (NR_CXL_HOST_BRIDGES + NR_CXL_SINGLE_HOST + NR_CXL_RCH)
> >
> > +/*
> > + * Interleave testing
> > + *
> > + * +---------------+ +---------------+
> > + * | host_bridge[0]| | host_bridge[1]|
> > + * +-/---------\---+ +--/---------\--+
> > + * /- -\ /- -\
> > + * /- -\ /- -\
> > + * +-------------+ +-------------+ +-------------+ +-------------+
> > + * |root_port[0] | |root_port[1] | |root_port[2] | |root_port[3] |
> > + * +------|------+ +------|------+ +------|------+ +------|------+
> > + * | | | |
> > + * +-------|-------+ +-------|-------+ +-------|-------+ +-------|-------+
> > + * |switch_uport[0]| |switch_uport[1]| |switch_uport[2]| |switch_uport[3]|
> > + * +---|-------|---+ +---/-------|---+ +---/-------|---+ +---|-------\---+
> > + * | \ / \ / \ / \
> > + * +----|----++--|------++---------++----|----++---------++----|----++----|----++---------+
> > + * |switch ||switch ||switch ||switch ||switch ||switch ||switch ||switch |
> > + * |_dport[0]||_dport[1]||_dport[2]||_dport[3]||_dport[4]||_dport[5]||_dport[6]||_dport[7]|
> > + * +----|----++--|------++----|----++----|----++----|----++----|----++----|----++----|----+
> > + * | | | | | | | |
> > + * +---|--+ +-|----+ +---|--+ +---|--+ +--|---+ +---|--+ +---|--+ +---|--+
> > + * |mem[0]| |mem[1]| |mem[2]| |mem[3]| |mem[4]| |mem[5]| |mem[6]| |mem[7]|
> > + * +------+ +------+ +------+ +------+ +------+ +------+ +------+ +------+
> > + */
>
> Circling back to merge this I realize that the numbering is off. For
> example a snippet from "cxl list -BPT -b cxl_test"
>
> "ports:root3":[
> {
> "port":"port5",
> "host":"cxl_host_bridge.1",
> "depth":1,
> "nr_dports":2,
> "dports":[
> {
> "dport":"cxl_root_port.1",
> "id":1
> },
> {
> "dport":"cxl_root_port.3",
> "id":3
> }
> ],
>
> This is due to the modulo math at setup time. I only noticed this
> because I wanted a diagram to refer to when doing some recent
> extensions.
:-( I did not realize this detail.
>
> I wonder if we could just use "cxl list" to maintain this diagram, or
> maybe circle back and use this to keep an image up to date on a web page
> somewhere:
>
> https://lore.kernel.org/linux-cxl/cover.1660895649.git.sunfishho12@gmail.com/
I forgot about this and this is a nice idea. Did that support land?
Ira
^ permalink raw reply [flat|nested] 15+ messages in thread