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

Fix of red screen #1676

Merged
merged 7 commits into from
Jun 23, 2021
Merged

Fix of red screen #1676

merged 7 commits into from
Jun 23, 2021

Conversation

onvej-sl
Copy link
Contributor

This PR fixes #1658.

From my point of view, the cause of the issue is that shutdown() is called from the unprivileged mode (see this comment). To solve that, I implemented a SVC handler that calls shutdown() from the privileged mode.

@onvej-sl onvej-sl requested a review from hiviah June 22, 2021 10:58
@onvej-sl onvej-sl requested a review from prusnak as a code owner June 22, 2021 10:58
Copy link
Member

@prusnak prusnak left a comment

Choose a reason for hiding this comment

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

PR looks good, left one suggestion to consider

Comment on lines +37 to +44
void shutdown(void) {
#ifdef USE_SVC_SHUTDOWN
svc_shutdown();
#else
// It won't work properly unless called from the privileged mode
shutdown_privileged();
#endif
}
Copy link
Member

@prusnak prusnak Jun 22, 2021

Choose a reason for hiding this comment

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

Maybe we can use define instead of a function here.

But maybe that's over-optimizing stuff and compiler does this anyway ...

Suggested change
void shutdown(void) {
#ifdef USE_SVC_SHUTDOWN
svc_shutdown();
#else
// It won't work properly unless called from the privileged mode
shutdown_privileged();
#endif
}
#ifdef USE_SVC_SHUTDOWN
#define shutdown svc_shutdown
#else
// It won't work properly unless called from the privileged mode
#define shutdown shutdown_privileged
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But maybe that's over-optimizing stuff and compiler does this anyway ...

Yes, compiler inlines it. I'd leave it as it is.

@onvej-sl onvej-sl force-pushed the onvej-sl/red_screen_fix branch from f1c6cdc to 2c61e65 Compare June 23, 2021 14:39
@onvej-sl onvej-sl merged commit b8b0ae0 into master Jun 23, 2021
@onvej-sl onvej-sl deleted the onvej-sl/red_screen_fix branch June 23, 2021 14:40
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.

Successful wipe code triggers RSOD
3 participants