| Message ID | 165781817516.1555691.3557156570639615515.stgit@dwillia2-xfh.jf.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | eef9685245d172a80e9a5dfd830942824e7d40b4 |
| Headers | show |
| Series | cxl: Region provisioning foundation | expand |
On Thu, 2022-07-14 at 10:02 -0700, Dan Williams wrote: > Exercise the fundamental region provisioning sysfs mechanisms of discovering > available DPA capacity, allocating DPA to a region, and programming HDM > decoders. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > test/cxl-region-sysfs.sh | 122 ++++++++++++++++++++++++++++++++++++++++++++++ > test/meson.build | 2 + > 2 files changed, 124 insertions(+) > create mode 100644 test/cxl-region-sysfs.sh Hi Dan, This is mostly looking good - just one note below found in testing: > > diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh > new file mode 100644 > index 000000000000..2582edb3f306 > --- /dev/null > +++ b/test/cxl-region-sysfs.sh > @@ -0,0 +1,122 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2022 Intel Corporation. All rights reserved. > + > +. $(dirname $0)/common > + > +rc=1 > + > +set -ex > + > +trap 'err $LINENO' ERR > + > +check_prereq "jq" > + > +modprobe -r cxl_test > +modprobe cxl_test > +udevadm settle > + > +# THEORY OF OPERATION: Create a x8 interleave across the pmem capacity > +# of the 8 endpoints defined by cxl_test, commit the decoders (which > +# just stubs out the actual hardware programming aspect, but updates the > +# driver state), and then tear it all down again. As with other cxl_test > +# tests if the CXL topology in tools/testing/cxl/test/cxl.c ever changes > +# then the paired update must be made to this test. > + > +# find the root decoder that spans both test host-bridges and support pmem > +decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] | > + select(.pmem_capable == true) | > + select(.nr_targets == 2) | > + .decoder") > + > +# find the memdevs mapped by that decoder > +readarray -t mem < <($CXL list -M -d $decoder | jq -r ".[].memdev") > + > +# ask cxl reserve-dpa to allocate pmem capacity from each of those memdevs > +readarray -t endpoint < <($CXL reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) | > + jq -r ".[] | .decoder.decoder") > + > +# instantiate an empty region > +region=$(cat /sys/bus/cxl/devices/$decoder/create_pmem_region) > +echo $region > /sys/bus/cxl/devices/$decoder/create_pmem_region > +uuidgen > /sys/bus/cxl/devices/$region/uuid > + > +# setup interleave geometry > +nr_targets=${#endpoint[@]} > +echo $nr_targets > /sys/bus/cxl/devices/$region/interleave_ways > +g=$(cat /sys/bus/cxl/devices/$decoder/interleave_granularity) > +echo $g > /sys/bus/cxl/devices/$region/interleave_granularity > +echo $((nr_targets * (256<<20))) > /sys/bus/cxl/devices/$region/size > + > +# grab the list of memdevs grouped by host-bridge interleave position > +port_dev0=$($CXL list -T -d $decoder | jq -r ".[] | > + .targets | .[] | select(.position == 0) | .target") > +port_dev1=$($CXL list -T -d $decoder | jq -r ".[] | > + .targets | .[] | select(.position == 1) | .target") With my pending update to make memdevs and regions the default listing if no other top level object specified, the above listing breaks as it can't deal with the extra memdevs now listed. I think it may make sense to fine tune the defaults a bit - i.e. if a -d is supplied, assume -D, but no other default top-level objects. However I think this would be more resilient regardless, if it explicitly specified a -D: --- diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh index 2582edb..eb28184 100644 --- a/test/cxl-region-sysfs.sh +++ b/test/cxl-region-sysfs.sh @@ -49,9 +49,9 @@ echo $g > /sys/bus/cxl/devices/$region/interleave_granularity echo $((nr_targets * (256<<20))) > /sys/bus/cxl/devices/$region/size # grab the list of memdevs grouped by host-bridge interleave position -port_dev0=$($CXL list -T -d $decoder | jq -r ".[] | +port_dev0=$($CXL list -DT -d $decoder | jq -r ".[] | .targets | .[] | select(.position == 0) | .target") -port_dev1=$($CXL list -T -d $decoder | jq -r ".[] | +port_dev1=$($CXL list -DT -d $decoder | jq -r ".[] | .targets | .[] | select(.position == 1) | .target") readarray -t mem_sort0 < <($CXL list -M -p $port_dev0 | jq -r ".[] | .memdev") readarray -t mem_sort1 < <($CXL list -M -p $port_dev1 | jq -r ".[] | .memdev")
Verma, Vishal L wrote: > On Thu, 2022-07-14 at 10:02 -0700, Dan Williams wrote: > > Exercise the fundamental region provisioning sysfs mechanisms of discovering > > available DPA capacity, allocating DPA to a region, and programming HDM > > decoders. > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > --- > > test/cxl-region-sysfs.sh | 122 ++++++++++++++++++++++++++++++++++++++++++++++ > > test/meson.build | 2 + > > 2 files changed, 124 insertions(+) > > create mode 100644 test/cxl-region-sysfs.sh > > Hi Dan, > > This is mostly looking good - just one note below found in testing: > > > > > diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh > > new file mode 100644 > > index 000000000000..2582edb3f306 > > --- /dev/null > > +++ b/test/cxl-region-sysfs.sh > > @@ -0,0 +1,122 @@ > > +#!/bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (C) 2022 Intel Corporation. All rights reserved. > > + > > +. $(dirname $0)/common > > + > > +rc=1 > > + > > +set -ex > > + > > +trap 'err $LINENO' ERR > > + > > +check_prereq "jq" > > + > > +modprobe -r cxl_test > > +modprobe cxl_test > > +udevadm settle > > + > > +# THEORY OF OPERATION: Create a x8 interleave across the pmem capacity > > +# of the 8 endpoints defined by cxl_test, commit the decoders (which > > +# just stubs out the actual hardware programming aspect, but updates the > > +# driver state), and then tear it all down again. As with other cxl_test > > +# tests if the CXL topology in tools/testing/cxl/test/cxl.c ever changes > > +# then the paired update must be made to this test. > > + > > +# find the root decoder that spans both test host-bridges and support pmem > > +decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] | > > + select(.pmem_capable == true) | > > + select(.nr_targets == 2) | > > + .decoder") > > + > > +# find the memdevs mapped by that decoder > > +readarray -t mem < <($CXL list -M -d $decoder | jq -r ".[].memdev") > > + > > +# ask cxl reserve-dpa to allocate pmem capacity from each of those memdevs > > +readarray -t endpoint < <($CXL reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) | > > + jq -r ".[] | .decoder.decoder") > > + > > +# instantiate an empty region > > +region=$(cat /sys/bus/cxl/devices/$decoder/create_pmem_region) > > +echo $region > /sys/bus/cxl/devices/$decoder/create_pmem_region > > +uuidgen > /sys/bus/cxl/devices/$region/uuid > > + > > +# setup interleave geometry > > +nr_targets=${#endpoint[@]} > > +echo $nr_targets > /sys/bus/cxl/devices/$region/interleave_ways > > +g=$(cat /sys/bus/cxl/devices/$decoder/interleave_granularity) > > +echo $g > /sys/bus/cxl/devices/$region/interleave_granularity > > +echo $((nr_targets * (256<<20))) > /sys/bus/cxl/devices/$region/size > > + > > +# grab the list of memdevs grouped by host-bridge interleave position > > +port_dev0=$($CXL list -T -d $decoder | jq -r ".[] | > > + .targets | .[] | select(.position == 0) | .target") > > +port_dev1=$($CXL list -T -d $decoder | jq -r ".[] | > > + .targets | .[] | select(.position == 1) | .target") > > With my pending update to make memdevs and regions the default listing > if no other top level object specified, the above listing breaks as it > can't deal with the extra memdevs now listed. > > I think it may make sense to fine tune the defaults a bit - i.e. if > a -d is supplied, assume -D, but no other default top-level objects. Yes, this is what I would expect. > However I think this would be more resilient regardless, if it > explicitly specified a -D: True, but it's a bit redundant. Why does the -RM default break: cxl list [-T] -d $decoder ...? Doesn't the final "num_list_flags() == 0" check come after: if (param.decoder_filter) param.decoders = true; ...has prevented the default?
On Thu, 2022-07-14 at 14:13 -0700, Dan Williams wrote: > Verma, Vishal L wrote: > > > > > > +# grab the list of memdevs grouped by host-bridge interleave position > > > +port_dev0=$($CXL list -T -d $decoder | jq -r ".[] | > > > + .targets | .[] | select(.position == 0) | .target") > > > +port_dev1=$($CXL list -T -d $decoder | jq -r ".[] | > > > + .targets | .[] | select(.position == 1) | .target") > > > > With my pending update to make memdevs and regions the default listing > > if no other top level object specified, the above listing breaks as it > > can't deal with the extra memdevs now listed. > > > > I think it may make sense to fine tune the defaults a bit - i.e. if > > a -d is supplied, assume -D, but no other default top-level objects. > > Yes, this is what I would expect. > > > However I think this would be more resilient regardless, if it > > explicitly specified a -D: > > True, but it's a bit redundant. > > Why does the -RM default break: > > cxl list [-T] -d $decoder > > ...? Doesn't the final "num_list_flags() == 0" check come after: > > if (param.decoder_filter) > param.decoders = true; > > ...has prevented the default? Ah yes I see the problem - I added the new default unconditionally in the first pass of num_list_flags() == 0, which was also checking the above condition. I basically need to let it run through the list of 'if -d then -D' type stuff, then check num_list_flags() again, and if 0, only then enable -R and -M. Will fix this in my patch, no need for the -D change above.
diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh new file mode 100644 index 000000000000..2582edb3f306 --- /dev/null +++ b/test/cxl-region-sysfs.sh @@ -0,0 +1,122 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2022 Intel Corporation. All rights reserved. + +. $(dirname $0)/common + +rc=1 + +set -ex + +trap 'err $LINENO' ERR + +check_prereq "jq" + +modprobe -r cxl_test +modprobe cxl_test +udevadm settle + +# THEORY OF OPERATION: Create a x8 interleave across the pmem capacity +# of the 8 endpoints defined by cxl_test, commit the decoders (which +# just stubs out the actual hardware programming aspect, but updates the +# driver state), and then tear it all down again. As with other cxl_test +# tests if the CXL topology in tools/testing/cxl/test/cxl.c ever changes +# then the paired update must be made to this test. + +# find the root decoder that spans both test host-bridges and support pmem +decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] | + select(.pmem_capable == true) | + select(.nr_targets == 2) | + .decoder") + +# find the memdevs mapped by that decoder +readarray -t mem < <($CXL list -M -d $decoder | jq -r ".[].memdev") + +# ask cxl reserve-dpa to allocate pmem capacity from each of those memdevs +readarray -t endpoint < <($CXL reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) | + jq -r ".[] | .decoder.decoder") + +# instantiate an empty region +region=$(cat /sys/bus/cxl/devices/$decoder/create_pmem_region) +echo $region > /sys/bus/cxl/devices/$decoder/create_pmem_region +uuidgen > /sys/bus/cxl/devices/$region/uuid + +# setup interleave geometry +nr_targets=${#endpoint[@]} +echo $nr_targets > /sys/bus/cxl/devices/$region/interleave_ways +g=$(cat /sys/bus/cxl/devices/$decoder/interleave_granularity) +echo $g > /sys/bus/cxl/devices/$region/interleave_granularity +echo $((nr_targets * (256<<20))) > /sys/bus/cxl/devices/$region/size + +# grab the list of memdevs grouped by host-bridge interleave position +port_dev0=$($CXL list -T -d $decoder | jq -r ".[] | + .targets | .[] | select(.position == 0) | .target") +port_dev1=$($CXL list -T -d $decoder | jq -r ".[] | + .targets | .[] | select(.position == 1) | .target") +readarray -t mem_sort0 < <($CXL list -M -p $port_dev0 | jq -r ".[] | .memdev") +readarray -t mem_sort1 < <($CXL list -M -p $port_dev1 | jq -r ".[] | .memdev") + +# TODO: add a cxl list option to list memdevs in valid region provisioning +# order, hardcode for now. +mem_sort=() +mem_sort[0]=${mem_sort0[0]} +mem_sort[1]=${mem_sort1[0]} +mem_sort[2]=${mem_sort0[2]} +mem_sort[3]=${mem_sort1[2]} +mem_sort[4]=${mem_sort0[1]} +mem_sort[5]=${mem_sort1[1]} +mem_sort[6]=${mem_sort0[3]} +mem_sort[7]=${mem_sort1[3]} + +# TODO: use this alternative memdev ordering to validate a negative test for +# specifying invalid positions of memdevs +#mem_sort[2]=${mem_sort0[0]} +#mem_sort[1]=${mem_sort1[0]} +#mem_sort[0]=${mem_sort0[2]} +#mem_sort[3]=${mem_sort1[2]} +#mem_sort[4]=${mem_sort0[1]} +#mem_sort[5]=${mem_sort1[1]} +#mem_sort[6]=${mem_sort0[3]} +#mem_sort[7]=${mem_sort1[3]} + +# re-generate the list of endpoint decoders in region position programming order +endpoint=() +for i in ${mem_sort[@]} +do + readarray -O ${#endpoint[@]} -t endpoint < <($CXL list -Di -d endpoint -m $i | jq -r ".[] | + select(.mode == \"pmem\") | .decoder") +done + +# attach all endpoint decoders to the region +pos=0 +for i in ${endpoint[@]} +do + echo $i > /sys/bus/cxl/devices/$region/target$pos + pos=$((pos+1)) +done +echo "$region added ${#endpoint[@]} targets: ${endpoint[@]}" + +# walk up the topology and commit all decoders +echo 1 > /sys/bus/cxl/devices/$region/commit + +# walk down the topology and de-commit all decoders +echo 0 > /sys/bus/cxl/devices/$region/commit + +# remove endpoints from the region +pos=0 +for i in ${endpoint[@]} +do + echo "" > /sys/bus/cxl/devices/$region/target$pos + pos=$((pos+1)) +done + +# release DPA capacity +readarray -t endpoint < <($CXL free-dpa -t pmem ${mem[*]} | + jq -r ".[] | .decoder.decoder") +echo "$region released ${#endpoint[@]} targets: ${endpoint[@]}" + +# validate no WARN or lockdep report during the run +log=$(journalctl -r -k --since "-$((SECONDS+1))s") +grep -q "Call Trace" <<< $log && err "$LINENO" + +modprobe -r cxl_test diff --git a/test/meson.build b/test/meson.build index 210dcb0b5ff1..3203d9cea243 100644 --- a/test/meson.build +++ b/test/meson.build @@ -151,6 +151,7 @@ max_extent = find_program('max_available_extent_ns.sh') pfn_meta_errors = find_program('pfn-meta-errors.sh') track_uuid = find_program('track-uuid.sh') cxl_topo = find_program('cxl-topology.sh') +cxl_sysfs = find_program('cxl-region-sysfs.sh') tests = [ [ 'libndctl', libndctl, 'ndctl' ], @@ -176,6 +177,7 @@ tests = [ [ 'pfn-meta-errors.sh', pfn_meta_errors, 'ndctl' ], [ 'track-uuid.sh', track_uuid, 'ndctl' ], [ 'cxl-topology.sh', cxl_topo, 'cxl' ], + [ 'cxl-region-sysfs.sh', cxl_sysfs, 'cxl' ], ] if get_option('destructive').enabled()
Exercise the fundamental region provisioning sysfs mechanisms of discovering available DPA capacity, allocating DPA to a region, and programming HDM decoders. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- test/cxl-region-sysfs.sh | 122 ++++++++++++++++++++++++++++++++++++++++++++++ test/meson.build | 2 + 2 files changed, 124 insertions(+) create mode 100644 test/cxl-region-sysfs.sh