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

feat(hal_x86_64): be less stupid about ISR tracing #496

Merged
merged 1 commit into from
Dec 30, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 39 additions & 22 deletions hal-x86_64/src/interrupt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,11 +523,10 @@ impl hal_core::interrupt::Control for Idt {
code: u64,
) {
unsafe {
// Safety: who cares!
crate::vga::writer().force_unlock();
if let Some(com1) = crate::serial::com1() {
com1.force_unlock();
}
// Safety: An invalid TSS exception is always an oops. Since we're
// not coming back from this, it's okay to forcefully unlock the
// tracing outputs.
force_unlock_tracing();
}
let selector = SelectorErrorCode(code as u16);
tracing::error!(?selector, "invalid task-state segment!");
Expand All @@ -548,11 +547,10 @@ impl hal_core::interrupt::Control for Idt {
code: u64,
) {
unsafe {
// Safety: who cares!
crate::vga::writer().force_unlock();
if let Some(com1) = crate::serial::com1() {
com1.force_unlock();
}
// Safety: An segment not present exception is always an oops.
// Since we're not coming back from this, it's okay to
// forcefully unlock the tracing outputs.
force_unlock_tracing();
}
let selector = SelectorErrorCode(code as u16);
tracing::error!(?selector, "a segment was not present!");
Expand All @@ -573,11 +571,10 @@ impl hal_core::interrupt::Control for Idt {
code: u64,
) {
unsafe {
// Safety: who cares!
crate::vga::writer().force_unlock();
if let Some(com1) = crate::serial::com1() {
com1.force_unlock();
}
// Safety: An stack-segment fault exeption is always an oops.
// Since we're not coming back from this, it's okay to
// forcefully unlock the tracing outputs.
force_unlock_tracing();
}
let selector = SelectorErrorCode(code as u16);
tracing::error!(?selector, "a stack-segment fault is happening");
Expand All @@ -598,12 +595,17 @@ impl hal_core::interrupt::Control for Idt {
code: u64,
) {
unsafe {
// Safety: who cares!

crate::vga::writer().force_unlock();
if let Some(com1) = crate::serial::com1() {
com1.force_unlock();
}
// Safety: A general protection fault is (currently) always an
// oops. Since we're not coming back from this, it's okay to
// forcefully unlock the tracing outputs.
//
// TODO(eliza): in the future, if we allow the kernel to
// recover from general protection faults in user mode programs,
// rather than treating them as invariably fatal, we should
// probably not always do this. Instead, we should just handle
// the user-mode GPF non-fatally, and only unlock the tracing
// stuff if we know we're going to do a kernel oops...
force_unlock_tracing();
}

let segment = if code > 0 {
Expand All @@ -625,7 +627,7 @@ impl hal_core::interrupt::Control for Idt {
}

extern "x86-interrupt" fn spurious_isr() {
tracing::trace!("spurious");
// TODO(eliza): do we need to actually do something here?
}

// === exceptions ===
Expand Down Expand Up @@ -675,6 +677,21 @@ impl hal_core::interrupt::Control for Idt {
}
}

/// Forcefully unlock the VGA port and COM1 serial port (used by tracing), so
/// that an ISR can log stuff without deadlocking.
///
/// # Safety
///
/// This forcefully unlocks a mutex, which is probably bad to do. Only do this
/// in ISRs that definitely represent real actual faults, and not just because
/// "you wanted to log something".
unsafe fn force_unlock_tracing() {
crate::vga::writer().force_unlock();
if let Some(com1) = crate::serial::com1() {
com1.force_unlock();
}
}

impl fmt::Debug for Registers {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self {
Expand Down
Loading