xtensa: gen_zsr: select software interrupt at <= EXCM_LEVEL #87483
Conversation
williamtcdns
force-pushed
the
xtensa_fix_sw_irq
branch
from
f437813 to
c7a68ec
Compare
The current logic selects a software interrupt even if it is at > EXCM_LEVEL. However the arch_irq_lock() function only locks out interrupts upto EXCM_LEVEL. In include/zephyr/arch/xtensa/irq.h - /** Implementation of @ref arch_irq_lock. */ static ALWAYS_INLINE unsigned int arch_irq_lock(void) { unsigned int key; __asm__ volatile("rsil %0, %1" : "=r"(key) : "i"(XCHAL_EXCM_LEVEL) : "memory"); return key; } Signed-off-by: William Tambe <williamt@cadence.com>
williamtcdns
force-pushed
the
xtensa_fix_sw_irq
branch
from
c7a68ec to
61a4b6d
Compare
|
Feedback on this PR ? |
There was a problem hiding this comment.
This sort of seems like an angels-on-a-pinhead argument? It's a software interrupt, it's triggered synchronously under control of the code being "interrupted". Is it really wrong that it works when interrupts are masked?
The purpose of this feature is to find a software interrupt we can use for testing purposes, e.g. for nested interrupts. And the test code doesn't mask interrupts around it currently, since it obviously works. So the effect of this patch is to disallow that on some number (maybe zero?) of hardware platforms, and therefore to reduce test coverage on those platforms.
Seems like a bad trade to me. Tentative -1, but I'm willing to be convinced if there's a bug somewhere that needs this as a fix.
The current logic selects a software interrupt even if it is at > EXCM_LEVEL.
However the arch_irq_lock() function only locks out interrupts upto EXCM_LEVEL.
In include/zephyr/arch/xtensa/irq.h -
/** Implementation of @ref arch_irq_lock. */
static ALWAYS_INLINE unsigned int arch_irq_lock(void) {
unsigned int key;
asm volatile("rsil %0, %1"
: "=r"(key) : "i"(XCHAL_EXCM_LEVEL) : "memory");
return key;
}