mbox series

[v1,0/2] kselftest/arm64: Small fixes for sve-ptrace test

Message ID 20220124175527.3260234-1-broonie@kernel.org
Headers show
Series kselftest/arm64: Small fixes for sve-ptrace test | expand

Message

Mark Brown Jan. 24, 2022, 5:55 p.m. UTC
A couple of fairly minor fixes for the sve-ptrace test, one output thing
and one for an issue which would generate spurious false positives.

Mark Brown (2):
  kselftest/arm64: Skip VL_INHERIT tests for unsupported vector types
  kselftest/arm64: Correct logging of FPSIMD register read via ptrace

 tools/testing/selftests/arm64/fp/sve-ptrace.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)


base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07

Comments

Shuah Khan Jan. 24, 2022, 9:27 p.m. UTC | #1
On 1/24/22 10:55 AM, Mark Brown wrote:
> Currently we unconditionally test the ability to set the vector length
> inheritance flag via ptrace meaning that we generate false failures on
> systems that don't support SVE when we attempt to set the vector length
> there. Check the hwcap and mark the tests as skipped when it's not present.
> 
> Fixes: 0ba1ce1e86052d ("selftests: arm64: Add coverage of ptrace flags for SVE VL inheritance")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   tools/testing/selftests/arm64/fp/sve-ptrace.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/arm64/fp/sve-ptrace.c b/tools/testing/selftests/arm64/fp/sve-ptrace.c
> index af798b9d232c..0cf78360c5bc 100644
> --- a/tools/testing/selftests/arm64/fp/sve-ptrace.c
> +++ b/tools/testing/selftests/arm64/fp/sve-ptrace.c
> @@ -557,7 +557,14 @@ static int do_parent(pid_t child)
>   		}
>   
>   		/* prctl() flags */
> -		ptrace_set_get_inherit(child, &vec_types[i]);
> +		if (getauxval(vec_types[i].hwcap_type) & vec_types[i].hwcap) {
> +			ptrace_set_get_inherit(child, &vec_types[i]);
> +		} else {
> +			ksft_test_result_skip("%s SVE_PT_VL_INHERIT set\n",
> +					      vec_types[i].name);
> +			ksft_test_result_skip("%s SVE_PT_VL_INHERIT cleared\n",
> +					      vec_types[i].name);

These messages are a bit confusing. Are we skipping two tests?
These messages can be combined into one like this one on lin 572:

ksft_test_result_skip("%s get/set VL %d\n", vec_types[i].name, vl);

> +		}
>   
>   		/* Step through every possible VQ */
>   		for (vq = SVE_VQ_MIN; vq <= SVE_VQ_MAX; vq++) {
> 

thanks,
-- Shuah
Mark Brown Jan. 24, 2022, 9:33 p.m. UTC | #2
On Mon, Jan 24, 2022 at 02:27:07PM -0700, Shuah Khan wrote:
> On 1/24/22 10:55 AM, Mark Brown wrote:

> > -		ptrace_set_get_inherit(child, &vec_types[i]);
> > +		if (getauxval(vec_types[i].hwcap_type) & vec_types[i].hwcap) {
> > +			ptrace_set_get_inherit(child, &vec_types[i]);
> > +		} else {
> > +			ksft_test_result_skip("%s SVE_PT_VL_INHERIT set\n",
> > +					      vec_types[i].name);
> > +			ksft_test_result_skip("%s SVE_PT_VL_INHERIT cleared\n",
> > +					      vec_types[i].name);

> These messages are a bit confusing. Are we skipping two tests?

ptrace_set_get_inherit() logs two test results (one for set, one for get).

> These messages can be combined into one like this one on lin 572:

> ksft_test_result_skip("%s get/set VL %d\n", vec_types[i].name, vl);

If we do that then the number of planned tests won't line up with the
number of expected tests.
Shuah Khan Jan. 24, 2022, 9:39 p.m. UTC | #3
On 1/24/22 2:33 PM, Mark Brown wrote:
> On Mon, Jan 24, 2022 at 02:27:07PM -0700, Shuah Khan wrote:
>> On 1/24/22 10:55 AM, Mark Brown wrote:
> 
>>> -		ptrace_set_get_inherit(child, &vec_types[i]);
>>> +		if (getauxval(vec_types[i].hwcap_type) & vec_types[i].hwcap) {
>>> +			ptrace_set_get_inherit(child, &vec_types[i]);
>>> +		} else {
>>> +			ksft_test_result_skip("%s SVE_PT_VL_INHERIT set\n",
>>> +					      vec_types[i].name);
>>> +			ksft_test_result_skip("%s SVE_PT_VL_INHERIT cleared\n",
>>> +					      vec_types[i].name);
> 
>> These messages are a bit confusing. Are we skipping two tests?
> 
> ptrace_set_get_inherit() logs two test results (one for set, one for get).
> 

Ah okay.

>> These messages can be combined into one like this one on lin 572:
> 
>> ksft_test_result_skip("%s get/set VL %d\n", vec_types[i].name, vl);
> 
> If we do that then the number of planned tests won't line up with the
> number of expected tests.
> 

Sounds good. Assuming this is going through ARM tree?

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah
Catalin Marinas Jan. 28, 2022, 4:36 p.m. UTC | #4
On Mon, 24 Jan 2022 17:55:25 +0000, Mark Brown wrote:
> A couple of fairly minor fixes for the sve-ptrace test, one output thing
> and one for an issue which would generate spurious false positives.
> 
> Mark Brown (2):
>   kselftest/arm64: Skip VL_INHERIT tests for unsupported vector types
>   kselftest/arm64: Correct logging of FPSIMD register read via ptrace
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/2] kselftest/arm64: Skip VL_INHERIT tests for unsupported vector types
      https://git.kernel.org/arm64/c/50806fd91428
[2/2] kselftest/arm64: Correct logging of FPSIMD register read via ptrace
      https://git.kernel.org/arm64/c/9ae279ecabe3