Skip to content

xtensa: gen_zsr: select software interrupt at <= EXCM_LEVEL#87483

Closed
williamtcdns wants to merge 1 commit intozephyrproject-rtos:mainfrom
williamtcdns:xtensa_fix_sw_irq
Closed

xtensa: gen_zsr: select software interrupt at <= EXCM_LEVEL#87483
williamtcdns wants to merge 1 commit intozephyrproject-rtos:mainfrom
williamtcdns:xtensa_fix_sw_irq

Conversation

@williamtcdns
Copy link
Contributor

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;
}

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
Copy link
Contributor Author

Feedback on this PR ?

@williamtcdns
Copy link
Contributor Author

@andyross @ceolin @nashif just one more review needed.

Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

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

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.

@github-actions
Copy link

github-actions bot commented Jun 6, 2025

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jun 6, 2025
@github-actions github-actions bot closed this Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Xtensa Xtensa Architecture Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments