arch.interrupt test fails on ARM64 QEMU targets when compiled with GCC 12

[stephanosio]

Describe the bug

The arch.interrupt test fails on the ARM64 QEMU targets (qemu_cortex_a53 and qemu_cortex_a53_smp) when compiled with GCC 12, which is the toolchain included in the Zephyr SDK 0.15.0.

To Reproduce

With Zephyr SDK 0.15.0,

scripts/twister -v -N -M -p qemu_cortex_a53 -s tests/kernel/interrupt/arch.interrupt

Expected behavior

Test passes.

Logs and console output

Running TESTSUITE interrupt_feature
===================================================================
START - test_isr_dynamic
installing dynamic ISR for IRQ 0
 PASS - test_isr_dynamic in 0.001 seconds
===================================================================
START - test_isr_offload_job
 SKIP - test_isr_offload_job in 0.001 seconds
===================================================================
START - test_isr_offload_job_identi
 PASS - test_isr_offload_job_identi in 0.015 seconds
===================================================================
START - test_isr_offload_job_multiple
 PASS - test_isr_offload_job_multiple in 0.021 seconds
===================================================================
START - test_nested_isr
Triggering irq : 14
isr0: Enter
Triggering irq : 15

    Assertion failed at WEST_TOPDIR/zephyr-testing/tests/kernel/interrupt/src/nested_irq.c:106: isr0: (isr1_result not equal to ISR1_TOKEN)
isr1 did not execute
isr0: Leave
isr1: Enter
isr1: Leave
 FAIL - test_nested_isr in 0.006 seconds
===================================================================
START - test_prevent_interruption
locking interrupts
unlocking interrupts
timer fired
 PASS - test_prevent_interruption in 2.540 seconds
===================================================================
TESTSUITE interrupt_feature failed.

------ TESTSUITE SUMMARY START ------

SUITE FAIL -  80.00% [interrupt_feature]: pass = 4, fail = 1, skip = 1, total = 6 duration = 2.584 seconds
 - PASS - [interrupt_feature.test_isr_dynamic] duration = 0.001 seconds
 - SKIP - [interrupt_feature.test_isr_offload_job] duration = 0.001 seconds
 - PASS - [interrupt_feature.test_isr_offload_job_identi] duration = 0.015 seconds
 - PASS - [interrupt_feature.test_isr_offload_job_multiple] duration = 0.021 seconds
 - FAIL - [interrupt_feature.test_nested_isr] duration = 0.006 seconds
 - PASS - [interrupt_feature.test_prevent_interruption] duration = 2.540 seconds

------ TESTSUITE SUMMARY END ------

===================================================================
RunID: 548cf47f515fc665f4459ceaf48fa404
PROJECT EXECUTION FAILED

Environment (please complete the following information):

• OS: Ubuntu 20.04
• Toolchain: Zephyr SDK 0.15.0
• Commit SHA: 822c377

Additional context

Adding a printk call in the arm_gic_get_active function makes this test pass even when compiling with GCC 12.

[stephanosio]

cc @carlocaione @npitre @povergoing @SgrrZhf

[povergoing]

Weird, I need to take a further look. fvp_base_revc_2xaemv8a also fails but fvp_baser_aemv8r has no issue

[povergoing]

I found it. Any changes related to printk, more specifically, va_list, va_start and va_end? Compared with the previous version, I found GCC12 will insert a ldp q0, q1, [sp, #xx] and ldp q0, q1, [sp, #48], for example,
Old version

    88001f70:   3d8017e0        str     q0, [sp, #80]
    88001f74:   3d801be1        str     q1, [sp, #96]
    88001f78:   3d801fe2        str     q2, [sp, #112]
    88001f7c:   3d8023e3        str     q3, [sp, #128]
    88001f80:   3d8027e4        str     q4, [sp, #144]
    88001f84:   3d802be5        str     q5, [sp, #160]
    88001f88:   3d802fe6        str     q6, [sp, #176]
    88001f8c:   3d8033e7        str     q7, [sp, #192]
    88001f90:   a90f9be5        stp     x5, x6, [sp, #248]
    88001f94:   f90087e7        str     x7, [sp, #264]
        vprintk(fmt, ap);
    88001f98:   97ffffd6        bl      88001ef0 <vprintk>

GCC12:

    88001ef4:   3d8017e0        str     q0, [sp, #80]
    88001ef8:   3d801be1        str     q1, [sp, #96]
        vprintk(fmt, ap);
    88001efc:   ad4187e0        ldp     q0, q1, [sp, #48]
{
    88001f00:   3d801fe2        str     q2, [sp, #112]
    88001f04:   3d8023e3        str     q3, [sp, #128]
        vprintk(fmt, ap);
    88001f08:   ad0087e0        stp     q0, q1, [sp, #16]
{
    88001f0c:   3d8027e4        str     q4, [sp, #144]
    88001f10:   3d802be5        str     q5, [sp, #160]
    88001f14:   3d802fe6        str     q6, [sp, #176]
    88001f18:   3d8033e7        str     q7, [sp, #192]
    88001f1c:   a90e93e3        stp     x3, x4, [sp, #232]
    88001f20:   a90f9be5        stp     x5, x6, [sp, #248]
    88001f24:   f90087e7        str     x7, [sp, #264]
        vprintk(fmt, ap);
    88001f28:   97ffffd9        bl      88001e8c <vprintk>

That will cause an issue with CONFIG_FPU_SHARING because this feature simulates the str qx, qy [sp #xx] for performance I think?

zephyr/arch/arm64/core/fpu.c

Lines 147 to 157 in 180f50f

	* In many cases, the FPU trap is triggered by va_start() that copies
	* the content of FP registers used for floating point argument passing
	* into the va_list object in case there were actual float arguments from
	* the caller. In practice this is almost never the case, especially if
	* FPU access is disabled and we're trapped while in exception context.
	* Rather than flushing the FPU context to its owner and enabling access
	* just to let the corresponding STR instructions execute, we simply
	* simulate them and leave the FPU access disabled. This also avoids the
	* need for disabling interrupts in syscalls and IRQ handlers as well.
	*/
	static bool simulate_str_q_insn(z_arch_esf_t *esf)

But doesn't simulate the ldp q0, q1, [sp, #48] instruction which will case a fpu trap, enabling the fpu and disabling the IRQ and thus forbid a nested irq. Thus test fails.

zephyr/arch/arm64/core/fpu.c

Lines 242 to 251 in 180f50f

	if (arch_exception_depth() > 1) {
	/*
	* We were already in exception when the FPU access trap.
	* We give it access and prevent any further IRQ recursion
	* by disabling IRQs as we wouldn't be able to preserve the
	* interrupted exception's FPU context.
	*/
	esf->spsr |= DAIF_IRQ_BIT;
	return;
	}

Is there any config for GCC12 to go back to the original va_list compiling behavior? Or does anyone think it will be better to simulate the ldr instruction (also plus a stp)?

[npitre]

Yep, more context in commit b8d24ff ("arm64: mitigate
FPU-in-exception usage side effects").

Is there any config for GCC12 to go back to the original va_list
compiling behavior?

I wonder why it has to move 32 bytes between two locations on the stack
like that in the first place.

Or does anyone think it will be better to simulate the ldr
instruction (also plus a stp)?

If gcc has a legitimate reason for doing this then yes.

[povergoing]

I wonder why it has to move 32 bytes between two locations on the stack
like that in the first place.

Yes, good question. I am trying to find out why :)

[povergoing]

I wonder why it has to move 32 bytes between two locations on the stack
like that in the first place.

@npitre Seems the GCC12 uses qn registers to copy content to substitute the original xn ones, for performance improvement maybe?
For example,

ldp     x4, x5, [x1]
ldp     x2, x3, [x1, #16]
stp     x4, x5, [sp]
stp     x2, x3, [sp, #16]

=>

ldp     q0, q1, [x1]
stp     q0, q1, [sp]

So, we may need to simulate the ldp and stp instructions. But due to the optimization, this pair (ldp and stp) of instructions can appear randomly, which is probably located separately from those str qx, [sp, #xx]s, which means the simulation would be done with several traps instead of the previous one (trap one time, simulate all and skip by fixing the pc before eret)
Something like:

str     q0, [sp, #80]
str     q1, [sp, #96]
...
str     q7, [sp, #192]
...
normal instructions
...
ldp     q0, q1, [x1]
...
normal instructions
...
stp     q0, q1, [sp]

or

str     q0, [sp, #80]
str     q1, [sp, #96]

ldp     q0, q1, [sp, #48]

str     q2, [sp, #112]
str     q3, [sp, #128]

stp     q0, q1, [sp, #16]

str     q4, [sp, #144]
str     q5, [sp, #160]
str     q6, [sp, #176]
str     q7, [sp, #192]