-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
[WIP] change the software interrupt to syscall instruction #14672
base: master
Are you sure you want to change the base?
Conversation
@tmedicci could you help review this pr? |
[Experimental Bot, please feedback here] This PR does not fully meet the NuttX requirements yet, though it's on its way. Here's a breakdown: Missing/Incomplete Information:
Recommendations:
By addressing these points, you'll significantly strengthen your PR and make it easier for reviewers to understand and approve the changes. |
I'm busy during these days, so I'll take a while to fully review it. For now, I'll run our internal pipeline and report its results. But I have question: IINM, (@Ouss4 and @gustavonihei may have some thoughts about it. If they have spare time, I'd like to hear about...). |
Hi @xiaoxiang781216 , can we mark it as a draft until we fully review this PR to avoid it being merged by mistake? |
The CI shows the same failed results regarding |
Hi @chirping78 , were you able to verify why |
@tmedicci |
With #14760 esp32s3-devkit:smp ostest is more stable, but still not 100% pass. |
There were some changes recently that affect |
Signed-off-by: chenxiaoyi <[email protected]>
Signed-off-by: chenxiaoyi <[email protected]>
Signed-off-by: chenxiaoyi <[email protected]>
0fe7476
to
c73f8bc
Compare
/* Enable the software interrupt. */ | ||
|
||
up_enable_irq(XTENSA_IRQ_SWINT); | ||
irq_attach(XTENSA_IRQ_SYSCALL, (xcpt_t)xtensa_swint, NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we remove (xcp_t)?
Summary
Currently xtensa arch porting is using software interrupt (through write
intset
register) to trigger context switch.But this put a strict requirement to the chip configuration:
there must be a software interrupt, it's priority is less or equal to XCHAL_EXCM_LEVEL;
and it's priority must be greater than XCHAL_SYSCALL_LEVEL (the XCHAL_SYSCALL_LEVEL is not a cadence macro, it just means normal hardware interrupt priority).
Since the xtensa arch is very flexible, the chip designer might have not noticed above requirement in chip design phase.
And if the chip has been taped out without this requirement, there is no posibility to port nuttx to the chip.
This PR will try to change the software interrupt to
syscall
instruction.The
syscall
instruction is a synchronous exception, and is not locked byPS.intlevel
, which means it's priority is high enough.Impact
This PR will impact context switch, interrupt handling, and protect build.
Testing
test summary (Will be continuously updated when the issues are fixed)
11/18: rebased, better than last time.
rebase to version: nuttx master 637a0f5, apps master fee82bd3d,
plus this PR.
esp32s3-devkit:nsh ostest pass
esp32s3-devkit:tickless ostest pass
esp32s3-devkit:smp ostest pass with percentage ~20%
The possibility of passing the test is higher if power off then on through plug and unplug usb.
esp32s3-devkit:knsh ostest pass with percentage ~40%
11/13:
With #14760 esp32s3-devkit:smp ostest is more stable, but still not 100% pass.
11/05: has regression problem
base version: nuttx master 56f292f, apps master 400a8e326,
plus this PR.
esp32s3-devkit:nsh (+CONFIG_TESTING_OSTEST)
ostest pass
esp32s3-devkit:tickless
ostest pass
esp32s3-devkit:smp
ostest failed (only "nested signal handler test" failed)
esp32s3-devkit:knsh
ostest failed (fail at random position with message "xtensa_panic: Unhandled Exception 2", 2 means double exception)
Since there is no esp32 and esp32s2 boards in hand (have placed order), so these boards have not been tested.