[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.22.394.2204201436530.66128@rhweight-WRK1>
Date: Wed, 20 Apr 2022 14:37:59 -0700 (PDT)
From: matthew.gerlach@...ux.intel.com
To: Russ Weight <russell.h.weight@...el.com>
cc: mcgrof@...nel.org, rafael@...nel.org, linux-kernel@...r.kernel.org,
trix@...hat.com, lgoncalv@...hat.com, yilun.xu@...el.com,
hao.wu@...el.com, basheer.ahmed.muddebihal@...el.com,
tianfei.zhang@...el.com
Subject: Re: [PATCH v4 8/8] selftests: firmware: Add firmware upload
selftests
On Tue, 19 Apr 2022, Russ Weight wrote:
> Add selftests to verify the firmware upload mechanism. These test
> include simple firmware uploads as well as upload cancellation and
> error injection. The test creates three firmware devices and verifies
> that they all work correctly and independently.
Hi Russ,
I applied your patches and ran the selftests. Everything looks good to
me.
Tested-by: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
>
> Reviewed-by: Luis Chamberlain <mcgrof@...nel.org>
> Reviewed-by: Tianfei zhang <tianfei.zhang@...el.com>
> Signed-off-by: Russ Weight <russell.h.weight@...el.com>
> ---
> v2:
> - No changes from v1
> v3:
> - Added Reviewed-by tag
> v4:
> - Added Reviewed-by tag
> ---
> tools/testing/selftests/firmware/Makefile | 2 +-
> tools/testing/selftests/firmware/config | 1 +
> tools/testing/selftests/firmware/fw_lib.sh | 7 +
> .../selftests/firmware/fw_run_tests.sh | 4 +
> tools/testing/selftests/firmware/fw_upload.sh | 214 ++++++++++++++++++
> 5 files changed, 227 insertions(+), 1 deletion(-)
> create mode 100755 tools/testing/selftests/firmware/fw_upload.sh
>
> diff --git a/tools/testing/selftests/firmware/Makefile b/tools/testing/selftests/firmware/Makefile
> index 40211cd8f0e6..7992969deaa2 100644
> --- a/tools/testing/selftests/firmware/Makefile
> +++ b/tools/testing/selftests/firmware/Makefile
> @@ -4,7 +4,7 @@ CFLAGS = -Wall \
> -O2
>
> TEST_PROGS := fw_run_tests.sh
> -TEST_FILES := fw_fallback.sh fw_filesystem.sh fw_lib.sh
> +TEST_FILES := fw_fallback.sh fw_filesystem.sh fw_upload.sh fw_lib.sh
> TEST_GEN_FILES := fw_namespace
>
> include ../lib.mk
> diff --git a/tools/testing/selftests/firmware/config b/tools/testing/selftests/firmware/config
> index bf634dda0720..6e402519b117 100644
> --- a/tools/testing/selftests/firmware/config
> +++ b/tools/testing/selftests/firmware/config
> @@ -3,3 +3,4 @@ CONFIG_FW_LOADER=y
> CONFIG_FW_LOADER_USER_HELPER=y
> CONFIG_IKCONFIG=y
> CONFIG_IKCONFIG_PROC=y
> +CONFIG_FW_UPLOAD=y
> diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
> index 5b8c0fedee76..fe8d34dbe7ca 100755
> --- a/tools/testing/selftests/firmware/fw_lib.sh
> +++ b/tools/testing/selftests/firmware/fw_lib.sh
> @@ -63,6 +63,7 @@ check_setup()
> HAS_FW_LOADER_USER_HELPER="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)"
> HAS_FW_LOADER_USER_HELPER_FALLBACK="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)"
> HAS_FW_LOADER_COMPRESS="$(kconfig_has CONFIG_FW_LOADER_COMPRESS=y)"
> + HAS_FW_UPLOAD="$(kconfig_has CONFIG_FW_UPLOAD=y)"
> PROC_FW_IGNORE_SYSFS_FALLBACK="0"
> PROC_FW_FORCE_SYSFS_FALLBACK="0"
>
> @@ -113,6 +114,12 @@ verify_reqs()
> exit 0
> fi
> fi
> + if [ "$TEST_REQS_FW_UPLOAD" = "yes" ]; then
> + if [ ! "$HAS_FW_UPLOAD" = "yes" ]; then
> + echo "firmware upload disabled so ignoring test"
> + exit 0
> + fi
> + fi
> }
>
> setup_tmp_file()
> diff --git a/tools/testing/selftests/firmware/fw_run_tests.sh b/tools/testing/selftests/firmware/fw_run_tests.sh
> index 777377078d5e..f6d95a2d5124 100755
> --- a/tools/testing/selftests/firmware/fw_run_tests.sh
> +++ b/tools/testing/selftests/firmware/fw_run_tests.sh
> @@ -22,6 +22,10 @@ run_tests()
> proc_set_force_sysfs_fallback $1
> proc_set_ignore_sysfs_fallback $2
> $TEST_DIR/fw_fallback.sh
> +
> + proc_set_force_sysfs_fallback $1
> + proc_set_ignore_sysfs_fallback $2
> + $TEST_DIR/fw_upload.sh
> }
>
> run_test_config_0001()
> diff --git a/tools/testing/selftests/firmware/fw_upload.sh b/tools/testing/selftests/firmware/fw_upload.sh
> new file mode 100755
> index 000000000000..c7a6f06c9adb
> --- /dev/null
> +++ b/tools/testing/selftests/firmware/fw_upload.sh
> @@ -0,0 +1,214 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# This validates the user-initiated fw upload mechanism of the firmware
> +# loader. It verifies that one or more firmware devices can be created
> +# for a device driver. It also verifies the data transfer, the
> +# cancellation support, and the error flows.
> +set -e
> +
> +TEST_REQS_FW_UPLOAD="yes"
> +TEST_DIR=$(dirname $0)
> +
> +progress_states="preparing transferring programming"
> +errors="hw-error
> + timeout
> + device-busy
> + invalid-file-size
> + read-write-error
> + flash-wearout"
> +error_abort="user-abort"
> +fwname1=fw1
> +fwname2=fw2
> +fwname3=fw3
> +
> +source $TEST_DIR/fw_lib.sh
> +
> +check_mods
> +check_setup
> +verify_reqs
> +
> +trap "upload_finish" EXIT
> +
> +upload_finish() {
> + local fwdevs="$fwname1 $fwname2 $fwname3"
> +
> + for name in $fwdevs; do
> + if [ -e "$DIR/$name" ]; then
> + echo -n "$name" > "$DIR"/upload_unregister
> + fi
> + done
> +}
> +
> +upload_fw() {
> + local name="$1"
> + local file="$2"
> +
> + echo 1 > "$DIR"/"$name"/loading
> + cat "$file" > "$DIR"/"$name"/data
> + echo 0 > "$DIR"/"$name"/loading
> +}
> +
> +verify_fw() {
> + local name="$1"
> + local file="$2"
> +
> + echo -n "$name" > "$DIR"/config_upload_name
> + if ! cmp "$file" "$DIR"/upload_read > /dev/null 2>&1; then
> + echo "$0: firmware compare for $name did not match" >&2
> + exit 1
> + fi
> +
> + echo "$0: firmware upload for $name works" >&2
> + return 0
> +}
> +
> +inject_error() {
> + local name="$1"
> + local status="$2"
> + local error="$3"
> +
> + echo 1 > "$DIR"/"$name"/loading
> + echo -n "inject":"$status":"$error" > "$DIR"/"$name"/data
> + echo 0 > "$DIR"/"$name"/loading
> +}
> +
> +await_status() {
> + local name="$1"
> + local expected="$2"
> + local status
> + local i
> +
> + let i=0
> + while [ $i -lt 50 ]; do
> + status=$(cat "$DIR"/"$name"/status)
> + if [ "$status" = "$expected" ]; then
> + return 0;
> + fi
> + sleep 1e-03
> + let i=$i+1
> + done
> +
> + echo "$0: Invalid status: Expected $expected, Actual $status" >&2
> + return 1;
> +}
> +
> +await_idle() {
> + local name="$1"
> +
> + await_status "$name" "idle"
> + return $?
> +}
> +
> +expect_error() {
> + local name="$1"
> + local expected="$2"
> + local error=$(cat "$DIR"/"$name"/error)
> +
> + if [ "$error" != "$expected" ]; then
> + echo "Invalid error: Expected $expected, Actual $error" >&2
> + return 1
> + fi
> +
> + return 0
> +}
> +
> +random_firmware() {
> + local bs="$1"
> + local count="$2"
> + local file=$(mktemp -p /tmp uploadfwXXX.bin)
> +
> + dd if=/dev/urandom of="$file" bs="$bs" count="$count" > /dev/null 2>&1
> + echo "$file"
> +}
> +
> +test_upload_cancel() {
> + local name="$1"
> + local status
> +
> + for status in $progress_states; do
> + inject_error $name $status $error_abort
> + if ! await_status $name $status; then
> + exit 1
> + fi
> +
> + echo 1 > "$DIR"/"$name"/cancel
> +
> + if ! await_idle $name; then
> + exit 1
> + fi
> +
> + if ! expect_error $name "$status":"$error_abort"; then
> + exit 1
> + fi
> + done
> +
> + echo "$0: firmware upload cancellation works"
> + return 0
> +}
> +
> +test_error_handling() {
> + local name=$1
> + local status
> + local error
> +
> + for status in $progress_states; do
> + for error in $errors; do
> + inject_error $name $status $error
> +
> + if ! await_idle $name; then
> + exit 1
> + fi
> +
> + if ! expect_error $name "$status":"$error"; then
> + exit 1
> + fi
> +
> + done
> + done
> + echo "$0: firmware upload error handling works"
> +}
> +
> +test_fw_too_big() {
> + local name=$1
> + local fw_too_big=`random_firmware 512 5`
> + local expected="preparing:invalid-file-size"
> +
> + upload_fw $name $fw_too_big
> + rm -f $fw_too_big
> +
> + if ! await_idle $name; then
> + exit 1
> + fi
> +
> + if ! expect_error $name $expected; then
> + exit 1
> + fi
> +
> + echo "$0: oversized firmware error handling works"
> +}
> +
> +echo -n "$fwname1" > "$DIR"/upload_register
> +echo -n "$fwname2" > "$DIR"/upload_register
> +echo -n "$fwname3" > "$DIR"/upload_register
> +
> +test_upload_cancel $fwname1
> +test_error_handling $fwname1
> +test_fw_too_big $fwname1
> +
> +fw_file1=`random_firmware 512 4`
> +fw_file2=`random_firmware 512 3`
> +fw_file3=`random_firmware 512 2`
> +
> +upload_fw $fwname1 $fw_file1
> +upload_fw $fwname2 $fw_file2
> +upload_fw $fwname3 $fw_file3
> +
> +verify_fw ${fwname1} ${fw_file1}
> +verify_fw ${fwname2} ${fw_file2}
> +verify_fw ${fwname3} ${fw_file3}
> +
> +echo -n "$fwname1" > "$DIR"/upload_unregister
> +echo -n "$fwname2" > "$DIR"/upload_unregister
> +echo -n "$fwname3" > "$DIR"/upload_unregister
> +
> +exit 0
> --
> 2.25.1
>
>
Powered by blists - more mailing lists