Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
56 changes: 56 additions & 0 deletions cpu/cortexm_common/include/thread_arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
extern "C" {
#endif

#define THREAD_API_INLINED

Check failure on line 25 in cpu/cortexm_common/include/thread_arch.h

View workflow job for this annotation

GitHub Actions / static-tests

Member THREAD_API_INLINED (macro definition) of file thread_arch.h is not documented.

#ifndef DOXYGEN /* Doxygen is in core/include/thread.h */

Expand All @@ -38,6 +38,62 @@

#endif /* DOXYGEN */

/**
* @brief All svc dispatch handlers have this type.
*/
typedef int (*svc_dispatch_handler_t)(
unsigned int svc_number, unsigned int *svc_args);

#ifdef NDEBUG

# define assert_svc_dispatch_manage_handler() do {} while (0)

#else

/**
* @brief Last file that asserted svc dispatch handler was free.
*/
extern const char *_free_svc_dispatch_handler_last_file;

/**
* @brief Asserts that SVC dispatch handler is free.
*
* @param file Calling file.
* @param line Line in calling file.
*
* @see assert_free_svc_dispatch_handler
*/
void assert_free_svc_dispatch_handler_ex(const char *file, int line);

/**
* @brief Asserts that SVC dispatch handler is free.
*
* To be used in files using set_memory_manage_handler.
*
* @see set_svc_dispatch_handler
* @see assert_free_svc_dispatch_handler_ex
*/
#define assert_free_svc_dispatch_handler() \
assert_free_svc_dispatch_handler_ex(__FILE__, __LINE__); \
_free_svc_dispatch_handler_last_file = __FILE__

#endif /* NDEBUG */

/**
* @brief SVC dispatch handler setter.
*
* @param handler Handler to set
*
* @retval < 0 on NULL handler or if a handler has already been set
* @retval >= 0 otherwise
*/
int set_svc_dispatch_handler(svc_dispatch_handler_t handler);
Copy link
Member

Choose a reason for hiding this comment

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

Just to be sure I'm not overlooking something:

Why does this need to be set dynamically at run time, when there is only a single callback slot for it anyway?

Why not just provide a weak no-op default handler and register at link time by providing a handler with a strong symbol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maribu That's an interesting question.

In the context of xipfs, we are mostly dealing with short lifecycle execution.
The rationale for the dynamic setting is to scale among usages; other software could use this mechanism without collisions.

Now the weak function is attractive, because of its simplicity, but only one actor could use it, here xipfs.

While we are at it, do you prefer to handle this modification in a separate PR too ?


/**
* @brief SVC dispatch handler cleaner.
*/
void remove_svc_dispatch_handler(void);

#ifdef __cplusplus
}
#endif
Expand Down
56 changes: 56 additions & 0 deletions cpu/cortexm_common/include/vectors_cortexm.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
* @brief Use this macro to make interrupt functions overridable with the
* dummy_handler as fallback in case they are not implemented
*/
#define WEAK_DEFAULT __attribute__((weak,alias("dummy_handler")))

Check warning on line 28 in cpu/cortexm_common/include/vectors_cortexm.h

View workflow job for this annotation

GitHub Actions / static-tests

comma should be followed by whitespace

/**
* @brief Use this macro to define the parts of the vector table
Expand All @@ -34,7 +34,7 @@
* (numeric) value given for `x`. The Cortex-M base vectors are always defined
* with `ISR_VECTOR(0)`, so the CPU specific vector(s) **must** start from 1.
*/
#define ISR_VECTOR(x) __attribute__((used,section(".vectors." # x )))

Check warning on line 37 in cpu/cortexm_common/include/vectors_cortexm.h

View workflow job for this annotation

GitHub Actions / static-tests

comma should be followed by whitespace

/**
* @brief Number of Cortex-M non-ISR exceptions
Expand Down Expand Up @@ -90,6 +90,62 @@
/* The following four exceptions are only present for Cortex-M3 and -M4 CPUs */
#if defined(CPU_CORE_CORTEX_M3) || defined(CPU_CORE_CORTEX_M4) || \
defined(CPU_CORE_CORTEX_M4F) || defined(CPU_CORE_CORTEX_M7)

/**
* @brief All mem manage handlers have this type.
*/
typedef int (*mem_manage_handler_t)(void);

#ifdef NDEBUG

