On Sun, 2023-07-09 at 19:50 +0800, Xiao Yang wrote:
> 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.
Hi Xiao,
The current behavior is that if a list of memdevs is supplied on the
command line, then that ordering is how the targets are assigned. The
plan is to eventually add a mode where cxl-create-region is allowed to
reorder the memdevs to fit the target rules. This should fix the issue
you are hitting, but that's a bigger piece of work. For now the
workaround is that if the automatic ordering fails (and it isn't
guaranteed to work), figure out the right ordering externally (using
cxl-list) and feed the correct order to cxl-create-region.
>
> 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 <[email protected]>
> > ---
> > 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));
