Changes to the interrupt wrappers of PIC32MZ port

Hello,

I did some changes to my local interrupt wrappers and port files to save up some memory space on my PIC32 device. Now I am wondering if my changes are save, or they could lead to strange behaviour in the future. Basically I changed the code so the portSAVE_CONTEXT and portRESTORE_CONTEXT macros only get used in one place, as my final project will use lots of interrupts, and this will use up lots of space.

My new Wrapper file looks like this:

#include <xc.h>
#include <sys/asm.h>
#include "ISR_Support.h"

.set nomips16
.set noreorder

.global Interrupt_Wrapper

.set	noreorder
.set 	noat
.ent	Interrupt_Wrapper

/* Interrupt entry point */
Interrupt_Wrapper:
/* Save the current task context */
portSAVE_CONTEXT
/* Call the function to handle the interrupt */
jal s2
nop
/* Restore the context of the next task to execute */
portRESTORE_CONTEXT
.end Interrupt_Wrapper
/******************************************************************/

.extern TIMER_2_InterruptHandler

.global TIMER_2_InterruptWrapper

.set	noreorder
.set 	noat
.ent	TIMER_2_InterruptWrapper

/* Interrupt entry point */
TIMER_2_InterruptWrapper:
portSAVE_CONTEXT_PREPERATION
/* Call the function to handle the interrupt */
la s2, TIMER_2_InterruptHandler
j Interrupt_Wrapper
nop
.end TIMER_2_InterruptWrapper
/******************************************************************/

.extern EXTERNAL_2_InterruptHandler

.global EXTERNAL_2_InterruptWrapper

.set	noreorder
.set 	noat
.ent	EXTERNAL_2_InterruptWrapper

/* Interrupt entry point */
EXTERNAL_2_InterruptWrapper:
portSAVE_CONTEXT_PREPERATION
/* Call the function to handle the interrupt */
la s2, EXTERNAL_2_InterruptHandler
j Interrupt_Wrapper
nop
.end EXTERNAL_2_InterruptWrapper

The concept is, that I cut of the first part of portSAVE_CONTEXT and put it into the new macro portSAVE_CONTEXT_PREPERATION , so that I can call it in my individual Wrappers. portSAVE_CONTEXT_PREPERATION should disable the interrupts and save all the sX registers I will need later and eneable the interrupts again. Then the individual Wrapper can store the function pointer for the handler which is to be called in s2 and then call the common Wrapper. The common Wrapper will do the rest of the needed actions to save the current context, then call the pointer in s2, and restore the context again. To achieve this the following macros in ISR_Support.h changed, or in case of portSAVE_CONTEXT_PREPERATION got added.

.macro	portSAVE_CONTEXT_PREPERATION

	/* Make room for the context. First save the current status so it can be
	manipulated, and the cause and EPC registers so their original values are
	captured. */
	mfc0		k0, _CP0_CAUSE
	addiu		sp, sp, -portCONTEXT_SIZE

	#if ( __mips_hard_float == 1 ) && ( configUSE_TASK_FPU_SUPPORT == 1 )
		/* Test if we are already using the system stack. Only tasks may use the
		FPU so if we are already in a nested interrupt then the FPU context does
		not require saving. */
		la			k1, uxInterruptNesting
		lw			k1, 0(k1)
		bne			k1, zero, 2f
		nop

		/* Test if the current task needs the FPU context saving. */
		la			k1, ulTaskHasFPUContext
		lw			k1, 0(k1)
		beq			k1, zero, 1f
		nop

		/* Adjust the stack to account for the additional FPU context.*/
		addiu		sp, sp, -portFPU_CONTEXT_SIZE

	1:
		/* Save the ulTaskHasFPUContext flag. */
		sw			k1, portTASK_HAS_FPU_STACK_LOCATION(sp)

	2:
	#endif

	mfc0		k1, _CP0_STATUS

	/* Also save s7, s6 and s5 so they can be used.  Any nesting interrupts
	should maintain the values of these registers across the ISR. */
	sw			s7, 48(sp)
	sw			s6, 44(sp)
	sw			s5, 40(sp)
	sw			s4, 36(sp)
	sw			s3, 32(sp)
	sw			s2, 28(sp)
	sw			s1, 24(sp)
	sw			s0, 20(sp)
   
	.endm
   
/******************************************************************/
.macro	portSAVE_CONTEXT

	sw			k1, portSTATUS_STACK_LOCATION(sp)

	/* Prepare to enable interrupts above the current priority. */
	srl			k0, k0, 0xa
	ins 		k1, k0, 10, 7
	srl			k0, k0, 0x7 /* This copies the MSB of the IPL, but it would be an error if it was set anyway. */
	ins 		k1, k0, 18, 1
	ins			k1, zero, 1, 4

	/* s5 is used as the frame pointer. */
	add			s5, zero, sp

	/* Check the nesting count value. */
	la			k0, uxInterruptNesting
	lw			s6, (k0)

	/* If the nesting count is 0 then swap to the the system stack, otherwise
	the system stack is already being used. */
	bne			s6, zero, 1f
	nop

	/* Swap to the system stack. */
	la			sp, xISRStackTop
	lw			sp, (sp)

	/* Increment and save the nesting count. */
1:	addiu		s6, s6, 1
	sw			s6, 0(k0)

	/* s6 holds the EPC value, this is saved after interrupts are re-enabled. */
	mfc0 		s6, _CP0_EPC

	/* Re-enable interrupts. */
	mtc0		k1, _CP0_STATUS

	/* Save the context into the space just created.  s6 is saved again
	here as it now contains the EPC value.  No other s registers need be
	saved. */
	sw			ra, 120(s5)
	sw			s8, 116(s5)
	sw			t9, 112(s5)
	sw			t8, 108(s5)
	sw			t7, 104(s5)
	sw			t6, 100(s5)
	sw			t5, 96(s5)
	sw			t4, 92(s5)
	sw			t3, 88(s5)
	sw			t2, 84(s5)
	sw			t1, 80(s5)
	sw			t0, 76(s5)
	sw			a3, 72(s5)
	sw			a2, 68(s5)
	sw			a1, 64(s5)
	sw			a0, 60(s5)
	sw			v1, 56(s5)
	sw			v0, 52(s5)
	sw			s6, portEPC_STACK_LOCATION(s5)
	sw			$1, 16(s5)

	/* Save the AC0, AC1, AC2, AC3 registers from the DSP.  s6 is used as a
	scratch register. */
	mfhi		s6, $ac1
	sw			s6, 128(s5)
	mflo		s6, $ac1
	sw			s6, 124(s5)

	mfhi		s6, $ac2
	sw			s6, 136(s5)
	mflo		s6, $ac2
	sw			s6, 132(s5)

	mfhi		s6, $ac3
	sw			s6, 144(s5)
	mflo		s6, $ac3
	sw			s6, 140(s5)

	/* Save the DSP Control register */
	rddsp		s6
	sw			s6, 148(s5)

	/* ac0 is done separately to match the MX port. */
	mfhi		s6, $ac0
	sw			s6, 12(s5)
	mflo		s6, $ac0
	sw			s6, 8(s5)

	/* Save the FPU context if the nesting count was zero. */
	#if ( __mips_hard_float == 1 ) && ( configUSE_TASK_FPU_SUPPORT == 1 )
		la			s6, uxInterruptNesting
		lw			s6, 0(s6)
		addiu		s6, s6, -1
		bne			s6, zero, 1f
		nop

		/* Test if the current task needs the FPU context saving. */
		lw			s6, portTASK_HAS_FPU_STACK_LOCATION(s5)
		beq			s6, zero, 1f
		nop

		/* Save the FPU registers. */
		portSAVE_FPU_REGS ( portCONTEXT_SIZE + 8 ), s5

		/* Save the FPU status register */
		cfc1		s6, $f31
		sw			s6, (portCONTEXT_SIZE + portFPCSR_STACK_LOCATION)(s5)

		1:
	#endif

	/* Update the task stack pointer value if nesting is zero. */
	la			s6, uxInterruptNesting
	lw			s6, (s6)
	addiu		s6, s6, -1
	bne			s6, zero, 1f
	nop

	/* Save the stack pointer. */
	la			s6, uxSavedTaskStackPointer
	sw			s5, (s6)
1:
	.endm

/******************************************************************/
.macro	portRESTORE_CONTEXT

	/* Restore the stack pointer from the TCB.  This is only done if the
	nesting count is 1. */
	la			s6, uxInterruptNesting
	lw			s6, (s6)
	addiu		s6, s6, -1
	bne			s6, zero, 1f
	nop
	la			s6, uxSavedTaskStackPointer
	lw			s5, (s6)

    #if ( __mips_hard_float == 1 ) && ( configUSE_TASK_FPU_SUPPORT == 1 )
		/* Restore the FPU context if required. */
		lw			s6, portTASK_HAS_FPU_STACK_LOCATION(s5)
		beq			s6, zero, 1f
		nop

		/* Restore the FPU registers. */
		portLOAD_FPU_REGS   ( portCONTEXT_SIZE + 8 ), s5

		/* Restore the FPU status register. */
		lw			s6, ( portCONTEXT_SIZE + portFPCSR_STACK_LOCATION )(s5)
		ctc1		s6, $f31
   	#endif

1:

	/* Restore the context. */
	lw			s6, 128(s5)
	mthi		s6, $ac1
	lw			s6, 124(s5)
	mtlo		s6, $ac1

	lw			s6, 136(s5)
	mthi		s6, $ac2
	lw			s6, 132(s5)
	mtlo		s6, $ac2

	lw			s6, 144(s5)
	mthi		s6, $ac3
	lw			s6, 140(s5)
	mtlo		s6, $ac3

	/* Restore DSPControl. */
	lw			s6, 148(s5)
	wrdsp		s6

	lw			s6, 8(s5)
	mtlo		s6, $ac0
	lw			s6, 12(s5)
	mthi		s6, $ac0
	lw			$1, 16(s5)

	/* s6 is loaded as it was used as a scratch register and therefore saved
	as part of the interrupt context. */
	lw			s7, 48(s5)
	lw			s6, 44(s5)
	lw			s4, 36(s5)
	lw			s3, 32(s5)
	lw			s2, 28(s5)
	lw			s1, 24(s5)
	lw			s0, 20(s5)
	lw			v0, 52(s5)
	lw			v1, 56(s5)
	lw			a0, 60(s5)
	lw			a1, 64(s5)
	lw			a2, 68(s5)
	lw			a3, 72(s5)
	lw			t0, 76(s5)
	lw			t1, 80(s5)
	lw			t2, 84(s5)
	lw			t3, 88(s5)
	lw			t4, 92(s5)
	lw			t5, 96(s5)
	lw			t6, 100(s5)
	lw			t7, 104(s5)
	lw			t8, 108(s5)
	lw			t9, 112(s5)
	lw			s8, 116(s5)
	lw			ra, 120(s5)

	/* Protect access to the k registers, and others. */
	di
	ehb

	/* Decrement the nesting count. */
	la			k0, uxInterruptNesting
	lw			k1, (k0)
	addiu		k1, k1, -1
	sw			k1, 0(k0)

	#if ( __mips_hard_float == 1 ) && ( configUSE_TASK_FPU_SUPPORT == 1 )
		/* If the nesting count is now zero then the FPU context may be restored. */
		bne			k1, zero, 1f
		nop

		/* Restore the value of ulTaskHasFPUContext */
		la			k0, ulTaskHasFPUContext
		lw			k1, 0(s5)
		sw			k1, 0(k0)

		/* If the task does not have an FPU context then adjust the stack normally. */
		beq			k1, zero, 1f
		nop

		/* Restore the STATUS and EPC registers */
		lw			k0, portSTATUS_STACK_LOCATION(s5)
		lw			k1, portEPC_STACK_LOCATION(s5)

		/* Leave the stack in its original state.  First load sp from s5, then
		restore s5 from the stack. */
		add			sp, zero, s5
		lw			s5, 40(sp)

		/* Adjust the stack pointer to remove the FPU context */
		addiu		sp, sp,	portFPU_CONTEXT_SIZE
		beq			zero, zero, 2f
		nop

		1:  /* Restore the STATUS and EPC registers */
		lw			k0, portSTATUS_STACK_LOCATION(s5)
		lw			k1, portEPC_STACK_LOCATION(s5)

		/* Leave the stack in its original state.  First load sp from s5, then
		restore s5 from the stack. */
		add			sp, zero, s5
		lw			s5, 40(sp)

		2:  /* Adjust the stack pointer */
		addiu		sp, sp, portCONTEXT_SIZE

	#else

		/* Restore the frame when there is no hardware FP support. */
		lw			k0, portSTATUS_STACK_LOCATION(s5)
		lw			k1, portEPC_STACK_LOCATION(s5)

		/* Leave the stack in its original state.  First load sp from s5, then
		restore s5 from the stack. */
		add			sp, zero, s5
		lw			s5, 40(sp)

		addiu		sp, sp,	portCONTEXT_SIZE

	#endif // ( __mips_hard_float == 1 ) && ( configUSE_TASK_FPU_SUPPORT == 1 )

	mtc0		k0, _CP0_STATUS
	mtc0 		k1, _CP0_EPC
	ehb
	eret
	nop

	.endm

My main fear is, that s2 could change when interrupt nesting happens, because I don’t understand fully how the saving of the CPU register works.

Apologies for the delayed reply.

This is a bit intricate to know for certain without trying it out. Did you consider having a single entry point for all interrupts (so you install the same handler for all interrupts), then have that handler call a C function that looks at which interrupt actually occurred and calls another C function that handles that interrupt? The actual handlers could just be in an array of function pointers - so the first C handler picks the right handler to call from the array and installing an interrupt handler is a matter of adding it to the array.

Until now it seems to be working fine the way it is, I am just not 100% sure that it won’t cause me problems in the future.

I also thought about having one entry point and check which interrupts need handling after the context is stored, but I fear that will cost more runtime, as there are quite some interrupt sources to check.

Hi Cinderspirit,

the changes you made go into the very heart of the OS, so it is very unlikely that potential problems will show only after a long time, let alone in the field. It is very heavily used code. If no issues show up during the usual development cycle, I believe it’s safe to assume that your code is legit.