# define assert_free_mem_manage_handler() do {} while (0)

#else

/**
* @brief Last file that asserted memory manage handler was free.
*/
extern const char *_free_mem_manage_handler_last_file;

/**
* @brief Asserts that memory manage handler is free.
*
* @param file Calling file.
* @param line Line in calling file.
*
* @see assert_free_mem_manage_handler
*/
void assert_free_mem_manage_handler_ex(const char *file, int line);

/**
* @brief Asserts that memory manage handler is free.
*
* To be used in files using set_memory_manage_handler.
*
* @see set_memory_manage_handler
* @see assert_free_mem_manage_handler_ex
*/
#define assert_free_mem_manage_handler() \
assert_free_mem_manage_handler_ex(__FILE__, __LINE__); \
_free_mem_manage_handler_last_file = __FILE__

#endif /* NDEBUG */

/**
* @brief Memory manage handler setter.
*
* @param handler Handler to set
*
* @retval < 0 on NULL handler or if a handler has already been set
* @retval >= 0 otherwise
*/
int set_memory_manage_handler(mem_manage_handler_t handler);

/**
* @brief Memory manage handler cleaner.
*/
void remove_memory_manage_handler(void);

/**
* @brief Memory management exception handler
*
Expand Down
46 changes: 46 additions & 0 deletions cpu/cortexm_common/thread_arch.c
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,11 @@ void *thread_isr_stack_start(void)
return (void *)&_sstack;
}

void *thread_isr_stack_end(void)
{
return (void *)&_estack;
}

void NORETURN cpu_switch_context_exit(void)
{
#ifdef MODULE_CORTEXM_FPU
Expand Down Expand Up @@ -488,6 +493,42 @@ void __attribute__((naked)) __attribute__((used)) isr_svc(void)
#endif
}

static svc_dispatch_handler_t _svc_dispatch_handler = NULL;

#ifndef NDEBUG
const char *_free_svc_dispatch_handler_last_file = NULL;

void assert_free_svc_dispatch_handler_ex(const char *file, int line) {
if (_svc_dispatch_handler != NULL) {
printf( "SVC dispatch handler is not free : assertion from "
"file %s at line %d, previously from file %s\n",
file, line,
_free_svc_dispatch_handler_last_file);

for (;;) {}
}
}

#endif

int set_svc_dispatch_handler(svc_dispatch_handler_t handler) {
if (handler == NULL) {
return -1;
}
if (_svc_dispatch_handler != NULL) {
return -1;
}
_svc_dispatch_handler = handler;
return 0;
}

void remove_svc_dispatch_handler(void) {
_svc_dispatch_handler = NULL;
#ifndef NDEBUG
_free_svc_dispatch_handler_last_file = NULL;
#endif
}

static void __attribute__((used)) _svc_dispatch(unsigned int *svc_args)
{
/* stack frame:
Expand All @@ -514,6 +555,11 @@ static void __attribute__((used)) _svc_dispatch(unsigned int *svc_args)
SCB->ICSR = SCB_ICSR_PENDSVSET_Msk;
break;
default:
if (_svc_dispatch_handler != NULL) {
if (_svc_dispatch_handler(svc_number, svc_args) >= 0) {
return;
}
}
DEBUG("svc: unhandled SVC #%u\n", svc_number);
break;
}
Expand Down
41 changes: 41 additions & 0 deletions cpu/cortexm_common/vectors_cortexm.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
/**
* @brief Allocation of the interrupt stack
*/
__attribute__((used,section(".isr_stack"))) uint8_t isr_stack[ISR_STACKSIZE];

Check warning on line 78 in cpu/cortexm_common/vectors_cortexm.c

View workflow job for this annotation

GitHub Actions / static-tests

comma should be followed by whitespace

/**
* @brief Pre-start routine for CPU-specific settings
Expand Down Expand Up @@ -472,8 +472,49 @@
#if defined(CPU_CORE_CORTEX_M3) || defined(CPU_CORE_CORTEX_M33) || \
defined(CPU_CORE_CORTEX_M4) || defined(CPU_CORE_CORTEX_M4F) || \
defined(CPU_CORE_CORTEX_M7)

static mem_manage_handler_t _mem_manage_handler = NULL;

#ifndef NDEBUG
const char *_free_mem_manage_handler_last_file = NULL;

void assert_free_mem_manage_handler_ex(const char *file, int line) {
if (_mem_manage_handler != NULL) {
printf( "Memory manage handler is not free : assertion from "
"file %s at line %d, previously from file %s\n",
file, line,
_free_mem_manage_handler_last_file);

for (;;) {}
}
}

#endif

int set_memory_manage_handler(mem_manage_handler_t handler) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to set this at run-time or would it be sufficient if this was a weak symbol that gets replaced at link-time?

Copy link
Member

Choose a reason for hiding this comment

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

Hah, I was faster! Quick-draw batch for me 🤣

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crasbe @maribu @benpicco
To sum it up :

  • thread_arch.c
__attribute__((weak)) int svc_dispatch_handler(unsigned int svc_number, unsigned int *svc_args) {
  return -ENOTSUP;
}
...
   switch (svc_number) {
        case 1: /* SVC number used by cpu_switch_context_exit */
            SCB->ICSR = SCB_ICSR_PENDSVSET_Msk;
            break;
        default:
             if (_svc_dispatch_handler(svc_number, svc_args) >= 0) {
                 return;
             }
            DEBUG("svc: unhandled SVC #%u\n", svc_number);
            break;
    }
  • vector_cortexm.c:
__attribute__((weak)) int mem_manage_handler(void) {
  return 0;
}

void mem_manage_default(void)
{
    if (_mem_manage_handler() == 1) {
         return;
    }
    core_panic(PANIC_MEM_MANAGE, "MEM MANAGE HANDLER");
}

The all above in a separate PR to keep things clean ?
What would be the PR title then ?

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't insist for separating that out, this PR is not too large anyway. IMO: Whatever works best for you.

if (handler == NULL) {
return -1;
}
if (_mem_manage_handler != NULL) {
return -1;
}
_mem_manage_handler = handler;
return 0;
}

void remove_memory_manage_handler(void) {
_mem_manage_handler = NULL;
#ifndef NDEBUG
_free_mem_manage_handler_last_file = NULL;
#endif
}

void mem_manage_default(void)
{
if (_mem_manage_handler != NULL) {
if (_mem_manage_handler() == 1)
return;

Check warning on line 516 in cpu/cortexm_common/vectors_cortexm.c

View workflow job for this annotation

GitHub Actions / static-tests

full block {} expected in the control structure
}
core_panic(PANIC_MEM_MANAGE, "MEM MANAGE HANDLER");
}

Expand Down
9 changes: 9 additions & 0 deletions examples/advanced/xipfs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,23 @@ QUIET ?= 1
TOOLCHAINS_BLACKLIST += llvm

BLOBS += hello-world.fae
BLOBS += dumper.fae

# Modules to include:
USEMODULE += shell
USEMODULE += shell_cmds_default
USEMODULE += ps
USEMODULE += saul_default
USEMODULE += cortexm_svc

FEATURES_REQUIRED += cortexm_mpu

# Use xipfs file system
USEMODULE += xipfs

# XIPFS MPU support
CFLAGS += -DXIPFS_ENABLE_SAFE_EXEC_SUPPORT
# XIPFS MPU ISR stack requirement
CFLAGS += -DISR_STACKSIZE=1024

include $(RIOTBASE)/Makefile.include
25 changes: 3 additions & 22 deletions examples/advanced/xipfs/Makefile.ci
Original file line number Diff line number Diff line change
@@ -1,24 +1,5 @@
BOARD_INSUFFICIENT_MEMORY := \
blackpill-stm32f103c8 \
bluepill-stm32f030c8 \
bluepill-stm32f103c8 \
i-nucleo-lrwan1 \
nucleo-c031c6 \
nucleo-f030r8 \
nucleo-f031k6 \
nucleo-f042k6 \
nucleo-f302r8 \
nucleo-f303k8 \
nucleo-f334r8 \
nucleo-l011k4 \
nucleo-l031k6 \
nucleo-l053r8 \
samd10-xmini \
slstk3400a \
stk3200 \
stm32f030f4-demo \
stm32f0discovery \
stm32g0316-disco \
stm32l0538-disco \
weact-g030f6 \
e104-bt5010a-tb \
e104-bt5011a-tb \
im880b \
#
Binary file added examples/advanced/xipfs/dumper.fae
Binary file not shown.
Binary file modified examples/advanced/xipfs/hello-world.fae
Binary file not shown.
Loading
Loading