mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	 Will Deacon <will@kernel.org>, Shuah Khan <shuah@kernel.org>,
	Marc Zyngier <maz@kernel.org>,  Oliver Upton <oupton@kernel.org>,
	Fuad Tabba <fuad.tabba@linux.dev>,
	 Mark Rutland <mark.rutland@arm.com>,
	linux-kernel@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 06/11] kselftest/arm64: Factor startup code out of fp-stress load programs
Date: Wed, 30 Sep 2026 13:39:41 +0100	[thread overview]
Message-ID: <arzjdxKC2Mr-TAnq@gremlin> (raw)
In-Reply-To: <20260901-arm64-fp-stress-kvm-v1-6-31bce995b49b@kernel.org>

On Tue, Sep 01, 2026 at 06:06:46PM +0100, Mark Brown wrote:
> The fp-stress load programs all start up with some startup code which
> installs signal handlers and fetches and reports the current PID into
> x20. In order to facilitate running as KVM guests and reduce duplication
> factor this code out into a asm-utils-linux.S, making the per-test entry
> point main() similarly to more normal environments. As the
> irritator_handler varies per test they are left in place.
>
> Since the PID is now reported before main() runs it is printed before
> rather than after the vector length for those programs that report one,
> fp-stress does not parse the output so this has no impact.
>
> Since asm-utils is also used by fp-pidbench that also gets converted to
> the new _start implementation, this will install some signal handlers
> that were not previously there and report a PID which was not previously
> reported but should otherwise have no impact. Since it does not need an

You're adding enable_gcs to fp-pidbench too, maybe worth mentioning as
well?

> irritator none is provided and a weak definition left in
> asm-utils-linux.S, this will end up as 0 which happens to be SIG_DFL
> which is what we would have anyway.

Hm I'm a bit confused by the SIG_DFL thing here? You directly check for 0
if irritator_handler is not available?

>
> Signed-off-by: Mark Brown <broonie@kernel.org>

