From e3cf967fa0ed7fc7637efb8983e41c965daa9bda Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 30 Nov 2022 11:11:44 -0800 Subject: [PATCH 1/7] feat(kernel): dump commands for GDT and timer (#375) --- src/arch/x86_64.rs | 1 + src/arch/x86_64/interrupt.rs | 2 +- src/arch/x86_64/shell.rs | 25 +++++++++++++++++++++++++ src/shell.rs | 7 +++++++ 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/arch/x86_64/shell.rs diff --git a/src/arch/x86_64.rs b/src/arch/x86_64.rs index ff5e424a..2be21014 100644 --- a/src/arch/x86_64.rs +++ b/src/arch/x86_64.rs @@ -14,6 +14,7 @@ mod framebuf; pub mod interrupt; mod oops; pub mod pci; +pub mod shell; pub use self::{ boot::ArchInfo, oops::{oops, Oops}, diff --git a/src/arch/x86_64/interrupt.rs b/src/arch/x86_64/interrupt.rs index 0bb4084e..71d69e46 100644 --- a/src/arch/x86_64/interrupt.rs +++ b/src/arch/x86_64/interrupt.rs @@ -54,7 +54,7 @@ static TSS: sync::Lazy = sync::Lazy::new(|| { tss }); -static GDT: sync::InitOnce = sync::InitOnce::uninitialized(); +pub(in crate::arch) static GDT: sync::InitOnce = sync::InitOnce::uninitialized(); const TIMER_INTERVAL: time::Duration = time::Duration::from_millis(10); pub(super) static TIMER: time::Timer = time::Timer::new(TIMER_INTERVAL); diff --git a/src/arch/x86_64/shell.rs b/src/arch/x86_64/shell.rs new file mode 100644 index 00000000..aea6dcf2 --- /dev/null +++ b/src/arch/x86_64/shell.rs @@ -0,0 +1,25 @@ +use crate::shell::{self, Command}; + +pub const DUMP_ARCH: Command = Command::new("arch") + .with_help("dump architecture-specific structures") + .with_subcommands(&[ + Command::new("gdt") + .with_help("print the global descriptor table (GDT)") + .with_fn(|_| { + let gdt = super::interrupt::GDT.get(); + tracing::info!(?gdt); + Ok(()) + }), + // Command::new("idt") + // .with_help("print the interrupt descriptor table (IDT)") + // .with_fn(|_| { + // let gdt = super::interrupt::GDT.get(); + // tracing::info!(?gdt); + // Ok(()) + // }), + ]); + +pub fn dump_timer(_: &str) -> Result<(), shell::Error> { + tracing::info!(timer = ?super::interrupt::TIMER); + Ok(()) +} diff --git a/src/shell.rs b/src/shell.rs index 85f69916..4d223f4a 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -40,6 +40,13 @@ pub fn eval(line: &str) { Command::new("archinfo") .with_help("print the architecture information structure") .with_fn(|line| Err(Error::other(line, "not yet implemented"))), + Command::new("timer") + .with_help("print the timer wheel") + .with_fn(crate::arch::shell::dump_timer), + Command::new("timer") + .with_help("print the timer wheel") + .with_fn(crate::arch::shell::dump_timer), + crate::arch::shell::DUMP_ARCH, Command::new("heap") .with_help("print kernel heap statistics") .with_fn(|_| { From c292ae8a2eed7044fbc77e7464135a7f553763ec Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Dec 2022 09:40:19 -0800 Subject: [PATCH 2/7] fix(hal-x86_64): nicer `Debug` output for `Idt` (#375) This improves the `Idt` type's `fmt::Debug` output, including fixing a potential panic when trying to format a null descriptor. Since the IDT is sparse and many descriptors may be null, only the present descriptors are printed, along with their indices. --- hal-x86_64/src/interrupt/idt.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/hal-x86_64/src/interrupt/idt.rs b/hal-x86_64/src/interrupt/idt.rs index f63e57c5..0ca0454b 100644 --- a/hal-x86_64/src/interrupt/idt.rs +++ b/hal-x86_64/src/interrupt/idt.rs @@ -1,7 +1,9 @@ use super::apic::IoApic; -use crate::{cpu, segment}; -use core::fmt; -use mycelium_util::bits; +use crate::{ + cpu::{self, DescriptorTable}, + segment, +}; +use mycelium_util::{bits, fmt}; #[repr(C)] #[repr(align(16))] @@ -205,7 +207,12 @@ impl Idt { impl fmt::Debug for Idt { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { let Self { descriptors } = self; - f.debug_list().entries(descriptors[..].iter()).finish() + let descriptors = descriptors[..] + .iter() + .filter(|&descr| descr != &Descriptor::null()) + .enumerate() + .map(|(i, descr)| (fmt::hex(i), descr)); + f.debug_map().entries(descriptors).finish() } } From fba6da89337d508618d81bdafc96736f4422b278 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Dec 2022 09:46:34 -0800 Subject: [PATCH 3/7] feat(bitfield): skip `_` fields in `fmt::Debug` (#375) Currently, the derived `fmt::Debug` impl for bitfields will print the value of reserved fields (those whose names start with `_`). This is rarely useful, and adds noise to the formatted output. This branch updates the `fmt::Debug` impl to skip formatting those fields. --- bitfield/src/bitfield.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/bitfield/src/bitfield.rs b/bitfield/src/bitfield.rs index a895642b..6012068e 100644 --- a/bitfield/src/bitfield.rs +++ b/bitfield/src/bitfield.rs @@ -290,7 +290,16 @@ macro_rules! bitfield { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { let mut dbg = f.debug_struct(stringify!($Name)); $( - dbg.field(stringify!($Field), &self.get(Self::$Field)); + { + // skip reserved fields (names starting with `_`). + // + // NOTE(eliza): i hope this `if` gets const-folded...we + // could probably do this in a macro and guarantee that + // it happens at compile-time, but this is fine for now. + if !stringify!($Field).starts_with('_') { + dbg.field(stringify!($Field), &self.get(Self::$Field)); + } + } )+ dbg.finish() From fd436a1e7abd9d573558648f90cb784e722aa931 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Dec 2022 09:46:56 -0800 Subject: [PATCH 4/7] feat(x86_64): add `dump arch idt` shell command (#375) --- hal-x86_64/src/interrupt.rs | 4 ++++ src/arch/x86_64/shell.rs | 16 ++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/hal-x86_64/src/interrupt.rs b/hal-x86_64/src/interrupt.rs index b46f13eb..85f885a0 100644 --- a/hal-x86_64/src/interrupt.rs +++ b/hal-x86_64/src/interrupt.rs @@ -100,6 +100,10 @@ static INTERRUPT_CONTROLLER: InitOnce = InitOnce::uninitialized(); impl Controller { // const DEFAULT_IOAPIC_BASE_PADDR: u64 = 0xFEC00000; + pub fn idt() -> spin::MutexGuard<'static, idt::Idt> { + IDT.lock() + } + #[tracing::instrument(level = "info", name = "interrupt::Controller::init")] pub fn init>() { tracing::info!("intializing IDT..."); diff --git a/src/arch/x86_64/shell.rs b/src/arch/x86_64/shell.rs index aea6dcf2..1d488d6e 100644 --- a/src/arch/x86_64/shell.rs +++ b/src/arch/x86_64/shell.rs @@ -7,16 +7,16 @@ pub const DUMP_ARCH: Command = Command::new("arch") .with_help("print the global descriptor table (GDT)") .with_fn(|_| { let gdt = super::interrupt::GDT.get(); - tracing::info!(?gdt); + tracing::info!(GDT = ?gdt); + Ok(()) + }), + Command::new("idt") + .with_help("print the interrupt descriptor table (IDT)") + .with_fn(|_| { + let idt = super::interrupt::Controller::idt(); + tracing::info!(IDT = ?idt); Ok(()) }), - // Command::new("idt") - // .with_help("print the interrupt descriptor table (IDT)") - // .with_fn(|_| { - // let gdt = super::interrupt::GDT.get(); - // tracing::info!(?gdt); - // Ok(()) - // }), ]); pub fn dump_timer(_: &str) -> Result<(), shell::Error> { From 47d64103d0b3b50821c5ccdbd722ffb8f1fd8bce Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Dec 2022 09:55:44 -0800 Subject: [PATCH 5/7] refac: move timer static from `arch` to `rt` (#375) --- src/arch/x86_64.rs | 4 ---- src/arch/x86_64/interrupt.rs | 7 ++----- src/arch/x86_64/shell.rs | 5 ----- src/lib.rs | 3 +++ src/rt.rs | 16 ++++++++++++++-- src/shell.rs | 8 ++++---- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/arch/x86_64.rs b/src/arch/x86_64.rs index 2be21014..0da26af7 100644 --- a/src/arch/x86_64.rs +++ b/src/arch/x86_64.rs @@ -25,10 +25,6 @@ mod tests; pub type MinPageSize = mm::size::Size4Kb; -pub fn tick_timer() { - interrupt::TIMER.advance_ticks(0); -} - #[cfg(target_os = "none")] bootloader::entry_point!(arch_entry); diff --git a/src/arch/x86_64/interrupt.rs b/src/arch/x86_64/interrupt.rs index 71d69e46..9fdf0eb6 100644 --- a/src/arch/x86_64/interrupt.rs +++ b/src/arch/x86_64/interrupt.rs @@ -25,8 +25,6 @@ pub fn enable_hardware_interrupts(acpi: Option<&acpi::InterruptModel>) { controller .start_periodic_timer(TIMER_INTERVAL) .expect("10ms should be a reasonable interval for the PIT or local APIC timer..."); - time::set_global_timer(&TIMER) - .expect("`enable_hardware_interrupts` should only be called once!"); tracing::info!(granularity = ?TIMER_INTERVAL, "global timer initialized") } @@ -56,8 +54,7 @@ static TSS: sync::Lazy = sync::Lazy::new(|| { pub(in crate::arch) static GDT: sync::InitOnce = sync::InitOnce::uninitialized(); -const TIMER_INTERVAL: time::Duration = time::Duration::from_millis(10); -pub(super) static TIMER: time::Timer = time::Timer::new(TIMER_INTERVAL); +pub const TIMER_INTERVAL: time::Duration = time::Duration::from_millis(10); static TEST_INTERRUPT_WAS_FIRED: AtomicUsize = AtomicUsize::new(0); @@ -96,7 +93,7 @@ impl hal_core::interrupt::Handlers for InterruptHandlers { } fn timer_tick() { - TIMER.pend_ticks(1); + crate::rt::TIMER.pend_ticks(1); } fn ps2_keyboard(scancode: u8) { diff --git a/src/arch/x86_64/shell.rs b/src/arch/x86_64/shell.rs index 1d488d6e..6bc31d76 100644 --- a/src/arch/x86_64/shell.rs +++ b/src/arch/x86_64/shell.rs @@ -18,8 +18,3 @@ pub const DUMP_ARCH: Command = Command::new("arch") Ok(()) }), ]); - -pub fn dump_timer(_: &str) -> Result<(), shell::Error> { - tracing::info!(timer = ?super::interrupt::TIMER); - Ok(()) -} diff --git a/src/lib.rs b/src/lib.rs index afd14063..a093000e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -147,6 +147,9 @@ pub fn kernel_start(bootinfo: impl BootInfo, archinfo: crate::arch::ArchInfo) -> // tracing. arch::init(&bootinfo, &archinfo); + // initialize the kernel runtime. + rt::init(); + #[cfg(test)] arch::run_tests(); diff --git a/src/rt.rs b/src/rt.rs index 793ee76a..7ad402fd 100644 --- a/src/rt.rs +++ b/src/rt.rs @@ -5,7 +5,10 @@ use core::{ future::Future, sync::atomic::{AtomicBool, AtomicUsize, Ordering::*}, }; -use maitake::scheduler::{self, StaticScheduler, Stealer}; +use maitake::{ + scheduler::{self, StaticScheduler, Stealer}, + time, +}; use mycelium_util::sync::InitOnce; use rand::Rng; @@ -49,6 +52,8 @@ struct Runtime { /// 512 CPU cores ought to be enough for anybody... pub const MAX_CORES: usize = 512; +pub static TIMER: time::Timer = time::Timer::new(arch::interrupt::TIMER_INTERVAL); + static RUNTIME: Runtime = { // This constant is used as an array initializer; the clippy warning that it // contains interior mutability is not actually a problem here, since we @@ -83,6 +88,13 @@ where }) } +/// Initialize the kernel runtime. +pub fn init() { + time::set_global_timer(&TIMER).expect("`rt::init` should only be called once!"); + + tracing::info!("kernel runtime initialized"); +} + static SCHEDULER: arch::LocalKey>> = arch::LocalKey::new(|| Cell::new(None)); @@ -108,7 +120,7 @@ impl Core { // turn the timer wheel if it wasn't turned recently and no one else is // holding a lock, ensuring any pending timer ticks are consumed. - arch::tick_timer(); + TIMER.advance_ticks(0); // if there are remaining tasks to poll, continue without stealing. if tick.has_remaining { diff --git a/src/shell.rs b/src/shell.rs index 4d223f4a..968695d0 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -42,10 +42,10 @@ pub fn eval(line: &str) { .with_fn(|line| Err(Error::other(line, "not yet implemented"))), Command::new("timer") .with_help("print the timer wheel") - .with_fn(crate::arch::shell::dump_timer), - Command::new("timer") - .with_help("print the timer wheel") - .with_fn(crate::arch::shell::dump_timer), + .with_fn(|_| { + tracing::info!(timer = ?crate::rt::TIMER); + Ok(()) + }), crate::arch::shell::DUMP_ARCH, Command::new("heap") .with_help("print kernel heap statistics") From 84b47eb7d7dc23c5a74e385c09e324d2e5508871 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Dec 2022 10:05:13 -0800 Subject: [PATCH 6/7] feat(util): less noisy `Lazy`/`InitOnce` `Debug` (#375) This branch simplifies the `Lazy` and `InitOnce` types' `fmt::Debug` impls to behave transparently if the value is initialized. It's not actually that useful to indicate that the value is in a lazy cell once it has been initialized, since that can be determined from reading the code. --- util/src/sync/once.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/util/src/sync/once.rs b/util/src/sync/once.rs index c5eca279..a9062cdd 100644 --- a/util/src/sync/once.rs +++ b/util/src/sync/once.rs @@ -205,12 +205,10 @@ impl InitOnce { impl fmt::Debug for InitOnce { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut d = f.debug_struct("InitOnce"); - d.field("type", &any::type_name::()); match self.state.load(Ordering::Acquire) { - INITIALIZED => d.field("value", self.get()).finish(), - INITIALIZING => d.field("value", &format_args!("")).finish(), - UNINITIALIZED => d.field("value", &format_args!("")).finish(), + INITIALIZED => self.get().fmt(f), + INITIALIZING => f.pad(""), + UNINITIALIZED => f.pad(""), _state => unsafe { unreachable_unchecked!("unexpected state value {}, this is a bug!", _state) }, @@ -358,13 +356,13 @@ where T: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut d = f.debug_struct("Lazy"); - d.field("type", &any::type_name::()) - .field("initializer", &format_args!("...")); match self.state.load(Ordering::Acquire) { - INITIALIZED => d.field("value", self.get_if_present().unwrap()).finish(), - INITIALIZING => d.field("value", &format_args!("")).finish(), - UNINITIALIZED => d.field("value", &format_args!("")).finish(), + INITIALIZED => self + .get_if_present() + .expect("if state is `INITIALIZED`, value should be present") + .fmt(f), + INITIALIZING => f.pad(""), + UNINITIALIZED => f.pad(""), _state => unsafe { unreachable_unchecked!("unexpected state value {}, this is a bug!", _state) }, From 00ecf3170e682702046b9726edc87a4c21fdf3d4 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Dec 2022 10:06:08 -0800 Subject: [PATCH 7/7] feat(kernel): add `dump rt` command (#375) This adds a shell command that prints the state of the kernel's async runtime. --- src/rt.rs | 20 +++++++++++++++++++- src/shell.rs | 1 + 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/rt.rs b/src/rt.rs index 7ad402fd..733f7f88 100644 --- a/src/rt.rs +++ b/src/rt.rs @@ -9,7 +9,7 @@ use maitake::{ scheduler::{self, StaticScheduler, Stealer}, time, }; -use mycelium_util::sync::InitOnce; +use mycelium_util::{fmt, sync::InitOnce}; use rand::Rng; pub use maitake::task::JoinHandle; @@ -95,6 +95,13 @@ pub fn init() { tracing::info!("kernel runtime initialized"); } +pub const DUMP_RT: crate::shell::Command = crate::shell::Command::new("rt") + .with_help("print the kernel's async runtime") + .with_fn(|_| { + tracing::info!(runtime = ?RUNTIME); + Ok(()) + }); + static SCHEDULER: arch::LocalKey>> = arch::LocalKey::new(|| Cell::new(None)); @@ -281,3 +288,14 @@ impl Runtime { self.cores[idx].try_get()?.try_steal().ok() } } + +impl fmt::Debug for Runtime { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let cores = self.active_cores(); + f.debug_struct("Runtime") + .field("active_cores", &cores) + .field("cores", &&self.cores[..cores]) + .field("injector", &self.injector) + .finish() + } +} diff --git a/src/shell.rs b/src/shell.rs index 968695d0..700eea1a 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -46,6 +46,7 @@ pub fn eval(line: &str) { tracing::info!(timer = ?crate::rt::TIMER); Ok(()) }), + crate::rt::DUMP_RT, crate::arch::shell::DUMP_ARCH, Command::new("heap") .with_help("print kernel heap statistics")