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

[Xiao Yang]

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...@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));