Aside from commit message nits I've had a good dig through here and it LGTM
so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  tools/testing/selftests/arm64/fp/asm-utils-linux.S | 87 ++++++++++++++++++++++
>  tools/testing/selftests/arm64/fp/fp-pidbench.S     |  6 +-
>  tools/testing/selftests/arm64/fp/fpsimd-test.S     | 76 +------------------
>  tools/testing/selftests/arm64/fp/sve-test.S        | 76 +------------------
>  tools/testing/selftests/arm64/fp/za-test.S         | 75 +------------------
>  tools/testing/selftests/arm64/fp/zt-test.S         | 75 +------------------
>  6 files changed, 106 insertions(+), 289 deletions(-)
>
> diff --git a/tools/testing/selftests/arm64/fp/asm-utils-linux.S b/tools/testing/selftests/arm64/fp/asm-utils-linux.S
> index 9cd31dc47709..9519be4d8f1e 100644
> --- a/tools/testing/selftests/arm64/fp/asm-utils-linux.S
> +++ b/tools/testing/selftests/arm64/fp/asm-utils-linux.S
> @@ -92,3 +92,90 @@ function exit_error
>  	svc	#0
>  endfunction
>  .globl	exit_error
> +
> +// Install a signal handler
> +//
> +// w0: signal number
> +// x1: sa_action
> +// w2: sa_flags
> +// Clobbers x0-x6,x8
> +function setsignal
> +	str	x30, [sp, #-((sa_sz + 15) / 16 * 16 + 16)]!
> +
> +	mov	w4, w0
> +	mov	x5, x1
> +	mov	w6, w2
> +
> +	add	x0, sp, #16
> +	mov	x1, #sa_sz
> +	bl	memclr
> +
> +	mov	w0, w4
> +	add	x1, sp, #16
> +	str	w6, [x1, #sa_flags]
> +	str	x5, [x1, #sa_handler]
> +	mov	x2, #0
> +	mov	x3, #sa_mask_sz
> +	mov	x8, #__NR_rt_sigaction
> +	svc	#0
> +
> +	cbz	w0, 1f
> +
> +	puts	"sigaction failure\n"
> +	b	exit_error
> +
> +1:	ldr	x30, [sp], #((sa_sz + 15) / 16 * 16 + 16)
> +	ret
> +endfunction
> +.globl	setsignal

LGTM, I went through manually and confirmed that this is verbatim what
every other setsignal function was doing.

> +
> +// Programs which want to be interrupted by SIGUSR1 provide an
> +// irritator_handler(), for those that don't the signal is left alone.
> +.weak	irritator_handler
> +
> +// Main program entry point.  Install the signal handlers shared by the
> +// programs, zeroing the signal count in x23 that the handlers maintain,
> +// read and report the PID the programs use for uniqueness in x20, then
> +// run the program specific main().
> +.globl _start
> +function _start
> +	enable_gcs
> +
> +	mov	x23, #0		// signal count
> +
> +	mov	w0, #SIGINT
> +	adr	x1, terminate_handler
> +	mov	w2, #SA_SIGINFO
> +	bl	setsignal
> +
> +	mov	w0, #SIGTERM
> +	adr	x1, terminate_handler
> +	mov	w2, #SA_SIGINFO
> +	bl	setsignal
> +

> +	// Load as a literal, adr can't reference the symbol when the
> +	// program provides no handler and it resolves to 0
> +	ldr	x1, =irritator_handler
> +	cbz	x1, 1f
> +	mov	w0, #SIGUSR1
> +	mov	w2, #SA_SIGINFO
> +	orr	w2, w2, #SA_NODEFER
> +	bl	setsignal

Looking through the code, I confirmed by hand that _start is the same everywhere
except for this block.

Before this was:

	mov	w0, #SIGUSR1
	adr	x1, irritator_handler
	mov	w2, #SA_SIGINFO
	orr	w2, w2, #SA_NODEFER
	bl	setsignal

So the delta is:

-	mov	w0, #SIGUSR1
-	adr	x1, irritator_handler
+	ldr	x1, =irritator_handler
+	cbz	x1, 1f

And also you've marked irritator_handler as .weak, So my understanding is
'adr' loads irritator_handler as a relative offset in x1, but since you
need to check for zero due to .weak, that can't work, hence the ldr, and
the = just puts the absolute address of irritator_handler in x1.

The 'cbz x1, 1f' is compare + branch if zero, and the 1f is 'next 1 label'
i.e. look forward for it, which is just below.

> +
> +1:	mov	w0, #SIGUSR2
> +	adr	x1, tickle_handler
> +	mov	w2, #SA_SIGINFO
> +	orr	w2, w2, #SA_NODEFER
> +	bl	setsignal
> +
> +	// Obtain our PID, to ensure test pattern uniqueness between processes
> +	mov	x8, #__NR_getpid
> +	svc	#0
> +	mov	x20, x0
> +
> +	puts	"PID:\t"
> +	mov	x0, x20
> +	bl	putdecn
> +
> +	b	main

And ack on what you mention in the commit msg, the PID output comes first
but necessary to hoist that up into the linux-specific bits.

And obviously then -> main for the rest of it.

> +endfunction
> diff --git a/tools/testing/selftests/arm64/fp/fp-pidbench.S b/tools/testing/selftests/arm64/fp/fp-pidbench.S
> index 881dfa3b342e..80f45560290d 100644
> --- a/tools/testing/selftests/arm64/fp/fp-pidbench.S
> +++ b/tools/testing/selftests/arm64/fp/fp-pidbench.S
> @@ -28,9 +28,9 @@
>  	puts	"\n"
>  .endm
>
> -// Main program entry point
> -.globl _start
> -function _start
> +// Program specific main function
> +.globl main
> +function main
>  	puts	"Iterations per test: "
>  	mov	x20, #10000
>  	lsl	x20, x20, #12
> diff --git a/tools/testing/selftests/arm64/fp/fpsimd-test.S b/tools/testing/selftests/arm64/fp/fpsimd-test.S
> index fcbb3a872490..df040283d076 100644
> --- a/tools/testing/selftests/arm64/fp/fpsimd-test.S
> +++ b/tools/testing/selftests/arm64/fp/fpsimd-test.S
> @@ -148,69 +148,11 @@ function irritator_handler
>
>  	ret
>  endfunction
> +.globl	irritator_handler
>
> -// w0: signal number
> -// x1: sa_action
> -// w2: sa_flags
> -// Clobbers x0-x6,x8
> -function setsignal
> -	str	x30, [sp, #-((sa_sz + 15) / 16 * 16 + 16)]!
> -
> -	mov	w4, w0
> -	mov	x5, x1
> -	mov	w6, w2
> -
> -	add	x0, sp, #16
> -	mov	x1, #sa_sz
> -	bl	memclr
> -
> -	mov	w0, w4
> -	add	x1, sp, #16
> -	str	w6, [x1, #sa_flags]
> -	str	x5, [x1, #sa_handler]
> -	mov	x2, #0
> -	mov	x3, #sa_mask_sz
> -	mov	x8, #__NR_rt_sigaction
> -	svc	#0
> -
> -	cbz	w0, 1f
> -
> -	puts	"sigaction failure\n"
> -	b	exit_error
> -
> -1:	ldr	x30, [sp], #((sa_sz + 15) / 16 * 16 + 16)
> -	ret
> -endfunction
> -
> -// Main program entry point
> -.globl _start
> -function _start
> -	enable_gcs
> -
> -	mov	x23, #0		// signal count
> -
> -	mov	w0, #SIGINT
> -	adr	x1, terminate_handler
> -	mov	w2, #SA_SIGINFO
> -	bl	setsignal
> -
> -	mov	w0, #SIGTERM
> -	adr	x1, terminate_handler
> -	mov	w2, #SA_SIGINFO
> -	bl	setsignal
> -
> -	mov	w0, #SIGUSR1
> -	adr	x1, irritator_handler
> -	mov	w2, #SA_SIGINFO
> -	orr	w2, w2, #SA_NODEFER
> -	bl	setsignal
> -
> -	mov	w0, #SIGUSR2
> -	adr	x1, tickle_handler
> -	mov	w2, #SA_SIGINFO
> -	orr	w2, w2, #SA_NODEFER
> -	bl	setsignal
> -
> +// Program specific main function
> +.globl main
> +function main
>  	// Sanity-check and report the vector length
>
>  	mov	x19, #128
> @@ -231,16 +173,6 @@ function _start
>  	bl	putdec
>  	puts	" bits\n"
>
> -	// Obtain our PID, to ensure test pattern uniqueness between processes
> -
> -	mov	x8, #__NR_getpid
> -	svc	#0
> -	mov	x20, x0
> -
> -	puts	"PID:\t"
> -	mov	x0, x20
> -	bl	putdecn
> -
>  	mov	x22, #0		// generation number, increments per iteration
>  .Ltest_loop:
>
> diff --git a/tools/testing/selftests/arm64/fp/sve-test.S b/tools/testing/selftests/arm64/fp/sve-test.S
> index 88c3220caf7c..c7d9514f964e 100644
> --- a/tools/testing/selftests/arm64/fp/sve-test.S
> +++ b/tools/testing/selftests/arm64/fp/sve-test.S
> @@ -316,69 +316,11 @@ function irritator_handler
>
>  	ret
>  endfunction
> +.globl	irritator_handler
>
> -// w0: signal number
> -// x1: sa_action
> -// w2: sa_flags
> -// Clobbers x0-x6,x8
> -function setsignal
> -	str	x30, [sp, #-((sa_sz + 15) / 16 * 16 + 16)]!
> -
> -	mov	w4, w0
> -	mov	x5, x1
> -	mov	w6, w2
> -
> -	add	x0, sp, #16
> -	mov	x1, #sa_sz
> -	bl	memclr
> -
> -	mov	w0, w4
> -	add	x1, sp, #16
> -	str	w6, [x1, #sa_flags]
> -	str	x5, [x1, #sa_handler]
> -	mov	x2, #0
> -	mov	x3, #sa_mask_sz
> -	mov	x8, #__NR_rt_sigaction
> -	svc	#0
> -
> -	cbz	w0, 1f
> -
> -	puts	"sigaction failure\n"
> -	b	exit_error
> -
> -1:	ldr	x30, [sp], #((sa_sz + 15) / 16 * 16 + 16)
> -	ret
> -endfunction
> -
> -// Main program entry point
> -.globl _start
> -function _start
> -	enable_gcs
> -
> -	mov	x23, #0		// Irritation signal count
> -
> -	mov	w0, #SIGINT
> -	adr	x1, terminate_handler
> -	mov	w2, #SA_SIGINFO
> -	bl	setsignal
> -
> -	mov	w0, #SIGTERM
> -	adr	x1, terminate_handler
> -	mov	w2, #SA_SIGINFO
> -	bl	setsignal
> -
> -	mov	w0, #SIGUSR1
> -	adr	x1, irritator_handler
> -	mov	w2, #SA_SIGINFO
> -	orr	w2, w2, #SA_NODEFER
> -	bl	setsignal
> -
> -	mov	w0, #SIGUSR2
> -	adr	x1, tickle_handler
> -	mov	w2, #SA_SIGINFO
> -	orr	w2, w2, #SA_NODEFER
> -	bl	setsignal
> -
> +// Program specific main function
> +.globl main
> +function main
>  #ifdef SSVE
>  	puts	"Streaming mode "
>  	smstart_sm
> @@ -404,16 +346,6 @@ function _start
>  	bl	putdec
>  	puts	" bits\n"
>
> -	// Obtain our PID, to ensure test pattern uniqueness between processes
> -
> -	mov	x8, #__NR_getpid
> -	svc	#0
> -	mov	x20, x0
> -
> -	puts	"PID:\t"
> -	mov	x0, x20
> -	bl	putdecn
> -
>  #ifdef SSVE
>  	smstart_sm		// syscalls will have exited streaming mode
>  #endif
> diff --git a/tools/testing/selftests/arm64/fp/za-test.S b/tools/testing/selftests/arm64/fp/za-test.S
> index 4a5b4b37d475..a0165a81a937 100644
> --- a/tools/testing/selftests/arm64/fp/za-test.S
> +++ b/tools/testing/selftests/arm64/fp/za-test.S
> @@ -161,69 +161,11 @@ function irritator_handler
>
>  	ret
>  endfunction
> +.globl	irritator_handler
>
> -// w0: signal number
> -// x1: sa_action
> -// w2: sa_flags
> -// Clobbers x0-x6,x8
> -function setsignal
> -	str	x30, [sp, #-((sa_sz + 15) / 16 * 16 + 16)]!
> -
> -	mov	w4, w0
> -	mov	x5, x1
> -	mov	w6, w2
> -
> -	add	x0, sp, #16
> -	mov	x1, #sa_sz
> -	bl	memclr
> -
> -	mov	w0, w4
> -	add	x1, sp, #16
> -	str	w6, [x1, #sa_flags]
> -	str	x5, [x1, #sa_handler]
> -	mov	x2, #0
> -	mov	x3, #sa_mask_sz
> -	mov	x8, #__NR_rt_sigaction
> -	svc	#0
> -
> -	cbz	w0, 1f
> -
> -	puts	"sigaction failure\n"
> -	b	exit_error
> -
> -1:	ldr	x30, [sp], #((sa_sz + 15) / 16 * 16 + 16)
> -	ret
> -endfunction
> -
> -// Main program entry point
> -.globl _start
> -function _start
> -	enable_gcs
> -
> -	mov	x23, #0		// signal count
> -
> -	mov	w0, #SIGINT
> -	adr	x1, terminate_handler
> -	mov	w2, #SA_SIGINFO
> -	bl	setsignal
> -
> -	mov	w0, #SIGTERM
> -	adr	x1, terminate_handler
> -	mov	w2, #SA_SIGINFO
> -	bl	setsignal
> -
> -	mov	w0, #SIGUSR1
> -	adr	x1, irritator_handler
> -	mov	w2, #SA_SIGINFO
> -	orr	w2, w2, #SA_NODEFER
> -	bl	setsignal
> -
> -	mov	w0, #SIGUSR2
> -	adr	x1, tickle_handler
> -	mov	w2, #SA_SIGINFO
> -	orr	w2, w2, #SA_NODEFER
> -	bl	setsignal
> -
> +// Program specific main function
> +.globl main
> +function main
>  	puts	"Streaming mode "
>  	smstart_za
>
> @@ -247,15 +189,6 @@ function _start
>  	bl	putdec
>  	puts	" bits\n"
>
> -	// Obtain our PID, to ensure test pattern uniqueness between processes
> -	mov	x8, #__NR_getpid
> -	svc	#0
> -	mov	x20, x0
> -
> -	puts	"PID:\t"
> -	mov	x0, x20
> -	bl	putdecn
> -
>  	mov	x22, #0		// generation number, increments per iteration
>  .Ltest_loop:
>  	rdsvl	0, 8
> diff --git a/tools/testing/selftests/arm64/fp/zt-test.S b/tools/testing/selftests/arm64/fp/zt-test.S
> index 82a76bb2307a..1c7db1298fb1 100644
> --- a/tools/testing/selftests/arm64/fp/zt-test.S
> +++ b/tools/testing/selftests/arm64/fp/zt-test.S
> @@ -130,80 +130,13 @@ function irritator_handler
>
>  	ret
>  endfunction
> +.globl	irritator_handler
>
> -// w0: signal number
> -// x1: sa_action
> -// w2: sa_flags
> -// Clobbers x0-x6,x8
> -function setsignal
> -	str	x30, [sp, #-((sa_sz + 15) / 16 * 16 + 16)]!
> -
> -	mov	w4, w0
> -	mov	x5, x1
> -	mov	w6, w2
> -
> -	add	x0, sp, #16
> -	mov	x1, #sa_sz
> -	bl	memclr
> -
> -	mov	w0, w4
> -	add	x1, sp, #16
> -	str	w6, [x1, #sa_flags]
> -	str	x5, [x1, #sa_handler]
> -	mov	x2, #0
> -	mov	x3, #sa_mask_sz
> -	mov	x8, #__NR_rt_sigaction
> -	svc	#0
> -
> -	cbz	w0, 1f
> -
> -	puts	"sigaction failure\n"
> -	b	exit_error
> -
> -1:	ldr	x30, [sp], #((sa_sz + 15) / 16 * 16 + 16)
> -	ret
> -endfunction
> -
> -// Main program entry point
> -.globl _start
> -function _start
> -	enable_gcs
> -
> -	mov	x23, #0		// signal count
> -
> -	mov	w0, #SIGINT
> -	adr	x1, terminate_handler
> -	mov	w2, #SA_SIGINFO
> -	bl	setsignal
> -
> -	mov	w0, #SIGTERM
> -	adr	x1, terminate_handler
> -	mov	w2, #SA_SIGINFO
> -	bl	setsignal
> -
> -	mov	w0, #SIGUSR1
> -	adr	x1, irritator_handler
> -	mov	w2, #SA_SIGINFO
> -	orr	w2, w2, #SA_NODEFER
> -	bl	setsignal
> -
> -	mov	w0, #SIGUSR2
> -	adr	x1, tickle_handler
> -	mov	w2, #SA_SIGINFO
> -	orr	w2, w2, #SA_NODEFER
> -	bl	setsignal
> -
> +// Program specific main function
> +.globl main
> +function main
>  	smstart_za
>
> -	// Obtain our PID, to ensure test pattern uniqueness between processes
> -	mov	x8, #__NR_getpid
> -	svc	#0
> -	mov	x20, x0
> -
> -	puts	"PID:\t"
> -	mov	x0, x20
> -	bl	putdecn
> -
>  	mov	x22, #0		// generation number, increments per iteration
>  .Ltest_loop:
>  	mov	x0, x20
>
> --
> 2.47.3
>

--
Cheers, Lorenzo

  reply	other threads:[~2026-09-30 12:39 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01 17:06 [PATCH 00/11] kselftest/arm64: Add coverage of interactions with KVM to fp-stress Mark Brown
2026-09-01 17:06 ` [PATCH 01/11] kselftest/arm64: Factor Linux syscalls out of asm-utils.S Mark Brown
2026-09-29 16:01   ` Lorenzo Stoakes (ARM)
2026-09-01 17:06 ` [PATCH 02/11] kselftest/arm64: Factor shared signal handlers out of fp-stress loads Mark Brown
2026-09-29 16:07   ` Lorenzo Stoakes (ARM)
2026-09-01 17:06 ` [PATCH 03/11] kselftest/arm64: Move exit calls " Mark Brown
2026-09-29 16:22   ` Lorenzo Stoakes (ARM)
2026-09-29 21:56     ` Mark Brown
2026-09-30  8:41       ` Lorenzo Stoakes (ARM)
2026-09-29 16:27   ` Mark Rutland
2026-09-29 22:03     ` Mark Brown
2026-09-30  8:51     ` Mark Rutland
2026-09-01 17:06 ` [PATCH 04/11] kselftest/arm64: Use exit_error() rather than SIGABRT in fp-stress Mark Brown
2026-09-29 16:28   ` Mark Rutland
2026-09-29 16:31   ` Lorenzo Stoakes (ARM)
2026-09-01 17:06 ` [PATCH 05/11] kselftest/arm64: Exit with an error code on data mismatches " Mark Brown
2026-09-29 16:37   ` Lorenzo Stoakes (ARM)
2026-09-01 17:06 ` [PATCH 06/11] kselftest/arm64: Factor startup code out of fp-stress load programs Mark Brown
2026-09-30 12:39   ` Lorenzo Stoakes (ARM) [this message]
2026-09-01 17:06 ` [PATCH 07/11] kselftest/arm64: Remove the sched_yield()s from the fp-stress loads Mark Brown
2026-09-30 14:17   ` Lorenzo Stoakes (ARM)
2026-09-01 17:06 ` [PATCH 08/11] kselftest/arm64: Add a very simple VMM for use in fp-stress Mark Brown
2026-09-01 17:06 ` [PATCH 09/11] kselftest/arm64: Build KVM guest versions of the fp-stress loads Mark Brown
2026-09-01 17:06 ` [PATCH 10/11] kselftest/arm64: Use execv() to start fp-stress test loads Mark Brown
2026-09-01 17:06 ` [PATCH 11/11] kselftest/arm64: Run KVM guests from fp-stress Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arzjdxKC2Mr-TAnq@gremlin \
    --to=ljs@kernel.org \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=fuad.tabba@linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=shuah@kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®