Re: [NDCTL PATCH] cxl/region: Always use the correct target position

[Xiao Yang <yangx.jy@fujitsu.com>]

Hi all,

Kindly ping.

This patch can only fixes the case that 2 way interleave is enabled 
across 2 CXL host bridges and each host bridge has 1 CXL Root Port.
PS: In other word, this patch is wrong when 2 way interleave is enabled 
across 2 CXL host bridges and each host bridge has 2 CXL Root Ports.

I am trying to find a better solution. If you have any suggestion, 
please let me know.

Best Regards,
Xiao Yang

On 2023/6/30 23:12, Xiao Yang wrote:
> create_region() uses the wrong target position in some cases.
> For example, cxl create-region command fails to create a new
> region in 2 way interleave set when mem0 connects target1(position:1)
> and mem1 connects target0(position:0):
> 
> $ cxl list -M -P -D -T -u
> [
>    {
>      "ports":[
>        {
>          "port":"port1",
>          "host":"pci0000:16",
>          "depth":1,
>          "nr_dports":1,
>          "dports":[
>            {
>              "dport":"0000:16:00.0",
>              "id":"0"
>            }
>          ],
>          "memdevs:port1":[
>            {
>              "memdev":"mem0",
>              "ram_size":"512.00 MiB (536.87 MB)",
>              "serial":"0",
>              "host":"0000:17:00.0"
>            }
>          ]
>        },
>        {
>          "port":"port2",
>          "host":"pci0000:0c",
>          "depth":1,
>          "nr_dports":1,
>          "dports":[
>            {
>              "dport":"0000:0c:00.0",
>              "id":"0"
>            }
>          ],
>          "memdevs:port2":[
>            {
>              "memdev":"mem1",
>              "ram_size":"512.00 MiB (536.87 MB)",
>              "serial":"0",
>              "host":"0000:0d:00.0"
>            }
>          ]
>        }
>      ]
>    },
>    {
>      "root decoders":[
>        {
>          "decoder":"decoder0.0",
>          "resource":"0x750000000",
>          "size":"4.00 GiB (4.29 GB)",
>          "interleave_ways":2,
>          "interleave_granularity":8192,
>          "max_available_extent":"4.00 GiB (4.29 GB)",
>          "pmem_capable":true,
>          "volatile_capable":true,
>          "accelmem_capable":true,
>          "nr_targets":2,
>          "targets":[
>            {
>              "target":"pci0000:16",
>              "alias":"ACPI0016:00",
>              "position":1,
>              "id":"0x16"
>            },
>            {
>              "target":"pci0000:0c",
>              "alias":"ACPI0016:01",
>              "position":0,
>              "id":"0xc"
>            }
>          ]
>        }
>      ]
>    }
> ]
> 
> $ cxl create-region -t ram -d decoder0.0 -m mem0 mem1
> cxl region: create_region: region0: failed to set target0 to mem0
> cxl region: cmd_create_region: created 0 regions
> 
> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
> ---
>   cxl/region.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 07ce4a3..946b5ff 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -667,6 +667,8 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>   		struct json_object *jobj =
>   			json_object_array_get_idx(p->memdevs, i);
>   		struct cxl_memdev *memdev = json_object_get_userdata(jobj);
> +		struct cxl_target *target = cxl_decoder_get_target_by_memdev(p->root_decoder,
> +										memdev);
>   
>   		ep_decoder = cxl_memdev_find_decoder(memdev);
>   		if (!ep_decoder) {
> @@ -683,7 +685,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>   			try(cxl_decoder, set_mode, ep_decoder, p->mode);
>   		}
>   		try(cxl_decoder, set_dpa_size, ep_decoder, size/p->ways);
> -		rc = cxl_region_set_target(region, i, ep_decoder);
> +		rc = cxl_region_set_target(region, cxl_target_get_position(target), ep_decoder);
>   		if (rc) {
>   			log_err(&rl, "%s: failed to set target%d to %s\n",
>   				devname, i, cxl_memdev_get_devname(memdev));