| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <xee5pz7qha3nn2ldr6ogtikbrc23d4mrxabdfv6ujtbtj7fcch@whh726b6xlhc>
Date: Sun, 29 Sep 2024 22:03:23 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Wasim Nazir <quic_wasimn@...cinc.com>
Cc: Shuah Khan <shuah@...nel.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>, linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
linux-remoteproc@...r.kernel.org
Subject: Re: [PATCH v2] selftest: remoteproc: Add basic test for start/stop
sequence
On Fri, Sep 27, 2024 at 04:51:32PM GMT, Wasim Nazir wrote:
> This test includes:
> 1) Start/stop test for each rproc instance sequencially
> 2) Start/stop test for all rproc instances concurrently
>
This fails to describe the purpose of the patch. Provide a proper commit
mesasge.
In particular, I expect an argumentation for your test scheme. Will this
work across all remoteproc instances? Does it have any dependencies,
etc...
> Changes in v2:
> - Update commit message
> - Addressed start/stop flow
The changelog goes below the '---' line, adjacent to your diffstat -
which is missing from your patch. I don't know how you're sending these
patches, but your system is either configured weirdly or you're not
following my instructions on go/upstream.
>
> Signed-off-by: Wasim Nazir <quic_wasimn@...cinc.com>
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a77770cd96b8..02ebad5ae790 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19596,6 +19596,7 @@ F: Documentation/staging/remoteproc.rst
> F: drivers/remoteproc/
> F: include/linux/remoteproc.h
> F: include/linux/remoteproc/
> +F: tools/testing/selftests/remoteproc/
>
> REMOTE PROCESSOR MESSAGING (RPMSG) SUBSYSTEM
> M: Bjorn Andersson <andersson@...nel.org>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index b38199965f99..0c8a0f427d01 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -82,6 +82,7 @@ TARGETS += proc
> TARGETS += pstore
> TARGETS += ptrace
> TARGETS += openat2
> +TARGETS += remoteproc
> TARGETS += resctrl
> TARGETS += riscv
> TARGETS += rlimits
> diff --git a/tools/testing/selftests/remoteproc/Makefile b/tools/testing/selftests/remoteproc/Makefile
> new file mode 100644
> index 000000000000..a84b3934fd36
> --- /dev/null
> +++ b/tools/testing/selftests/remoteproc/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +TEST_PROGS := remoteproc_test.sh
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/remoteproc/config b/tools/testing/selftests/remoteproc/config
> new file mode 100644
> index 000000000000..a5c237d2f3b4
> --- /dev/null
> +++ b/tools/testing/selftests/remoteproc/config
> @@ -0,0 +1 @@
> +CONFIG_REMOTEPROC=y
> diff --git a/tools/testing/selftests/remoteproc/remoteproc_test.sh b/tools/testing/selftests/remoteproc/remoteproc_test.sh
> new file mode 100644
> index 000000000000..589368285307
> --- /dev/null
> +++ b/tools/testing/selftests/remoteproc/remoteproc_test.sh
> @@ -0,0 +1,134 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> +#
> +
> +DIR="$(dirname $(readlink -f "$0"))"
> +
> +KTAP_HELPERS="${DIR}/../kselftest/ktap_helpers.sh"
> +if [ -e "$KTAP_HELPERS" ]; then
> + . "$KTAP_HELPERS"
> +else
> + echo -n "1..0 # SKIP $KTAP_HELPERS file not found"
> + exit 4
> +fi
> +
> +RPROC_SYS=/sys/class/remoteproc
> +RPROC_SEQ_SLEEP=5
> +rproc_instances=
> +# Declare an array to save initial states of each instance
> +org_instance_to_state=""
> +num_tests=0
> +test_err=0
> +
> +check_error() {
> + if [ $? -ne 0 ]; then
> + test_err=$((test_err+1))
> + ktap_print_msg "$@"
> + fi
> +}
> +
> +rproc_stop_instances() {
> + for instance in ${rproc_instances}; do
> + rproc=${RPROC_SYS}/$instance
> + rproc_name=$(cat $rproc/name)
> + rproc_state=$(cat $rproc/state)
> +
> + echo stop > "$rproc/state"
> + check_error "$rproc_name state-stop failed at state $rproc_state"
> + done
> + sleep ${RPROC_SEQ_SLEEP}
> +}
> +
> +rproc_start_instances() {
> + for instance in ${rproc_instances}; do
> + rproc=${RPROC_SYS}/$instance
> + rproc_name=$(cat $rproc/name)
> + rproc_state=$(cat $rproc/state)
> +
> + echo start > "$rproc/state"
> + check_error "$rproc_name state-start failed at state $rproc_state"
> + done
> + sleep ${RPROC_SEQ_SLEEP}
> +}
> +
> +rproc_seq_test_instance_one() {
> + instance=$1
> + rproc=${RPROC_SYS}/$instance
> + rproc_name=$(cat $rproc/name)
> + rproc_state=$(cat $rproc/state)
> + ktap_print_msg "Testing rproc sequence for $rproc_name"
> +
> + # Reset test_err value
> + test_err=0
> +
> + # Begin start/stop sequence
> + echo start > "$rproc/state"
> + check_error "$rproc_name state-start failed at state $rproc_state"
> +
> + sleep ${RPROC_SEQ_SLEEP}
> +
> + echo stop > "$rproc/state"
> + check_error "$rproc_name state-stop failed at state $rproc_state"
> +
> + if [ $test_err -ne 0 ]; then
> + ktap_test_fail "$rproc_name"
> + else
> + ktap_test_pass "$rproc_name"
> + fi
> +}
> +
> +rproc_seq_test_instances_concurrently() {
> + # Reset test_err value
> + test_err=0
> +
> + rproc_start_instances
> +
> + rproc_stop_instances
> +
> + if [ $test_err -ne 0 ]; then
> + ktap_test_fail "for any of $rproc_instances"
> + else
> + ktap_test_pass "for all $rproc_instances"
> + fi
> +}
> +
> +ktap_print_header
> +
> +if [ ! -d "${RPROC_SYS}" ]; then
> + ktap_skip_all "${RPROC_SYS} doesn't exist."
> + exit "${KSFT_SKIP}"
> +fi
> +
> +rproc_instances=$(find ${RPROC_SYS}/remoteproc* -maxdepth 1 -exec basename {} \;)
> +num_tests=$(echo ${rproc_instances} | wc -w)
> +if [ "${num_tests}" -eq 0 ]; then
> + ktap_skip_all "${RPROC_SYS}/remoteproc* doesn't exist."
> + exit "${KSFT_SKIP}"
> +fi
> +
> +# Total tests will be:
> +# 1) Seq tests for each instance sequencially
> +# 2) Seq tests for all instances concurrently
> +num_tests=$((num_tests+1))
> +
> +ktap_set_plan "${num_tests}"
> +
> +# Stop all instances
> +rproc_stop_instances
Will this not fail for remoteproc instances that aren't started
automatically?
Regards,
Bjorn
> +
> +# Test 1
> +ktap_print_msg "Testing rproc start/stop sequence for each instance sequencially"
> +for instance in ${rproc_instances}; do
> + rproc_seq_test_instance_one $instance
> +done
> +
> +# Test 2
> +ktap_print_msg "Testing rproc start/stop sequence for all instances concurrently"
> +rproc_seq_test_instances_concurrently
> +
> +# Restore all instances
> +rproc_start_instances
> +
> +ktap_finished
> --
> 2.46.1
>
Powered by blists - more mailing lists