Dave Jiang wrote:
> Add a unit test to verify the features ioctl commands. Test support added
> for locating a features device, retrieve and verify the supported features
> commands, retrieve specific feature command data, retrieve test feature
> data, and write and verify test feature data.
>
> Signed-off-by: Dave Jiang <dave.ji...@intel.com>
> ---
> v5:
> - Make command prep common. (Alison)
> - Rename fwctl.c to cxl-features-control.c. (Alison)
> - Update test code to retrieve cxl_fwctl object.
> - Create helper for aligned allocation and zeroing. (Alison)
> - Correct check of open() return value. (Alison)
> ---
> test/cxl-features-control.c | 439 ++++++++++++++++++++++++++++++++++++
> test/cxl-features.sh | 31 +++
> test/cxl-topology.sh | 4 +
> test/meson.build | 45 ++++
> 4 files changed, 519 insertions(+)
> create mode 100644 test/cxl-features-control.c
> create mode 100755 test/cxl-features.sh
>
> diff --git a/test/cxl-features-control.c b/test/cxl-features-control.c
Not that this filename matters, but if someone was looking for FWCTL
examples with CXL filename does not shout "look here for
fwctl examples".
[..]
> diff --git a/test/cxl-features.sh b/test/cxl-features.sh
> new file mode 100755
> index 000000000000..e648242a4a01
> --- /dev/null
> +++ b/test/cxl-features.sh
> @@ -0,0 +1,31 @@
> +#!/bin/bash -Ex
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2025 Intel Corporation. All rights reserved.
> +
> +rc=77
> +# 237 is -ENODEV
> +ERR_NODEV=237
> +
> +. $(dirname $0)/common
> +FEATURES="$TEST_PATH"/cxl-features-control
> +
> +trap 'err $LINENO' ERR
> +
> +modprobe cxl_test
> +
> +test -x "$FEATURES" || do_skip "no CXL Features Contrl"
> +# disable trap
> +trap - $(compgen -A signal)
> +"$FEATURES"
> +rc=$?
> +
> +echo "error: $rc"
> +if [ "$rc" -eq "$ERR_NODEV" ]; then
> + do_skip "no CXL FWCTL char dev"
> +elif [ "$rc" -ne 0 ]; then
> + echo "fail: $LINENO" && exit 1
> +fi
> +
> +trap 'err $LINENO' ERR
> +
> +_cxl_cleanup
> diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
> index e8b9f56543b5..90b9c98273db 100644
> --- a/test/cxl-topology.sh
> +++ b/test/cxl-topology.sh
> @@ -172,11 +172,15 @@ done
> # validate host bridge tear down for the first 2 bridges
> for b in ${bridge[0]} ${bridge[1]}
> do
> + echo "XXX SHELL disable port $b to validate teardown" > /dev/kmsg
> +
> $CXL disable-port $b -f
> json=$($CXL list -M -i -p $b)
> count=$(jq "map(select(.state == \"disabled\")) | length" <<< $json)
> ((count == 4)) || err "$LINENO"
>
> + echo "XXX SHELL enable port $b to validate teardown" > /dev/kmsg
> +
Are these unrelated changes? I do not see what new cxl-topology.sh print
statements has to do with this new fwctl test?
> $CXL enable-port $b -m
> json=$($CXL list -M -p $b)
> count=$(jq "length" <<< $json)
> diff --git a/test/meson.build b/test/meson.build
> index d871e28e17ce..89e73c2575f6 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -17,6 +17,13 @@ ndctl_deps = libndctl_deps + [
> versiondep,
> ]
>
> +libcxl_deps = [
> + cxl_dep,
> + ndctl_dep,
> + uuid,
> + kmod,
> +]
> +
> libndctl = executable('libndctl', testcore + [ 'libndctl.c'],
> dependencies : libndctl_deps,
> include_directories : root_inc,
> @@ -130,6 +137,33 @@ revoke_devmem = executable('revoke_devmem', testcore + [
> include_directories : root_inc,
> )
>
> +fs = import('fs')
> +
> +feature_hdrs = [
> + '/usr/include/cxl/features.h',
> + '/usr/include/fwctl/cxl.h',
> + '/usr/include/fwctl/fwctl.h',
> +]
> +
> +feat_hdrs_exist = true
> +foreach file : feature_hdrs
> + if not fs.exists(file)
> + feat_hdrs_exist = false
> + break
> + endif
> +endforeach
> +
> +if feat_hdrs_exist
> + features = executable('cxl-features-control', [
> + 'cxl-features-control.c',
> + ],
> + include_directories : root_inc,
> + dependencies : libcxl_deps,
> + )
> +else
> + features = []
> +endif
This does not look like typical meson conditional functionality. I would
expect is to match what happens for conditional "keyutils"
functionality. A new "if get_option('fwctl').enabled()" option that
gates other dependencies.
You can add Acked-by for the series after addressing above comments.
