Incorrect nested trap handling

[vacantron]

In the current implementation, we fill the field mstatus.mie with mstatus.mpie and then save mstatus before re-enabling the global interrupt.

linmo/arch/riscv/hal.c

Lines 441 to 450 in ceee53c

	/* Save mstatus with interrupt state reconstruction. During timer
	* interrupts, mstatus.MIE is cleared, so we reconstruct the pre-trap
	* state from MPIE for consistent interrupt context preservation.
	*/
	"csrr t0, mstatus\n" /* Read current mstatus (MIE=0 in trap) */
	"andi t1, t0, ~8\n" /* Clear MIE bit first */
	"srli t2, t0, 4\n" /* Get MPIE bit to position 3 */
	"andi t2, t2, 8\n" /* Isolate bit 3 */
	"or t1, t1, t2\n" /* Combine cleared MIE with reconstructed bit */
	"sw t1, 16*4(%0)\n"

However, according to the specification 3.1.6.1. Privilege and Global Interrupt-Enable Stack in mstatus register:

When a trap is taken from privilege mode y into privilege mode x, xPIE is set to the value of xIE; xIE is set to 0; and xPP is set to y.

and

An MRET or SRET instruction is used to return from a trap in M-mode or S-mode respectively. When executing an xRET instruction, supposing xPP holds the value y, xIE is set to xPIE; the privilege mode is changed to y; xPIE is set to 1; and xPP is set to the least-privileged supported mode

The field mstatus.mie and mstatus.mpie would be maintained by the hardware when entering and leaving the ISR, so we do not need to manage these fields here.

The only thing we need to do here is saving mstatus and enabling mstatus.mie again to accept the incoming further interrupts or exceptions, and restoring them before leaving the ISR. The former could be moved to _isr() to keep the HAL clean.

[visitorckw]

I agree that we don't need to manually set mstatus.mie using mstatus.mpie.
But the issue title feels a bit misleading to me.
Currently, the only interrupt we actually handle is the timer interrupt from the clint, and mie is disabled during its handler, so no other interrupt should occur at that time.
So based on the title alone, we wouldn't expect nested traps to happen?

[vacantron]

I found this issue by using GDB to pause QEMU, and there was another timer interrupt being triggered when we resumed from the pausing. By default, QEMU would raise a timer interrupt in a certain time interval. If we were still in the ISR but enabled mstatus.mie, the nested trap occurred. i.e.,

linmo/kernel/task.c

Line 502 in ceee53c

hal_interrupt_tick();

By saving and restoring the necessary registers in the ISR, the kernel could serve the nested interrupt or exception correctly. If we do not want the nested trap allowed, the field mstatus.mie should not be modified during the ISR.