Skip to content
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

Allow mpie bit to be writeable in mstatus #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

btashton
Copy link

This bit is currently not r/w but this means that it is not possible to change the mstatus mie state from within an exception handler. The mie bit is always set to its previous state when mret is called. This is an issue with an RTOS where ecall may be called from a new task creation which is in a critical section. This mean that interrupts including the system tick will not be triggered.

This functionality was tested with the code in this GitHub
gist https://gist.github.com/btashton/4aa7ed42bc81ff4716821636997d9df9

This bit is currently not r/w but this means that it is
not possible to change the mstatus mie state from within
an exception handler.  The mie bit is always set to its
previous state when mret is called.  This is an issue
with an RTOS where ecall may be called from a new task creation
which is in a critial section.  This mean that interrupts
including the system tick will not be triggered.

This functionality was tested with the code in this GitHub
gist https://gist.github.com/btashton/4aa7ed42bc81ff4716821636997d9df9

Signed-off-by: Brennan Ashton <bashton@brennanashton.com>
@btashton
Copy link
Author

When I targeted the icebreaker there did not appear to be any change in resource usage. The logic change is quite minor since the wire for the mpie bit is already used for the mie CSR so I was not too surprised. I left the read unimplemented, I think that would add a fair bit more logic, and I don't know that it is that useful.

@btashton
Copy link
Author

I am realizing that the OS does in fact expect to be able to read the MPIE bit correctly for exactly the case of returning back to critical section. It would be bad if a blocked task restored into the critical section with the MPIE set and interrupts were then restored. I have not looked at what is required to support the read path for this bit.

@btashton
Copy link
Author

btashton commented Jul 1, 2020

I have also noticed another issue with MPIE that is unexpected. Here is a trace where the interrupt handler is triggered because of the timer. We see a pulse on the trap, and for mstatus MIE goes low and MPIE goes high as expected. However at instruction 0xa4 csrr s0, mstatus MPIE is brought low. So not only can we not read out MPIE to restore it, but reading it will cause its state to change.
image

@olofk
Copy link
Owner

olofk commented Jul 3, 2020

I prefer your fix if we can do without reading, but if I understand your follow-up comments correctly, we do need to read it as well.

@btashton
Copy link
Author

btashton commented Jul 4, 2020

@olofk I don't know a way around the read issue since we need to store all the register state including mstatus to restore. I'll take a look later this next week, I'm bringing up support for the EOS S3 and I want to get that in "workable" state before switching back to this. I'll also look at Zephyr and see how they are handling the context save/restore. My guess is that there is a lingering issue there as well, but maybe ecall is not used for context switch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants