From 2cdf925f942c17155b6b8b8611d6171f4b1c2ad7 Mon Sep 17 00:00:00 2001 From: Lephe Date: Wed, 6 May 2020 16:37:44 +0200 Subject: [PATCH] interrupts: save caller-saved registers in main handler This is an obvious requirement for the interrupt routine, which was forgotten and only surfaced when I used a timer callback started with multiplications in an innocent add-in. r0..r7 are saved automatically, which leaves pr, gbr, mach et macl susceptible to corruption by the interrupt handler. --- include/gint/gint.h | 13 +++++++----- src/core/exch.c | 5 +++++ src/core/gint.c | 2 +- src/core/inth.S | 48 +++++++++++++++++++++++++++++++++++++-------- src/core/setup.c | 2 +- src/dma/inth.s | 2 +- src/rtc/inth.s | 2 +- src/tmu/inth.s | 8 ++++---- 8 files changed, 61 insertions(+), 21 deletions(-) diff --git a/include/gint/gint.h b/include/gint/gint.h index 2d8e69f..9ad14b0 100644 --- a/include/gint/gint.h +++ b/include/gint/gint.h @@ -97,21 +97,24 @@ int gint_intlevel(int intid, int level); list of event codes and their associated interrupts. The handler function must be an interrupt handler: it must not raise - exceptions, must end with 'rte', and it will use the kernel register bank. + exceptions, must end with 'rts', and it will use the kernel register bank. + You read that right, it must end with rts because gint's main handler saves + registers besides the automated r0..r7. Do *not* use 'rte' in the handler. For convenience I allow any block size to be loaded as an interrupt handler, but it should really be a multiple of 0x20 bytes and not override other handlers. If it's not written in assembler, then you're likely doing - something wrong, even with __attribute__((interrupt_handler)). + something wrong, especially with __attribute__((interrupt_handler)) which + uses rte. It is common for interrupt handlers to have a few bytes of data, such as the address of a callback function. gint often stores this data in the last - bytes of the block. This function returns the VBR address of the block to - allow the caller to edit the parameters. + bytes of the block. This function returns the VBR address of the block which + has just been installed, to allow the caller to edit the parameters later. @event_code Identifier of the interrupt block @handler Address of handler function @size How many bytes to copy Returns the VBR address where the handlers was installed. */ -void *gint_inthandler(int event_code, const void *handler, size_t size); +void *gint_inthandler(int event_code, void const *handler, size_t size); #endif /* GINT_GINT */ diff --git a/src/core/exch.c b/src/core/exch.c index 2213526..54d044c 100644 --- a/src/core/exch.c +++ b/src/core/exch.c @@ -3,6 +3,7 @@ #include #include #include +#include #define dprint(x, y, ...) dprint(x, y, C_BLACK, C_NONE, __VA_ARGS__) #define dtext(x, y, str) dtext (x, y, str, C_BLACK, C_NONE) @@ -16,6 +17,9 @@ GNORETURN static void gint_default_panic(GUNUSED uint32_t code) uint32_t PC; __asm__("stc spc, %0" : "=r"(PC)); + uint32_t SGR = 0xffffffff; + if(isSH4()) __asm__("stc sgr, %0" : "=r"(SGR)); + dfont(NULL); dclear(C_WHITE); @@ -42,6 +46,7 @@ GNORETURN static void gint_default_panic(GUNUSED uint32_t code) dprint(1, 17, " PC :%08x", PC); dprint(1, 25, "TEA :%08x", TEA); dprint(1, 33, "TRA :%08x", TRA); + if(isSH4()) dprint(1, 41, "SGR :%08x", SGR); dtext(1, 49, "The add-in crashed."); dtext(1, 57, "Please reset the calc"); diff --git a/src/core/gint.c b/src/core/gint.c index 98c4459..802748c 100644 --- a/src/core/gint.c +++ b/src/core/gint.c @@ -58,6 +58,6 @@ void *gint_inthandler(int event_code, const void *handler, size_t size) /* Prevent overriding the entry gate */ if(event_code < 0) return NULL; - void *dest = (void *)&gint_vbr + event_code + 0x620; + void *dest = (void *)&gint_vbr + event_code + 0x640; return memcpy(dest, handler, size); } diff --git a/src/core/inth.S b/src/core/inth.S index 01672eb..153781f 100644 --- a/src/core/inth.S +++ b/src/core/inth.S @@ -54,9 +54,16 @@ _gint_inth_7305: 1: .long 0xff000028 #endif -/* SH7305-TYPE INTERRUPT HANDLER ENTRY - 20 BYTES */ +/* SH7305-TYPE INTERRUPT HANDLER ENTRY - 40 BYTES */ _gint_inth_7305: + /* Save caller-saved registers which might currently be in use by the + interrupted function, as we don't know what the callback will do */ + sts.l pr, @-r15 + stc.l gbr, @-r15 + sts.l mach, @-r15 + sts.l macl, @-r15 + /* Get the event code from the INTEVT register */ mov.l 1f, r0 mov.l @r0, r0 @@ -66,19 +73,34 @@ _gint_inth_7305: shll8 r1 sub r1, r0 - /* Add the distance to the first entry and jump */ - add #16, r0 - braf r0 + /* Add the distance to the first entry and jump as a subroutine */ + add #40, r0 + bsrf r0 nop - .zero 12 + /* Restore caller-saved registers */ + lds.l @r15+, macl + lds.l @r15+, mach + ldc.l @r15+, gbr + lds.l @r15+, pr + + rte + nop + + .zero 24 1: .long 0xff000028 #ifdef FX9860G -/* SH7705-TYPE INTERRUT HANDLER ENTRY - 32 BYTES */ +/* SH7705-TYPE INTERRUT HANDLER ENTRY - 52 BYTES */ _gint_inth_7705: + /* Save caller-saved registers as before */ + sts.l pr, @-r15 + stc.l gbr, @-r15 + sts.l mach, @-r15 + sts.l macl, @-r15 + /* Get the event code from the INTEVT2 register */ mov.l 1f, r0 mov.l @r0, r0 /* r0 = old_code */ @@ -93,10 +115,20 @@ _gint_inth_7705: shld r3, r0 /* r0 = new_code - 0x400 */ /* Add the distance between nop and the first entry, and jump */ - add #8, r0 - braf r0 + add #32, r0 + bsrf r0 nop + /* Restore saved registers */ + lds.l @r15+, mach + lds.l @r15+, macl + ldc.l @r15+, gbr + lds.l @r15+, pr + + rte + nop + + .zero 12 1: .long 0xa4000000 /* INTEVT2 register */ 2: .long _inth_remap diff --git a/src/core/setup.c b/src/core/setup.c index 03216f7..1939af7 100644 --- a/src/core/setup.c +++ b/src/core/setup.c @@ -85,7 +85,7 @@ void gint_install(void) /* Load the event handler entry points into memory */ memcpy(vbr + 0x100, gint_exch_tlbh, exch_tlbh_size); memcpy(vbr + 0x400, gint_exch_tlbh, exch_tlbh_size); - memcpy(vbr + 0x600, inth_entry, 32); + memcpy(vbr + 0x600, inth_entry, 64); /* Time to switch VBR and roll! */ system_vbr = gint_setvbr((uint32_t)vbr, lock); diff --git a/src/dma/inth.s b/src/dma/inth.s index e540534..53a4ffb 100644 --- a/src/dma/inth.s +++ b/src/dma/inth.s @@ -25,7 +25,7 @@ _inth_dma_te: and r2, r0 mov.w r0, @r1 - rte + rts nop 1: .long 0 /* CHCR, set dynamically */ diff --git a/src/rtc/inth.s b/src/rtc/inth.s index 69544d3..b21b5f7 100644 --- a/src/rtc/inth.s +++ b/src/rtc/inth.s @@ -54,7 +54,7 @@ _inth_rtc_pri_helper: .end: lds.l @r15+, pr - rte + rts nop .zero 8 diff --git a/src/tmu/inth.s b/src/tmu/inth.s index 7eb0307..a7ad4bb 100644 --- a/src/tmu/inth.s +++ b/src/tmu/inth.s @@ -81,7 +81,7 @@ _inth_tmu_1: .end: lds.l @r15+, pr - rte + rts nop .zero 12 @@ -176,7 +176,7 @@ _inth_etmu_help: nop lds.l @r15+, pr - rte + rts nop .zero 18 @@ -199,10 +199,10 @@ _inth_etmux: /* Offset from VBR where extra timer 2 is located: - 0x600 to reach the interrupt handlers - - 0x020 to jump over the entry gate + - 0x040 to jump over the entry gate - 0x840 to reach the handler of ETMU2 - 0x004 to skip its first instructions (the size is hardcoded) */ -1: .long 0xe64 +1: .long 0xe84 .id_etmux: .long 0 /* Timer ID */