From 44ac26f7d0de3ff30e27969f69c52ddbe8382179 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 3 Jan 2025 11:26:59 -0800 Subject: [PATCH 1/3] feat: adopt `core::error::Error` --- Cargo.lock | 58 ++++++++++++++++++-------- Cargo.toml | 3 ++ alloc/src/buddy.rs | 10 ++--- hal-core/Cargo.toml | 2 + hal-core/src/addr.rs | 16 +------ hal-core/src/interrupt.rs | 8 +++- hal-core/src/mem/page.rs | 51 ++++++++++------------ hal-x86_64/Cargo.toml | 2 + hal-x86_64/src/interrupt.rs | 27 ++++++------ hal-x86_64/src/interrupt/apic/local.rs | 4 +- hal-x86_64/src/time.rs | 11 +---- hal-x86_64/src/time/pit.rs | 18 +++----- pci/src/error.rs | 3 +- src/allocator.rs | 4 +- src/arch/x86_64/tests.rs | 6 +-- 15 files changed, 109 insertions(+), 114 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88156a48..03fbda48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -217,7 +217,7 @@ checksum = "16e62a023e7c117e27523144c5d2459f4397fcc3cab0085af8e2224f643a0193" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -234,7 +234,7 @@ checksum = "c980ee35e870bd1a4d2c8294d4c04d0499e67bca1e4b5cefcc693c2fa00caea9" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -495,7 +495,7 @@ dependencies = [ "heck 0.4.1", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -705,7 +705,7 @@ checksum = "03cdc46ec28bd728e67540c528013c6a10eb69a02eb31078a1bda695438cbfb8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -932,7 +932,7 @@ checksum = "87750cf4b7a4c0625b1529e4c543c2182106e4dedc60a2a6455e00d212c489ac" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -1058,6 +1058,7 @@ dependencies = [ "embedded-graphics-core", "maitake-sync", "mycelium-util", + "thiserror 2.0.9", "tracing 0.2.0", ] @@ -1074,6 +1075,7 @@ dependencies = [ "proptest", "rand_core", "raw-cpuid", + "thiserror 2.0.9", "tracing 0.2.0", "volatile", ] @@ -1485,7 +1487,7 @@ dependencies = [ "bitvec", "serde", "serde-big-array", - "thiserror", + "thiserror 1.0.56", ] [[package]] @@ -1832,7 +1834,7 @@ checksum = "4359fd9c9171ec6e8c62926d6faaf553a8dc3f64e1507e76da7911b4f6a04405" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -1941,9 +1943,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.78" +version = "1.0.92" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2422ad645d89c99f8f3e6b88a9fdeca7fabeac836b1002371c4367c8f984aae" +checksum = "37d3544b3f2748c54e147655edb5025752e2303145b5aefb3c3ea2c78b973bb0" dependencies = [ "unicode-ident", ] @@ -2250,7 +2252,7 @@ checksum = "46fe8f8603d81ba86327b23a2e9cdf49e1255fb94a4c5f297f6ee0547178ea2c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -2342,9 +2344,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.48" +version = "2.0.94" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f3531638e407dfc0814761abb7c00a5b54992b849452a0646b7f65c9f770f3f" +checksum = "987bc0be1cdea8b10216bd06e2ca407d40b9543468fafd3ddfb02f36e77f71f3" dependencies = [ "proc-macro2", "quote", @@ -2405,7 +2407,16 @@ version = "1.0.56" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d54378c645627613241d077a3a79db965db602882668f9136ac42af9ecb730ad" dependencies = [ - "thiserror-impl", + "thiserror-impl 1.0.56", +] + +[[package]] +name = "thiserror" +version = "2.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f072643fd0190df67a8bab670c20ef5d8737177d6ac6b2e9a236cb096206b2cc" +dependencies = [ + "thiserror-impl 2.0.9", ] [[package]] @@ -2416,7 +2427,18 @@ checksum = "fa0faa943b50f3db30a20aa7e265dbc66076993efed8463e8de414e5d06d3471" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b50fa271071aae2e6ee85f842e2e28ba8cd2c5fb67f11fcb1fd70b276f9e7d4" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.94", ] [[package]] @@ -2508,7 +2530,7 @@ checksum = "5b8a1e28f2deaa14e508979454cb3a223b10b938b45af148bc0986de36f1923b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -2639,7 +2661,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -2649,7 +2671,7 @@ source = "git+https://github.com/tokio-rs/tracing#aa50d0e61373bb4af1ba567963e396 dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.94", ] [[package]] @@ -2833,7 +2855,7 @@ dependencies = [ "rustc_version", "rustversion", "sysinfo", - "thiserror", + "thiserror 1.0.56", "time", ] diff --git a/Cargo.toml b/Cargo.toml index f1ca61c1..010eeedf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,9 @@ members = [ ] resolver = "2" +[workspace.dependencies] +thiserror = { version = "2.0.9", default_features = false } + [package] name = "mycelium-kernel" version = "0.0.1" diff --git a/alloc/src/buddy.rs b/alloc/src/buddy.rs index 6d6239b7..b6eae3f4 100644 --- a/alloc/src/buddy.rs +++ b/alloc/src/buddy.rs @@ -4,7 +4,7 @@ use core::{ }; use hal_core::{ mem::{ - page::{self, AllocErr, PageRange, Size}, + page::{self, AllocError, PageRange, Size}, Region, RegionKind, }, Address, PAddr, VAddr, @@ -44,7 +44,7 @@ pub struct Alloc { free_lists: [Mutex>; FREE_LISTS], } -type Result = core::result::Result; +type Result = core::result::Result; pub struct Free { magic: usize, @@ -317,7 +317,7 @@ impl Alloc { // Find the order of the free list on which the freed range belongs. let min_order = self.order_for(layout); tracing::trace!(?min_order); - let min_order = min_order.ok_or_else(AllocErr::oom)?; + let min_order = min_order.ok_or_else(AllocError::oom)?; let Some(size) = self.size_for(layout) else { // XXX(eliza): is it better to just leak it? @@ -523,7 +523,7 @@ where // If the size of the page range would overflow, we *definitely* // can't allocate that lol. .checked_mul(actual_len) - .ok_or_else(AllocErr::oom)?; + .ok_or_else(AllocError::oom)?; debug_assert!( total_size.is_power_of_two(), @@ -540,7 +540,7 @@ where }; // Try to allocate the page range - let block = unsafe { self.alloc_inner(layout) }.ok_or_else(AllocErr::oom)?; + let block = unsafe { self.alloc_inner(layout) }.ok_or_else(AllocError::oom)?; // Return the allocation! let range = unsafe { block.as_ref() }.region().page_range(size); diff --git a/hal-core/Cargo.toml b/hal-core/Cargo.toml index 786b4223..e8ed4b69 100644 --- a/hal-core/Cargo.toml +++ b/hal-core/Cargo.toml @@ -4,11 +4,13 @@ version = "0.1.0" authors = ["Eliza Weisman ", "iximeow "] edition = "2021" license = "MIT" +rust_version = "1.81.0" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] tracing = { git = "https://github.com/tokio-rs/tracing", default_features = false } +thiserror = { workspace = true } maitake-sync = { path = "../maitake-sync", default-features = false } mycelium-util = { path = "../util" } embedded-graphics-core = { version = "0.3", optional = true } diff --git a/hal-core/src/addr.rs b/hal-core/src/addr.rs index 60c6eac9..1716c34e 100644 --- a/hal-core/src/addr.rs +++ b/hal-core/src/addr.rs @@ -1,5 +1,4 @@ use core::{fmt, ops}; -use mycelium_util::error::Error; pub trait Address: Copy @@ -142,7 +141,8 @@ pub struct PAddr(usize); #[repr(transparent)] pub struct VAddr(usize); -#[derive(Clone, Debug)] +#[derive(Clone, Debug, thiserror::Error)] +#[error("invalid address {addr:#x} for target architecture: {msg}")] pub struct InvalidAddress { msg: &'static str, addr: usize, @@ -408,18 +408,6 @@ impl InvalidAddress { Self { msg, addr } } } -impl fmt::Display for InvalidAddress { - #[inline] - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "address {:#x} not valid for target architecture: {}", - self.addr, self.msg - ) - } -} - -impl Error for InvalidAddress {} #[cfg(test)] mod tests { diff --git a/hal-core/src/interrupt.rs b/hal-core/src/interrupt.rs index ecd3c4cf..c37ef400 100644 --- a/hal-core/src/interrupt.rs +++ b/hal-core/src/interrupt.rs @@ -68,7 +68,8 @@ pub trait Handlers { } /// Errors that may occur while registering an interrupt handler. -#[derive(Clone, Eq, PartialEq)] +#[derive(Clone, Eq, PartialEq, thiserror::Error)] +#[error("{kind}")] pub struct RegistrationError { kind: RegistrationErrorKind, } @@ -78,10 +79,13 @@ pub struct CriticalGuard<'a, C: Control + ?Sized> { ctrl: &'a mut C, } -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Clone, Eq, PartialEq, thiserror::Error)] enum RegistrationErrorKind { + #[error("the provided interrupt vector does not exist")] Nonexistant, + #[error("an interrupt handler is already registered for this vector")] AlreadyRegistered, + #[error("{0}")] Other(&'static str), } diff --git a/hal-core/src/mem/page.rs b/hal-core/src/mem/page.rs index bd69a629..eb381ff3 100644 --- a/hal-core/src/mem/page.rs +++ b/hal-core/src/mem/page.rs @@ -32,7 +32,7 @@ pub unsafe trait Alloc { /// # Returns /// - `Ok(Page)` if a page was successfully allocated. /// - `Err` if no more pages can be allocated by this allocator. - fn alloc(&self, size: S) -> Result, AllocErr> { + fn alloc(&self, size: S) -> Result, AllocError> { self.alloc_range(size, 1).map(|r| r.start()) } @@ -41,7 +41,7 @@ pub unsafe trait Alloc { /// # Returns /// - `Ok(PageRange)` if a range of pages was successfully allocated /// - `Err` if the requested range could not be satisfied by this allocator. - fn alloc_range(&self, size: S, len: usize) -> Result, AllocErr>; + fn alloc_range(&self, size: S, len: usize) -> Result, AllocError>; /// Deallocate a single page. /// @@ -51,7 +51,7 @@ pub unsafe trait Alloc { /// # Returns /// - `Ok(())` if the page was successfully deallocated. /// - `Err` if the requested range could not be deallocated. - fn dealloc(&self, page: Page) -> Result<(), AllocErr> { + fn dealloc(&self, page: Page) -> Result<(), AllocError> { self.dealloc_range(page.range_inclusive(page)) } @@ -60,7 +60,7 @@ pub unsafe trait Alloc { /// # Returns /// - `Ok(())` if a range of pages was successfully deallocated /// - `Err` if the requested range could not be deallocated. - fn dealloc_range(&self, range: PageRange) -> Result<(), AllocErr>; + fn dealloc_range(&self, range: PageRange) -> Result<(), AllocError>; } pub trait Map @@ -391,21 +391,28 @@ pub struct PageRange { pub struct EmptyAlloc { _p: (), } -pub struct NotAligned { + +#[derive(thiserror::Error)] +#[error("not aligned on a {size} boundary")] +pub struct NotAligned { size: S, } -#[derive(Debug)] -pub struct AllocErr { +#[derive(Debug, thiserror::Error)] +#[error("allocator error")] +pub struct AllocError { // TODO: eliza _p: (), } -#[derive(Clone, Eq, PartialEq)] +#[derive(Clone, Eq, PartialEq, thiserror::Error)] #[non_exhaustive] pub enum TranslateError { + #[error("cannot translate an unmapped page/address")] NotMapped, + #[error("mapped page is a different size ({0})")] WrongSize(S), + #[error("error translating page/address: {0}")] Other(&'static str), } @@ -646,12 +653,12 @@ where } unsafe impl Alloc for EmptyAlloc { - fn alloc_range(&self, _: S, _len: usize) -> Result, AllocErr> { - Err(AllocErr { _p: () }) + fn alloc_range(&self, _: S, _len: usize) -> Result, AllocError> { + Err(AllocError { _p: () }) } - fn dealloc_range(&self, _range: PageRange) -> Result<(), AllocErr> { - Err(AllocErr { _p: () }) + fn dealloc_range(&self, _range: PageRange) -> Result<(), AllocError> { + Err(AllocError { _p: () }) } } @@ -689,36 +696,20 @@ impl fmt::Debug for TranslateError { } } -impl fmt::Display for TranslateError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self { - TranslateError::Other(msg) => write!(f, "error translating page/address: {msg}"), - TranslateError::NotMapped => f.pad("error translating page/address: not mapped"), - TranslateError::WrongSize(_) => write!( - f, - "error translating page: mapped page is a different size ({})", - core::any::type_name::() - ), - } - } -} - // === impl AllocErr === -impl AllocErr { +impl AllocError { pub fn oom() -> Self { Self { _p: () } } } -impl From> for AllocErr { +impl From> for AllocError { fn from(_na: NotAligned) -> Self { Self { _p: () } // TODO(eliza) } } -impl mycelium_util::error::Error for TranslateError {} - impl Size for S where S: StaticSize, diff --git a/hal-x86_64/Cargo.toml b/hal-x86_64/Cargo.toml index bfc1362a..64b662a5 100644 --- a/hal-x86_64/Cargo.toml +++ b/hal-x86_64/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" authors = ["Eliza Weisman ", "iximeow "] edition = "2021" license = "MIT" +rust_version = "1.81.0" [features] default = ["alloc"] @@ -19,6 +20,7 @@ mycelium-trace = { path = "../trace" } mycotest = { path = "../mycotest"} rand_core = { version = "0.6.4", default_features = false, optional = true } raw-cpuid = "10.6.0" +thiserror = { workspace = true } tracing = { git = "https://github.com/tokio-rs/tracing", default_features = false, features = ["attributes"] } volatile = { version = "0.4.5", features = ["unstable"] } diff --git a/hal-x86_64/src/interrupt.rs b/hal-x86_64/src/interrupt.rs index 43cfadf7..14a14ba8 100644 --- a/hal-x86_64/src/interrupt.rs +++ b/hal-x86_64/src/interrupt.rs @@ -42,11 +42,14 @@ pub struct CodeFault<'a> { /// An interrupt service routine. pub type Isr = extern "x86-interrupt" fn(&mut Context); -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum PeriodicTimerError { - Pit(time::PitError), - InvalidDuration(time::InvalidDuration), - Apic(apic::local::LocalApicError), + #[error("could not start PIT periodic timer: {0}")] + Pit(#[from] time::PitError), + #[error(transparent)] + InvalidDuration(#[from] time::InvalidDuration), + #[error("could not start local APIC periodic timer: {0}")] + Apic(#[from] apic::local::LocalApicError), } #[derive(Debug)] @@ -133,10 +136,6 @@ pub struct Registers { static IDT: Mutex = Mutex::new_with_raw_mutex(idt::Idt::new(), Spinlock::new()); static INTERRUPT_CONTROLLER: InitOnce = InitOnce::uninitialized(); -pub enum MaskError { - NotHwIrq, -} - /// ISA interrupt vectors /// /// See: [the other wiki](https://wiki.osdev.org/Interrupts#General_IBM-PC_Compatible_Interrupt_Information) @@ -378,13 +377,11 @@ impl Controller { InterruptModel::Pic(_) => crate::time::PIT .lock() .start_periodic_timer(interval) - .map_err(PeriodicTimerError::Pit), - InterruptModel::Apic { ref local, .. } => local - .with(|apic| { - apic.start_periodic_timer(interval, Idt::LOCAL_APIC_TIMER as u8) - .map_err(PeriodicTimerError::InvalidDuration) - }) - .map_err(PeriodicTimerError::Apic)?, + .map_err(Into::into), + InterruptModel::Apic { ref local, .. } => local.with(|apic| { + apic.start_periodic_timer(interval, Idt::LOCAL_APIC_TIMER as u8) + .map_err(PeriodicTimerError::from) + })?, } } } diff --git a/hal-x86_64/src/interrupt/apic/local.rs b/hal-x86_64/src/interrupt/apic/local.rs index f16733b8..9d48d0cb 100644 --- a/hal-x86_64/src/interrupt/apic/local.rs +++ b/hal-x86_64/src/interrupt/apic/local.rs @@ -35,13 +35,15 @@ pub trait RegisterAccess { ) -> Volatile<&'static mut Self::Target, Self::Access>; } -#[derive(Debug, Eq, PartialEq)] +#[derive(Debug, Eq, PartialEq, thiserror::Error)] pub enum LocalApicError { /// The system is configured to use the PIC interrupt model rather than the /// APIC interrupt model. + #[error("interrupt model is PIC, not APIC")] NoApic, /// The local APIC is uninitialized. + #[error("the local APIC has not been initialized on this core")] Uninitialized, } diff --git a/hal-x86_64/src/time.rs b/hal-x86_64/src/time.rs index 284023e6..4eeb431f 100644 --- a/hal-x86_64/src/time.rs +++ b/hal-x86_64/src/time.rs @@ -5,11 +5,11 @@ pub use self::{ pit::{Pit, PitError, PIT}, tsc::Rdtsc, }; -use core::fmt; pub use core::time::Duration; /// Error indicating that a [`Duration`] was invalid for a particular use. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, thiserror::Error)] +#[error("invalid duration {duration:?}: {message}")] pub struct InvalidDuration { duration: Duration, message: &'static str, @@ -17,13 +17,6 @@ pub struct InvalidDuration { // === impl InvalidDuration === -impl fmt::Display for InvalidDuration { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let Self { duration, message } = self; - write!(f, "invalid duration {duration:?}: {message}") - } -} - impl InvalidDuration { /// Returns the [`Duration`] that was invalid. #[must_use] diff --git a/hal-x86_64/src/time/pit.rs b/hal-x86_64/src/time/pit.rs index 6ecaf3a9..cb0e4f32 100644 --- a/hal-x86_64/src/time/pit.rs +++ b/hal-x86_64/src/time/pit.rs @@ -8,7 +8,6 @@ use core::{ }; use mycelium_util::{ bits::{bitfield, enum_from_bits}, - fmt, sync::{blocking::Mutex, spin::Spinlock}, }; @@ -87,14 +86,17 @@ pub struct Pit { } /// Errors returned by [`Pit::start_periodic_timer`] and [`Pit::sleep_blocking`]. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum PitError { /// The periodic timer is already running. + #[error("PIT periodic timer is already running")] AlreadyRunning, /// A [`Pit::sleep_blocking`] call is in progress. + #[error("a PIT sleep is already in progress")] SleepInProgress, /// The provided duration was invalid. - InvalidDuration(InvalidDuration), + #[error(transparent)] + InvalidDuration(#[from] InvalidDuration), } bitfield! { @@ -416,13 +418,3 @@ impl PitError { Self::InvalidDuration(InvalidDuration::new(duration, msg)) } } - -impl fmt::Display for PitError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::AlreadyRunning => write!(f, "PIT periodic timer is already running"), - Self::SleepInProgress => write!(f, "a PIT sleep is currently in progress"), - Self::InvalidDuration(e) => write!(f, "invalid PIT duration: {e}"), - } - } -} diff --git a/pci/src/error.rs b/pci/src/error.rs index 942daa3d..6e048d6c 100644 --- a/pci/src/error.rs +++ b/pci/src/error.rs @@ -1,5 +1,4 @@ use core::fmt; -use mycelium_util::error::Error; pub(crate) fn unexpected(value: T) -> UnexpectedValue where @@ -39,4 +38,4 @@ where } } -impl Error for UnexpectedValue where T: fmt::LowerHex + fmt::Debug {} +impl core::error::Error for UnexpectedValue where T: fmt::LowerHex + fmt::Debug {} diff --git a/src/allocator.rs b/src/allocator.rs index 0cd3fb07..1af7221d 100644 --- a/src/allocator.rs +++ b/src/allocator.rs @@ -126,7 +126,7 @@ where &self, size: S, len: usize, - ) -> Result, page::AllocErr> { + ) -> Result, page::AllocError> { self.allocating.fetch_add(1, Ordering::Release); let res = self.allocator.alloc_range(size, len); self.allocating.fetch_sub(1, Ordering::Release); @@ -134,7 +134,7 @@ where } #[inline] - fn dealloc_range(&self, range: page::PageRange) -> Result<(), page::AllocErr> { + fn dealloc_range(&self, range: page::PageRange) -> Result<(), page::AllocError> { self.deallocating.fetch_add(1, Ordering::Release); let res = self.allocator.dealloc_range(range); self.deallocating.fetch_sub(1, Ordering::Release); diff --git a/src/arch/x86_64/tests.rs b/src/arch/x86_64/tests.rs index b7899bd7..7cbb0e97 100644 --- a/src/arch/x86_64/tests.rs +++ b/src/arch/x86_64/tests.rs @@ -1,7 +1,7 @@ use hal_x86_64::mm; mycotest::decl_test! { - fn alloc_some_4k_pages() -> Result<(), hal_core::mem::page::AllocErr> { + fn alloc_some_4k_pages() -> Result<(), hal_core::mem::page::AllocError> { use hal_core::mem::page::Alloc; let page1 = tracing::info_span!("alloc page 1").in_scope(|| { let res = crate::ALLOC.alloc(mm::size::Size4Kb); @@ -50,7 +50,7 @@ mycotest::decl_test! { } mycotest::decl_test! { - fn alloc_4k_pages_and_ranges() -> Result<(), hal_core::mem::page::AllocErr> { + fn alloc_4k_pages_and_ranges() -> Result<(), hal_core::mem::page::AllocError> { use hal_core::mem::page::Alloc; let range1 = tracing::info_span!("alloc range 1").in_scope(|| { let res = crate::ALLOC.alloc_range(mm::size::Size4Kb, 16); @@ -96,7 +96,7 @@ mycotest::decl_test! { } mycotest::decl_test! { - fn alloc_some_pages() -> Result<(), hal_core::mem::page::AllocErr> { + fn alloc_some_pages() -> Result<(), hal_core::mem::page::AllocError> { use hal_core::mem::page::Alloc; let page1 = tracing::info_span!("alloc page 1").in_scope(|| { let res = crate::ALLOC.alloc(mm::size::Size4Kb); From 4b4298e4a285087b0a4a68f01868ff41eaf16d44 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 3 Jan 2025 11:45:15 -0800 Subject: [PATCH 2/3] chore: get kernel compiling on nightly-2025-01-01 It currently panics because the latest bootloader maps some stuff on huge pages which we don't expect... --- Cargo.lock | 24 ++++++++++++++++-------- hal-core/Cargo.toml | 2 +- hal-x86_64/Cargo.toml | 2 +- hal-x86_64/src/segment.rs | 9 +++++++-- inoculate/Cargo.toml | 4 ++-- rust-toolchain.toml | 2 +- src/arch/x86_64/oops.rs | 5 +---- x86_64-mycelium.json | 4 ++-- 8 files changed, 31 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 03fbda48..0e7d398a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -404,9 +404,9 @@ dependencies = [ [[package]] name = "bootloader" -version = "0.11.5" +version = "0.11.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0668e5a71825bbf8d9af46b68441e4426f070e7465b45070464977214dfada9e" +checksum = "8bace22e5c5b93f2e97859d0b74fa1338b8878009cdf752c377d1e8b44eccf26" dependencies = [ "anyhow", "async-process", @@ -423,9 +423,9 @@ dependencies = [ [[package]] name = "bootloader-boot-config" -version = "0.11.5" +version = "0.11.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be20fe897a953c6f2e63d392ea067237af98d02cd15ef97a48fef2768c1ed269" +checksum = "1efd4468bc7632e80a35b231c9694d20abff7bc5367b7dff8ce162d3b145a3dd" dependencies = [ "serde", ] @@ -1672,6 +1672,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "num-conv" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" + [[package]] name = "num-integer" version = "0.1.45" @@ -2453,12 +2459,13 @@ dependencies = [ [[package]] name = "time" -version = "0.3.31" +version = "0.3.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f657ba42c3f86e7680e53c8cd3af8abbe56b5491790b46e22e19c0d57463583e" +checksum = "35e7868883861bd0e56d9ac6efcaaca0d6d5d82a2a7ec8209ff492c07cf37b21" dependencies = [ "deranged", "itoa", + "num-conv", "powerfmt", "serde", "time-core", @@ -2473,10 +2480,11 @@ checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" [[package]] name = "time-macros" -version = "0.2.16" +version = "0.2.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26197e33420244aeb70c3e8c78376ca46571bc4e701e4791c2cd9f57dcb3a43f" +checksum = "2834e6017e3e5e4b9834939793b282bc03b37a3336245fa820e35e233e2a85de" dependencies = [ + "num-conv", "time-core", ] diff --git a/hal-core/Cargo.toml b/hal-core/Cargo.toml index e8ed4b69..1e872026 100644 --- a/hal-core/Cargo.toml +++ b/hal-core/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" authors = ["Eliza Weisman ", "iximeow "] edition = "2021" license = "MIT" -rust_version = "1.81.0" +rust-version = "1.81.0" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/hal-x86_64/Cargo.toml b/hal-x86_64/Cargo.toml index 64b662a5..2d243f70 100644 --- a/hal-x86_64/Cargo.toml +++ b/hal-x86_64/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" authors = ["Eliza Weisman ", "iximeow "] edition = "2021" license = "MIT" -rust_version = "1.81.0" +rust-version = "1.81.0" [features] default = ["alloc"] diff --git a/hal-x86_64/src/segment.rs b/hal-x86_64/src/segment.rs index 6f052949..a422d8eb 100644 --- a/hal-x86_64/src/segment.rs +++ b/hal-x86_64/src/segment.rs @@ -237,10 +237,15 @@ impl Selector { tracing::trace!("setting code segment..."); asm!( "push {selector}", - "lea {retaddr}, [1f + rip]", + "lea {retaddr}, [2f + rip]", "push {retaddr}", "retfq", - "1:", + // This is cool: apparently we can't use '0' or '1' as a label in + // LLVM assembly, due to an LLVM bug: + // https://github.com/llvm/llvm-project/issues/99547 + // + // So, this is 2 instead of 1. lmao. + "2:", selector = in(reg) self.0 as u64, retaddr = lateout(reg) _, options(preserves_flags), diff --git a/inoculate/Cargo.toml b/inoculate/Cargo.toml index 1110d224..e05b77fc 100644 --- a/inoculate/Cargo.toml +++ b/inoculate/Cargo.toml @@ -16,7 +16,7 @@ tracing = "0.1.23" tracing-subscriber = "0.3.16" tracing-error = "0.2" color-eyre = "0.6" -bootloader = "0.11.2" +bootloader = "0.11.9" bootloader-boot-config = "0.11.3" locate-cargo-manifest = "0.2" wait-timeout = "0.2" @@ -25,4 +25,4 @@ atty = "0.2" mycotest = { path = "../mycotest", features = ["alloc"] } heck = "0.3.3" # used for UEFI booting in QEMU -ovmf-prebuilt = "0.1.0-alpha.1" \ No newline at end of file +ovmf-prebuilt = "0.1.0-alpha.1" diff --git a/rust-toolchain.toml b/rust-toolchain.toml index eaeebf9e..0087141f 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,5 +1,5 @@ [toolchain] -channel = "nightly-2024-01-20" +channel = "nightly-2025-01-01" components = [ "clippy", "rustfmt", diff --git a/src/arch/x86_64/oops.rs b/src/arch/x86_64/oops.rs index 7e20ac8a..825b00f4 100644 --- a/src/arch/x86_64/oops.rs +++ b/src/arch/x86_64/oops.rs @@ -111,10 +111,7 @@ pub fn oops(oops: Oops<'_>) -> ! { } => writeln!(mk_writer.make_writer(), "a {kind} occurred!\n").unwrap(), OopsSituation::Panic(panic) => { let mut writer = mk_writer.make_writer(); - match panic.message() { - Some(msg) => writeln!(writer, "mycelium panicked: {msg}").unwrap(), - None => writeln!(writer, "mycelium panicked!").unwrap(), - } + write!(writer, "mycelium panicked: {}", panic.message()).unwrap(); if let Some(loc) = panic.location() { writeln!(writer, "at {}:{}:{}", loc.file(), loc.line(), loc.column()).unwrap(); } diff --git a/x86_64-mycelium.json b/x86_64-mycelium.json index 653e62d4..b53f3461 100644 --- a/x86_64-mycelium.json +++ b/x86_64-mycelium.json @@ -1,6 +1,6 @@ { "llvm-target": "x86_64-unknown-none", - "data-layout": "e-m:e-i64:64-f80:128-n8:16:32:64-S128", + "data-layout": "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128", "arch": "x86_64", "target-endian": "little", "target-pointer-width": "64", @@ -21,4 +21,4 @@ "panic-strategy": "abort", "disable-redzone": true, "features": "-mmx,-sse,+soft-float" -} \ No newline at end of file +} From 670a68bc30d12558b0db0ad2402d0a66bf7e84a8 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 4 Jan 2025 11:35:19 -0800 Subject: [PATCH 3/3] wip attempt to rewrite paging to support hugepage --- Cargo.toml | 1 + hal-core/src/mem/page.rs | 121 ++++++++++++++--------- hal-x86_64/src/interrupt/apic/ioapic.rs | 9 +- hal-x86_64/src/mm.rs | 123 ++++++++++++++++++++---- 4 files changed, 185 insertions(+), 69 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 010eeedf..f38b1563 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ maitake = { path = "maitake", features = ["tracing-02"] } mycelium-pci = { path = "pci" } mycelium-util = { path = "util" } mycelium-trace = { path = "trace", features = ["embedded-graphics"] } + rand_xoshiro = "0.6" rand = { version = "0.8", default_features = false } rlibc = "1.0" diff --git a/hal-core/src/mem/page.rs b/hal-core/src/mem/page.rs index eb381ff3..14cdff13 100644 --- a/hal-core/src/mem/page.rs +++ b/hal-core/src/mem/page.rs @@ -63,10 +63,9 @@ pub unsafe trait Alloc { fn dealloc_range(&self, range: PageRange) -> Result<(), AllocError>; } -pub trait Map +pub trait Map<'mapper, S, A> where S: Size, - A: Alloc, { type Entry: PageFlags; @@ -95,13 +94,13 @@ where /// /// Good luck and have fun! unsafe fn map_page( - &mut self, + &'mapper mut self, virt: Page, phys: Page, frame_alloc: &A, - ) -> Handle<'_, S, Self::Entry>; + ) -> Handle; - fn flags_mut(&mut self, virt: Page) -> Handle<'_, S, Self::Entry>; + fn flags_mut(&'mapper mut self, virt: Page) -> Handle; /// Unmap the provided virtual page, returning the physical page it was /// previously mapped to. @@ -121,10 +120,10 @@ where /// Identity map the provided physical page to the virtual page with the /// same address. fn identity_map( - &mut self, + &'mapper mut self, phys: Page, frame_alloc: &A, - ) -> Handle<'_, S, Self::Entry> { + ) -> Handle { let base_paddr = phys.base_addr().as_usize(); let virt = Page::containing(VAddr::from_usize(base_paddr), phys.size()); unsafe { self.map_page(virt, phys, frame_alloc) } @@ -169,34 +168,35 @@ where /// /// Good luck and have fun! unsafe fn map_range( - &mut self, + &'mapper mut self, virt: PageRange, phys: PageRange, mut set_flags: F, frame_alloc: &A, ) -> PageRange where - F: FnMut(&mut Handle<'_, S, Self::Entry>), + F: FnMut(&mut Handle), { - let _span = tracing::trace_span!("map_range", ?virt, ?phys).entered(); - assert_eq!( - virt.len(), - phys.len(), - "virtual and physical page ranges must have the same number of pages" - ); - assert_eq!( - virt.size(), - phys.size(), - "virtual and physical pages must be the same size" - ); - for (virt, phys) in virt.into_iter().zip(&phys) { - tracing::trace!(virt.page = ?virt, phys.page = ?phys, "mapping..."); - let mut flags = self.map_page(virt, phys, frame_alloc); - set_flags(&mut flags); - flags.commit(); - tracing::trace!(virt.page = ?virt, phys.page = ?phys, "mapped"); - } - virt + todo!("agh, mutability"); + // let _span = tracing::trace_span!("map_range", ?virt, ?phys).entered(); + // assert_eq!( + // virt.len(), + // phys.len(), + // "virtual and physical page ranges must have the same number of pages" + // ); + // assert_eq!( + // virt.size(), + // phys.size(), + // "virtual and physical pages must be the same size" + // ); + // for (virt, phys) in virt.into_iter().zip(&phys) { + // tracing::trace!(virt.page = ?virt, phys.page = ?phys, "mapping..."); + // let mut flags = self.map_page(virt, phys, frame_alloc); + // set_flags(&mut flags); + // flags.commit(); + // tracing::trace!(virt.page = ?virt, phys.page = ?phys, "mapped"); + // } + // virt } /// Unmap the provided range of virtual pages. @@ -254,13 +254,13 @@ where /// - If any page's physical address is invalid. /// - If any page is already mapped. fn identity_map_range( - &mut self, + &'mapper mut self, phys: PageRange, set_flags: F, frame_alloc: &A, ) -> PageRange where - F: FnMut(&mut Handle<'_, S, Self::Entry>), + F: FnMut(&mut Handle), { let base_paddr = phys.base_addr().as_usize(); let page_size = phys.start().size(); @@ -272,26 +272,25 @@ where } } -impl Map for &mut M +impl<'mapper, M, A, S> Map<'mapper, S, A> for &'mapper mut M where - M: Map, + M: Map<'mapper, S, A>, S: Size, - A: Alloc, { type Entry = M::Entry; #[inline] unsafe fn map_page( - &mut self, + &'mapper mut self, virt: Page, phys: Page, frame_alloc: &A, - ) -> Handle<'_, S, Self::Entry> { + ) -> Handle { (*self).map_page(virt, phys, frame_alloc) } #[inline] - fn flags_mut(&mut self, virt: Page) -> Handle<'_, S, Self::Entry> { + fn flags_mut(&'mapper mut self, virt: Page) -> Handle { (*self).flags_mut(virt) } @@ -302,10 +301,10 @@ where #[inline] fn identity_map( - &mut self, + &'mapper mut self, phys: Page, frame_alloc: &A, - ) -> Handle<'_, S, Self::Entry> { + ) -> Handle { (*self).identity_map(phys, frame_alloc) } } @@ -362,13 +361,43 @@ pub trait PageFlags: fmt::Debug { fn commit(&mut self, page: Page); } +impl PageFlags for &'_ mut dyn PageFlags { + unsafe fn set_writable(&mut self, writable: bool) { + (*self).set_writable(writable) + } + + unsafe fn set_executable(&mut self, executable: bool) { + (*self).set_executable(executable) + } + + unsafe fn set_present(&mut self, present: bool) { + (*self).set_present(present) + } + + fn is_writable(&self) -> bool { + >::is_writable(*self) + } + + fn is_executable(&self) -> bool { + >::is_executable(*self) + } + + fn is_present(&self) -> bool { + >::is_present(*self) + } + + fn commit(&mut self, page: Page) { + (*self).commit(page) + } +} + /// A page in the process of being remapped. /// /// This reference allows updating page table flags prior to committing changes. #[derive(Debug)] #[must_use = "page table updates may not be reflected until changes are committed (using `Handle::commit`)"] -pub struct Handle<'mapper, S: Size, E: PageFlags> { - entry: &'mapper mut E, +pub struct Handle> { + entry: E, page: Page, } @@ -721,12 +750,12 @@ where // === impl Handle === -impl<'mapper, S, E> Handle<'mapper, S, E> +impl Handle where S: Size, E: PageFlags, { - pub fn new(page: Page, entry: &'mapper mut E) -> Self { + pub fn new(page: Page, entry: E) -> Self { Self { entry, page } } @@ -745,7 +774,7 @@ where /// page table (i.e. it has multiple page table entries pointing to it) may /// also cause undefined behavior. #[inline] - pub unsafe fn set_writable(self, writable: bool) -> Self { + pub unsafe fn set_writable(mut self, writable: bool) -> Self { self.entry.set_writable(writable); self } @@ -759,7 +788,7 @@ where /// undefined behavior. Also, this can be used to execute the contents of /// arbitrary memory, which (of course) is wildly unsafe. #[inline] - pub unsafe fn set_executable(self, executable: bool) -> Self { + pub unsafe fn set_executable(mut self, executable: bool) -> Self { self.entry.set_executable(executable); self } @@ -770,7 +799,7 @@ where /// /// Manual control of page flags can be used to violate Rust invariants. #[inline] - pub unsafe fn set_present(self, present: bool) -> Self { + pub unsafe fn set_present(mut self, present: bool) -> Self { self.entry.set_present(present); self } @@ -791,7 +820,7 @@ where } #[inline] - pub fn commit(self) -> Page { + pub fn commit(mut self) -> Page { tracing::debug!( page = ?self.page, entry = ?self.entry, diff --git a/hal-x86_64/src/interrupt/apic/ioapic.rs b/hal-x86_64/src/interrupt/apic/ioapic.rs index 0a0426df..7fb0ef2f 100644 --- a/hal-x86_64/src/interrupt/apic/ioapic.rs +++ b/hal-x86_64/src/interrupt/apic/ioapic.rs @@ -320,12 +320,13 @@ impl IoApic { /// # Returns /// - `Some(IoApic)` if this CPU supports the APIC interrupt model. /// - `None` if this CPU does not support APIC interrupt handling. - pub fn try_new( + pub fn try_new<'m, A, M>( base_paddr: PAddr, - pagectrl: &mut impl page::Map, + pagectrl: &'m mut M, frame_alloc: &A, ) -> Result where + M: page::Map<'m, mm::size::AnySize, A>, A: page::Alloc, { if !super::is_supported() { @@ -338,8 +339,8 @@ impl IoApic { unsafe { // ensure the I/O APIC's MMIO page is mapped and writable. - let virt = VirtPage::::containing_fixed(base); - let phys = PhysPage::::containing_fixed(base_paddr); + let virt = VirtPage::containing_addr(base, mm::size::AnySize::Size4Kb); + let phys = PhysPage::containing_addr(base_paddr, mm::size::AnySize::Size4Kb).unwrap(); tracing::debug!(?virt, ?phys, "mapping I/O APIC MMIO page..."); pagectrl .map_page(virt, phys, frame_alloc) diff --git a/hal-x86_64/src/mm.rs b/hal-x86_64/src/mm.rs index b709f704..5659b3ab 100644 --- a/hal-x86_64/src/mm.rs +++ b/hal-x86_64/src/mm.rs @@ -138,18 +138,103 @@ impl PageTable { } } -impl Map for PageCtrl +impl<'mapper, A> Map<'mapper, AnySize, A> for PageCtrl where A: page::Alloc, { - type Entry = Entry; + type Entry = &'mapper dyn page::PageFlags; unsafe fn map_page( - &mut self, + &'mapper mut self, + virt: Page, + phys: Page, + frame_alloc: &A, + ) -> page::Handle { + // XXX(eliza): most of this fn is *internally* safe and should be + // factored out into a safe function... + let span = tracing::debug_span!("map_page", ?virt, ?phys); + let _e = span.enter(); + let pml4 = self.pml4.as_mut(); + + let vaddr = virt.base_addr(); + tracing::trace!(?vaddr); + + let pdpt = pml4 + .create_next_table(virt, frame_alloc) + .expect("PML4 shouldn't point directly to a hugepage lol"); + let pd = match pdpt.create_next_table(virt, alloc) { + Ok(pd) => pd, + Err(entry) => { + tracing::debug!(?entry, "found 1GB hugepage"); + assert!( + !entry.is_present(), + "mapped page table entry already in use" + ); + let phys = Page::starting_at(phys.base_addr(), AnySize::Size1Gb).unwrap(); + entry.set_phys_page(phys).set_present(true); + tracing::trace!(?entry, "flags set"); + let virt = Page::starting_at(virt.base_addr(), AnySize::Size1Gb).unwrap(); + return page::Handle::new(virt, entry); + } + }; + let page_table = match create_next_table(virt, frame_alloc) { + Ok(pt) => pt, + Err(entry) => { + tracing::debug!(?entry, "found 2MB hugepage"); + assert!( + !entry.is_present(), + "mapped page table entry already in use" + ); + let phys = Page::starting_at(phys.base_addr(), AnySize::Size2Mb).unwrap(); + entry.set_phys_page(phys).set_present(true); + tracing::trace!(?entry, "flags set"); + let virt = Page::starting_at(virt.base_addr(), AnySize::Size2Mb).unwrap(); + return page::Handle::new(virt, entry); + } + }; + tracing::debug!(?page_table); + + let entry = &mut page_table[virt]; + tracing::trace!(?entry); + assert!( + !entry.is_present(), + "mapped page table entry already in use" + ); + assert!(!entry.is_huge(), "huge bit should not be set for 4KB entry"); + let phys = Page::starting_at(phys.base_addr(), AnySize::Size4Kb).unwrap(); + let entry = entry.set_phys_page(phys).set_present(true); + tracing::trace!(?entry, "flags set"); + let virt = Page::starting_at(virt.base_addr(), AnySize::Size4Kb).unwrap(); + page::Handle::new(virt, entry) + } + + fn flags_mut( + &'mapper mut self, + _virt: Page, + ) -> page::Handle { + unimplemented!() + } + + /// # Safety + /// + /// Unmapping a page can break pretty much everything. + unsafe fn unmap(&mut self, _virt: Page) -> Page { + unimplemented!() + } +} + +impl<'mapper, A> Map<'mapper, Size4Kb, A> for PageCtrl +where + A: page::Alloc, +{ + type Entry = &'mapper mut Entry; + + unsafe fn map_page( + &'mapper mut self, virt: Page, phys: Page, frame_alloc: &A, - ) -> page::Handle<'_, Size4Kb, Self::Entry> { + ) -> page::Handle { // XXX(eliza): most of this fn is *internally* safe and should be // factored out into a safe function... let span = tracing::debug_span!("map_page", ?virt, ?phys); @@ -177,14 +262,17 @@ where page::Handle::new(virt, entry) } - fn flags_mut(&mut self, _virt: Page) -> page::Handle<'_, Size4Kb, Self::Entry> { + fn flags_mut( + &'mapper mut self, + _virt: Page, + ) -> page::Handle { unimplemented!() } /// # Safety /// /// Unmapping a page can break pretty much everything. - unsafe fn unmap(&mut self, _virt: Page) -> Page { + unsafe fn unmap(&'mapper mut self, _virt: Page) -> Page { unimplemented!() } } @@ -356,26 +444,22 @@ impl PageTable { &mut self, idx: VirtPage, alloc: &impl page::Alloc, - ) -> &mut PageTable { + ) -> Result<&mut PageTable, &mut Entry> { let span = tracing::trace_span!("create_next_table", ?idx, self.level = %R::NAME, next.level = %::NAME); let _e = span.enter(); if self.next_table(idx).is_some() { tracing::trace!("next table already exists"); - return self + return Ok(self .next_table_mut(idx) - .expect("if next_table().is_some(), the next table exists!"); + .expect("if next_table().is_some(), the next table exists!")); } tracing::trace!("no next table exists"); let entry = &mut self[idx]; if entry.is_huge() { - panic!( - "cannot create {} table for {:?}: the corresponding entry is huge!\n{:#?}", - ::NAME, - idx, - entry - ); + tracing::trace!("page is hudge"); + return Err(entry); } tracing::trace!("trying to allocate page table frame..."); @@ -395,9 +479,10 @@ impl PageTable { .set_phys_addr(frame.base_addr()); tracing::trace!(?entry, ?frame, "set page table entry to point to frame"); - self.next_table_mut(idx) + Ok(self + .next_table_mut(idx) .expect("we should have just created this table!") - .zero() + .zero()) } } @@ -559,7 +644,7 @@ impl Entry { } } -impl page::PageFlags for Entry { +impl page::PageFlags for Entry { #[inline] fn is_writable(&self) -> bool { self.is_writable() @@ -590,7 +675,7 @@ impl page::PageFlags for Entry { self.set_present(present); } - fn commit(&mut self, page: Page) { + fn commit(&mut self, page: Page) { unsafe { tlb::flush_page(page.base_addr()); }