* [NDCTL PATCH] cxl/region: Always use the correct target position
@ 2023-06-30 15:12 Xiao Yang
2023-07-09 11:50 ` Xiao Yang
0 siblings, 1 reply; 3+ messages in thread
From: Xiao Yang @ 2023-06-30 15:12 UTC (permalink / raw)
To: nvdimm; +Cc: linux-cxl, Xiao Yang
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));
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [NDCTL PATCH] cxl/region: Always use the correct target position
2023-06-30 15:12 [NDCTL PATCH] cxl/region: Always use the correct target position Xiao Yang
@ 2023-07-09 11:50 ` Xiao Yang
2023-07-24 21:16 ` Verma, Vishal L
0 siblings, 1 reply; 3+ messages in thread
From: Xiao Yang @ 2023-07-09 11:50 UTC (permalink / raw)
To: nvdimm; +Cc: linux-cxl
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));
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [NDCTL PATCH] cxl/region: Always use the correct target position
2023-07-09 11:50 ` Xiao Yang
@ 2023-07-24 21:16 ` Verma, Vishal L
0 siblings, 0 replies; 3+ messages in thread
From: Verma, Vishal L @ 2023-07-24 21:16 UTC (permalink / raw)
To: yangx.jy@fujitsu.com, nvdimm@lists.linux.dev; +Cc: linux-cxl@vger.kernel.org
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 <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));
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-24 21:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-30 15:12 [NDCTL PATCH] cxl/region: Always use the correct target position Xiao Yang
2023-07-09 11:50 ` Xiao Yang
2023-07-24 21:16 ` Verma, Vishal L
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox