Skip to content

Add the optional interrupt stack to the Xtensa architecture #2014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
masayuki2009 merged 2 commits into from
Oct 26, 2020
Merged

Add the optional interrupt stack to the Xtensa architecture #2014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af4bb9e
arch/xtensa: Add the optional interrupt stack.
Ouss4 Oct 12, 2020
06b4a51
arch/xtensa/src/esp32: SMP case of interruptstack.
Ouss4 Oct 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions arch/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ config ARCH_X86_64

config ARCH_XTENSA
bool "Xtensa"
select ARCH_HAVE_INTERRUPTSTACK
select ARCH_HAVE_STACKCHECK
select ARCH_HAVE_CUSTOMOPT
select ARCH_HAVE_TESTSET
Expand Down
34 changes: 18 additions & 16 deletions arch/xtensa/src/common/xtensa.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,16 @@

/* Check if an interrupt stack size is configured */

#define HAVE_INTERRUPTSTACK 1

#if !defined(CONFIG_ARCH_INTERRUPTSTACK)
#ifndef CONFIG_ARCH_INTERRUPTSTACK
# define CONFIG_ARCH_INTERRUPTSTACK 0
# undef HAVE_INTERRUPTSTACK
#elif CONFIG_ARCH_INTERRUPTSTACK < 16
# warning CONFIG_ARCH_INTERRUPTSTACK is to small
# undef HAVE_INTERRUPTSTACK
#else
# define INTSTACK_ALIGNMENT 16
# define INTSTACK_ALIGN_MASK (INTSTACK_ALIGNMENT - 1)
# define INTSTACK_ALIGNDOWN(s) ((s) & ~INTSTACK_ALIGN_MASK)
# define INTSTACK_ALIGNUP(s) (((s) + INTSTACK_ALIGN_MASK) & ~INTSTACK_ALIGN_MASK)
# define INTSTACK_SIZE INTSTACK_ALIGNUP(CONFIG_ARCH_INTERRUPTSTACK)
#endif

#define INTERRUPTSTACK_SIZE ((CONFIG_ARCH_INTERRUPTSTACK + 15) & ~15)
#define INTERRUPT_STACKWORDS (INTERRUPTSTACK_SIZE >> 2)

/* An IDLE thread stack size for CPU0 must be defined */

#if !defined(CONFIG_IDLETHREAD_STACKSIZE)
Expand All @@ -109,10 +106,6 @@
#define IDLETHREAD_STACKSIZE ((CONFIG_IDLETHREAD_STACKSIZE + 15) & ~15)
#define IDLETHREAD_STACKWORDS (IDLETHREAD_STACKSIZE >> 2)

/* Used for stack usage measurements */

#define STACK_COLOR 0xdeadbeef

/* In the XTENSA model, the state is copied from the stack to the TCB, but
* only a referenced is passed to get the state from the TCB.
*
Expand Down Expand Up @@ -150,6 +143,14 @@
#define getreg32(a) (*(volatile uint32_t *)(a))
#define putreg32(v,a) (*(volatile uint32_t *)(a) = (v))

/* This is the value used to mark the stack for subsequent stack monitoring
* logic.
*/

#define STACK_COLOR 0xdeadbeef
#define INTSTACK_COLOR 0xdeadbeef
#define HEAP_COLOR 'h'

/****************************************************************************
* Public Types
****************************************************************************/
Expand Down Expand Up @@ -181,10 +182,11 @@ extern volatile uint32_t *g_current_regs[1];

#endif

#ifdef HAVE_INTERRUPTSTACK
#if !defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 15
/* The (optional) interrupt stack */

extern uint32_t g_intstack[INTERRUPT_STACKWORDS];
extern uint32_t g_intstackalloc; /* Allocated interrupt stack */
extern uint32_t g_intstackbase; /* Initial top of interrupt stack */
#endif

/* Address of the CPU0 IDLE thread */
Expand Down
12 changes: 8 additions & 4 deletions arch/xtensa/src/common/xtensa_checkstack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@

#include "xtensa.h"
#include "sched/sched.h"
#include "chip_macros.h"

#ifdef CONFIG_STACK_COLORATION

Expand Down Expand Up @@ -197,16 +198,19 @@ ssize_t up_check_stack_remain(void)
return up_check_tcbstack_remain(this_task());
}

#if CONFIG_ARCH_INTERRUPTSTACK > 3
#if CONFIG_ARCH_INTERRUPTSTACK > 15
size_t up_check_intstack(void)
{
return do_stackcheck((uintptr_t)&g_intstackalloc,
(CONFIG_ARCH_INTERRUPTSTACK & ~3));
#ifdef CONFIG_SMP
return do_stackcheck(xtensa_intstack_alloc(), INTSTACK_SIZE);
#else
return do_stackcheck((uintptr_t)&g_intstackalloc, INTSTACK_SIZE);
#endif
}

size_t up_check_intstack_remain(void)
{
return (CONFIG_ARCH_INTERRUPTSTACK & ~3) - up_check_intstack();
return INTSTACK_SIZE - up_check_intstack();
}
#endif

Expand Down
32 changes: 20 additions & 12 deletions arch/xtensa/src/common/xtensa_dumpstate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

#include "sched/sched.h"
#include "xtensa.h"
#include "chip_macros.h"

#ifdef CONFIG_DEBUG_ALERT

Expand Down Expand Up @@ -163,7 +164,7 @@ void xtensa_dumpstate(void)
uint32_t sp = xtensa_getsp();
uint32_t ustackbase;
uint32_t ustacksize;
#ifdef HAVE_INTERRUPTSTACK
#if CONFIG_ARCH_INTERRUPTSTACK > 15
uint32_t istackbase;
uint32_t istacksize;
#endif
Expand All @@ -185,10 +186,13 @@ void xtensa_dumpstate(void)

/* Get the limits on the interrupt stack memory */

#warning REVISIT interrupt stack
#ifdef HAVE_INTERRUPTSTACK
istackbase = (uint32_t)&g_intstack[INTERRUPT_STACKWORDS - 1];
istacksize = INTERRUPTSTACK_SIZE;
#if CONFIG_ARCH_INTERRUPTSTACK > 15
#ifdef CONFIG_SMP
istackbase = (uint32_t)xtensa_intstack_base();
#else
istackbase = (uint32_t)&g_intstackbase;
#endif
istacksize = INTSTACK_SIZE;

/* Show interrupt stack info */

Expand All @@ -209,20 +213,24 @@ void xtensa_dumpstate(void)
/* Yes.. dump the interrupt stack */

xtensa_stackdump(sp, istackbase);

/* Extract the user stack pointer which should lie
* at the base of the interrupt stack.
*/

sp = &g_instack[INTERRUPTSTACK_SIZE - sizeof(uint32_t)];
_alert("sp: %08x\n", sp);
}
else if (CURRENT_REGS)
{
_alert("ERROR: Stack pointer is not within the interrupt stack\n");
xtensa_stackdump(istackbase - istacksize, istackbase);
}

/* Extract the user stack pointer if we are in an interrupt handler.
* If we are not in an interrupt handler. Then sp is the user stack
* pointer (and the above range check should have failed).
*/

if (CURRENT_REGS)
{
sp = CURRENT_REGS[REG_A1];
_alert("sp: %08x\n", sp);
}

/* Show user stack info */

_alert("User stack:\n");
Expand Down
40 changes: 40 additions & 0 deletions arch/xtensa/src/common/xtensa_initialize.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,44 @@
#include <arch/board/board.h>

#include "xtensa.h"
#include "chip_macros.h"

/****************************************************************************
* Private Functions
****************************************************************************/

/****************************************************************************
* Name: xtensa_color_intstack
*
* Description:
* Set the interrupt stack to a value so that later we can determine how
* much stack space was used by interrupt handling logic
*
****************************************************************************/

#if defined(CONFIG_STACK_COLORATION) && CONFIG_ARCH_INTERRUPTSTACK > 15
static inline void xtensa_color_intstack(void)
{
#ifdef CONFIG_SMP
uint32_t *ptr = (uint32_t *)xtensa_intstack_alloc();
#else
uint32_t *ptr = (uint32_t *)&g_intstackalloc;
#endif
ssize_t size;

#ifdef CONFIG_SMP
for (size = INTSTACK_SIZE * CONFIG_SMP_NCPUS;
#else
for (size = INTSTACK_SIZE;
#endif
size > 0; size -= sizeof(uint32_t))
{
*ptr++ = INTSTACK_COLOR;
}
}
#else
# define xtensa_color_intstack()
#endif

/****************************************************************************
* Public Functions
Expand All @@ -81,6 +119,8 @@

void up_initialize(void)
{
xtensa_color_intstack();

#ifdef CONFIG_SMP
int i;

Expand Down
52 changes: 44 additions & 8 deletions arch/xtensa/src/common/xtensa_int_handlers.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,19 @@
#include "chip_macros.h"
#include "xtensa_timer.h"

#if !defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 15
.data
.align 16
Copy link
Contributor

@masayuki2009 masayuki2009 Oct 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Xtensa architecture, does .align 16 mean 16-byte alignment?
For ARM architecture, .align n means 2^n but it confuses me. So I will refactor them for ARM later.

However, we can use .balign instead of .align.
Please see the following URL.
https://developer.arm.com/documentation/dui0742/c/migrating-arm-syntax-assembly-code-to-gnu-syntax/alignment

Copy link
Member Author

@Ouss4 Ouss4 Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it means 16-byte alignement. (ref. https://sourceware.org/binutils/docs-2.25/as/Align.html)
Here is the relevant text:

The way the required alignment is specified varies from system to system. For ....and xtensa, the first expression is the alignment request in bytes.

There is also a clearer example here.

.global g_intstackalloc
.global g_intstackbase
.type g_intstackalloc, @object
.type g_intstackbase, @object
g_intstackalloc:
.skip INTSTACK_SIZE
g_intstackbase:
.size g_intstackalloc, .-g_intstackalloc
#endif

/****************************************************************************
* Assembly Language Macros
****************************************************************************/
Expand All @@ -91,6 +104,21 @@
addi \aout, \aout, 1 /* Return aout + 1 */
.endm

/************************************************************************************
* Name: setintstack
*
* Description:
* Set the current stack pointer to the "top" the interrupt stack. Single CPU
* case. Must be provided by MCU-specific logic in the SMP case.
*
************************************************************************************/

#if !defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 15
.macro setintstack tmp1 tmp2
movi a1, g_intstackbase
.endm
#endif

/****************************************************************************
* Macro dispatch_c_isr level mask
*
Expand All @@ -112,6 +140,7 @@
* Entry Conditions/Side Effects:
* level - interrupt level
* mask - interrupt bitmask for this level
* a12 - register save area
*
* Exit Conditions:
* This macro will use registers a0 and a2-a5 and a12.
Expand All @@ -122,12 +151,6 @@

.macro dispatch_c_isr level mask

/* Initially the register save area is in SP, but that could change as
* a consequence of context switching.
*/

mov a12, sp /* Address of save area */

#ifdef __XTENSA_CALL0_ABI__
/* Get mask of pending, enabled interrupts at this level into a2. */

Expand All @@ -143,7 +166,7 @@
*/

/* Argument 1: Set of CPU interrupt to dispatch */
mov a3, sp /* Argument 2: Top of stack = register save area */
mov a3, a12 /* Argument 2: Top of stack = register save area */
call0 xtensa_int_decode /* Call xtensa_int_decode */

/* On return from xtensa_int_decode, a2 will contain the address of the new
Expand All @@ -169,7 +192,7 @@
*/

/* Argument 1: Set of CPU interrupt to dispatch */
mov a7, sp /* Argument 2: Top of stack = register save area */
mov a7, a12 /* Argument 2: Top of stack = register save area */
call4 xtensa_int_decode /* Call xtensa_int_decode */

/* On return from xtensa_int_decode, a6 will contain the address of the new
Expand Down Expand Up @@ -263,6 +286,19 @@ _xtensa_level1_handler:
mov a2, sp /* Address of state save on stack */
call0 _xtensa_context_save /* Save full register state */

/* Save current SP before (possibly) overwriting it, it's the register save
* area. This value will be used later by dispatch_c_isr to retrive the
* register save area.
*/

mov a12, sp

/* Switch to an interrupt stack if we have one */

#if CONFIG_ARCH_INTERRUPTSTACK > 15
setintstack a13 a14
#endif

Copy link
Contributor

@masayuki2009 masayuki2009 Oct 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to adjust (i.e. subtract 16?) the stack pointer (a1?) to avoid data corruption, because g_cpu_intstack_top does not include offset now.

Copy link
Member Author

@Ouss4 Ouss4 Oct 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll run more tests with the adjustment this evening and verify the memories. Thanks for the reminder.
(Yes, a1 is sp)

Copy link
Member Author

@Ouss4 Ouss4 Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@masayuki2009 I don't think that's necessary, same as the discussion we were having with ARM, the SP is decremented on procedures entry.

Copy link
Contributor

@masayuki2009 masayuki2009 Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ouss4
Sorry for confusing you.

Copy link
Member Author

@Ouss4 Ouss4 Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, there was no confusion :)
I did look here and there to see how others are doing the same thing and I found it similar to what's proposed by this PR.

/* Set up PS for C, enable interrupts above this level and clear EXCM. */

ps_setup 1 a0
Expand Down
58 changes: 58 additions & 0 deletions arch/xtensa/src/esp32/chip_macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@

#define HANDLER_SECTION .iram1

/****************************************************************************
* Public Data
****************************************************************************/

#ifdef __ASSEMBLY__

#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 15
.global g_cpu_intstack_top
#endif /* CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 15 */

#endif /* __ASSEMBLY__ */

/****************************************************************************
* Assembly Language Macros
****************************************************************************/
Expand All @@ -71,5 +83,51 @@
2:
.endm

/****************************************************************************
* Name: setintstack
*
* Description:
* Set the current stack pointer to the "top" of the correct interrupt
* stack for the current CPU.
*
****************************************************************************/

#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 15
.macro setintstack tmp1 tmp2
getcoreid \tmp1 /* tmp1 = Core ID (0 or 1) */
movi \tmp2, g_cpu_intstack_top /* tmp2 = Array of stack pointers */
addx4 \tmp2, \tmp1, \tmp2 /* tmp2 = tmp2 + (tmp1 << 2) */
l32i a1, \tmp2, 0 /* a1 = *tmp2 */
.endm
#endif
#endif /* __ASSEMBLY */

/****************************************************************************
* Public Data
****************************************************************************/

#ifndef __ASSEMBLY__
#if defined(__cplusplus)
#define EXTERN extern "C"
extern "C"
{
#else
#define EXTERN extern
#endif

/****************************************************************************
* Public Functions Prototypes
****************************************************************************/

#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 15
uintptr_t xtensa_intstack_base(void);
uintptr_t xtensa_intstack_alloc(void);
#endif

#undef EXTERN
#if defined(__cplusplus)
}
#endif

#endif /* __ASSEMBLY__ */
#endif /* __ARCH_XTENSA_SRC_ESP32_CHIP_MACROS_H */
Loading