From 1d192adde0c6a46a8dbf76716ad8f7be20a4c1ed Mon Sep 17 00:00:00 2001 From: Jacob Hinchliffe Date: Thu, 6 Mar 2025 20:11:54 +0000 Subject: [PATCH] Removed messy debug logging, added print_oneshot!() --- kernel/src/arch/x86_64/cpu/apic.rs | 18 ++- kernel/src/arch/x86_64/drivers/ascii/mod.rs | 52 ++++++++- .../x86_64/memory/allocation/heap_alloc.rs | 3 +- kernel/src/lib.rs | 59 +++++----- kernel/src/prelude.rs | 7 ++ kernel/src/std/elf/mod.rs | 4 +- kernel/src/std/maths/geometry.rs | 8 ++ kernel/src/std/unwind/panic.rs | 9 +- kernel/src/std/unwind/unwinder.rs | 1 - kernel/src/step.rs | 104 ++++++++++++++++++ rustfmt.toml | 1 + 11 files changed, 216 insertions(+), 50 deletions(-) create mode 100644 kernel/src/prelude.rs create mode 100644 kernel/src/step.rs diff --git a/kernel/src/arch/x86_64/cpu/apic.rs b/kernel/src/arch/x86_64/cpu/apic.rs index 2339011..4ea40ab 100644 --- a/kernel/src/arch/x86_64/cpu/apic.rs +++ b/kernel/src/arch/x86_64/cpu/apic.rs @@ -2,10 +2,13 @@ use core::arch::x86_64::__cpuid; -use crate::arch::x86_64::cpu::msr::*; -use crate::arch::x86_64::memory::mapping::PHYSICAL_MEMORY_OFFSET; -use crate::arch::x86_64::memory::{FRAME_ALLOCATOR, OFFSET_PAGE_TABLE}; -use crate::{debugln, serial_print, serial_println}; +use crate::arch::x86_64::{ + cpu::msr::*, + memory::{ + FRAME_ALLOCATOR, OFFSET_PAGE_TABLE, mapping::PHYSICAL_MEMORY_OFFSET, + }, +}; + use x86_64::structures::paging::Translate; use x86_64::{ PhysAddr, VirtAddr, @@ -28,7 +31,6 @@ const CPUID_FEAT_EDX_APIC: u64 = 1 << 9; // the cpuid instruction will return th fn set_apic_base_enable(apic: PhysAddr) { let rax = (apic.as_u64() & 0xfffff0000) | IA32_APIC_BASE_MSR_ENABLE; - debugln!("apic {:?}", PhysAddr::new(rax)); cpu_set_msr(IA32_APIC_BASE_MSR, rax); } @@ -42,8 +44,6 @@ fn get_apic_base() -> PhysAddr { let mut value: u64 = 0; cpu_get_msr(IA32_APIC_BASE_MSR, &mut value); - debugln!("apic base {:#x}", value); - PhysAddr::new(value & 0xfffff0000) } @@ -55,7 +55,6 @@ fn write_apic_register(apic_base: &PhysAddr, reg: u64, value: u32) { let phys_check = OFFSET_PAGE_TABLE.get().unwrap().lock().translate(virt_addr); - debugln!("{:?}", phys_check); unsafe { *(virt_addr.as_u64() as *mut u32) = value }; } @@ -76,14 +75,13 @@ pub fn check_apic() -> bool { unsafe fn phys_to_virt(phys: PhysAddr) -> VirtAddr { let phys = phys.as_u64(); phys.checked_add(*PHYSICAL_MEMORY_OFFSET) - .map_or_else(|| panic!(" overflow"), VirtAddr::new) + .map_or_else(|| panic!("Overflow"), VirtAddr::new) } pub fn enable_apic() { let apic_base_physical_addr = get_apic_base(); set_apic_base_enable(apic_base_physical_addr); - debugln!("{:?}", apic_base_physical_addr); enable_timer(); write_apic_register( diff --git a/kernel/src/arch/x86_64/drivers/ascii/mod.rs b/kernel/src/arch/x86_64/drivers/ascii/mod.rs index e2a75fa..a102346 100644 --- a/kernel/src/arch/x86_64/drivers/ascii/mod.rs +++ b/kernel/src/arch/x86_64/drivers/ascii/mod.rs @@ -1,4 +1,4 @@ -use core::fmt; +use core::fmt::{self, Write}; use spin::{Lazy, Mutex}; use x86_64::instructions::interrupts; @@ -7,6 +7,7 @@ use crate::arch::x86_64::drivers::framebuffer::{ }; use crate::resources::font::{FONT_SPLEEN_8X16, Font}; +use crate::std::maths::geometry::Vec2; static FONT_WIDTH: u32 = 8; static FONT_HEIGHT: u32 = 16; @@ -25,11 +26,8 @@ pub struct Writer { screen_width: u32, /// Measured in chars not pixels. screen_height: u32, - /// 16 pixels tall. text_line: u32, - /// 8 pixels wide. text_col: u32, - fg_color: Colour, bg_color: Colour, } @@ -62,6 +60,15 @@ impl Writer { self.font = font; } + pub const fn set_pos(&mut self, coords: Vec2) { + self.text_col = coords.x(); + self.text_line = coords.y(); + } + + pub const fn pos(&self) -> Vec2 { + Vec2::new(self.text_col, self.text_line) + } + /// This is sent when the user types a backspace. const BACKSPACE: u8 = 8; @@ -206,6 +213,34 @@ pub fn clear_screen() { }); } +/// Prints a string at a given `coords` before returning to original position. +/// +/// These are 0 based indices and refer to character position, not pixel +/// positions. +pub fn _print_oneshot( + fg: Colour, + bg: Colour, + coords: Vec2, + args: fmt::Arguments, +) { + interrupts::without_interrupts(|| { + let mut writer = WRITER.lock(); + + // Save the old positions of `text_col` and `text_line`. + let old_pos = writer.pos(); + + // Also save the colors. + let old_fg_color = writer.fg_color; + let old_bg_color = writer.bg_color; + + writer.set_pos(coords); + writer.set_colour(fg, bg); + writer.write_fmt(args).unwrap(); + writer.set_colour(old_fg_color, old_bg_color); + writer.set_pos(old_pos); + }); +} + pub fn reset_cursor() { interrupts::without_interrupts(|| { let mut writer = WRITER.lock(); @@ -214,6 +249,15 @@ pub fn reset_cursor() { }); } +#[macro_export] +/// Prints a coloured string at a position before restoring previous position +/// and colour. +macro_rules! print_oneshot { + ($position:expr, $fg:expr, $bg:expr, $($arg:tt)*) => { + $crate::arch::x86_64::drivers::ascii::_print_oneshot($fg, $bg, $position, format_args!($($arg)*)); + }; +} + #[macro_export] macro_rules! println_log { () => ($crate::print_log!("\n")); diff --git a/kernel/src/arch/x86_64/memory/allocation/heap_alloc.rs b/kernel/src/arch/x86_64/memory/allocation/heap_alloc.rs index 6f8f8f9..7c9684c 100644 --- a/kernel/src/arch/x86_64/memory/allocation/heap_alloc.rs +++ b/kernel/src/arch/x86_64/memory/allocation/heap_alloc.rs @@ -3,8 +3,7 @@ use crate::arch::x86_64::memory::units::MemoryUnits; use crate::arch::x86_64::memory::{ FRAME_ALLOCATOR, HEAP_SIZE, HEAP_VIRTUAL_SPACE, }; -use crate::serial_println; -use crate::{debugln, serial_print}; +use crate::debugln; use core::alloc::{GlobalAlloc, Layout}; use core::ptr; use spin::{Mutex, MutexGuard}; diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index b5e5f2d..4ad95f1 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -1,5 +1,6 @@ #![no_std] #![feature(abi_x86_interrupt)] +#![feature(impl_trait_in_bindings)] #![warn( clippy::correctness, clippy::nursery, @@ -14,8 +15,6 @@ extern crate alloc; use crate::{ - // TODO: Fix nesting under `arch`. A lot of code does not NEED to run on - // x86_64. Note that the panic handler does. arch::x86_64::{ cpu::apic::enable_apic, drivers::{ @@ -26,34 +25,29 @@ use crate::{ allocation::{ heap_alloc::init_heap, page_alloc::FoundryOSFrameAllocator, }, - init_page_table, + init_page_table, mapping, units::MemoryUnits, }, }, prelude::*, }; -use arch::x86_64::memory::mapping; +use alloc::{boxed::Box, format}; use core::arch::asm; use limine::BaseRevision; use std::unwind::UNWINDER; use x86_64::VirtAddr; pub mod arch; +/// Commonly used re-exports. +pub mod prelude; pub mod resources; -#[allow(unused)] // We aren't using much of this right now. +// We aren't using much of this right now. +#[allow(unused)] pub mod std; +mod step; pub mod util; -pub mod prelude { - pub use crate::{ - debug, debugln, eprint, eprintln, print, print_log, println, - println_log, serial_print, serial_println, - std::debug::_debug, - std::io::{_print, _print_err, _print_log, _serial_write}, - }; -} - /// Sets the base revision to the latest revision supported by the crate. /// See specification for further info. /// Be sure to mark all limine requests with #[used], otherwise they may be @@ -73,12 +67,25 @@ pub fn hcf() -> ! { } } +#[derive(Debug)] +pub struct NoError {} + +impl core::fmt::Display for NoError { + fn fmt(&self, _f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + Ok(()) + } +} + +pub struct NoTags; + +impl core::error::Error for NoError {} + /// Panicking before this is initialised is unwise. We should probably extract /// very early init into it's own function because Stack Traces may require /// allocations etc. -pub fn boot() -> Result<(), &'static str> { +pub fn boot() -> Result<(), Box> { if !BASE_REVISION.is_supported() { - return Err("Base revision not supported"); + return Err("Base revision not supported.".into()); } use arch::x86_64::{gdt, interrupts}; @@ -116,22 +123,22 @@ pub fn boot() -> Result<(), &'static str> { FoundryOSFrameAllocator::init(memory_map); let available_bytes = FRAME_ALLOCATOR.get().unwrap().lock().available_memory(); + debugln!( " => Available Memory: {}", MemoryUnits::from_bytes(available_bytes as usize) ); - debugln!("[Success]"); - + // Allocations should be all fine past this point. debugln!(" Initialising Heap... "); - if unsafe { init_heap() }.is_err() { - return Err("Failed to initialise heap: error"); + match unsafe { init_heap() } { + Ok(_) => debugln!(" [Success]"), + Err(why) => return Err(format!("{:?}", why).into()), } - debugln!(" [Success]"); - // debug!(" Enabling PICs... "); - // interrupts::enable_pic(); - // debugln!("[Success]"); + debug!(" Enabling PICs... "); + interrupts::enable_pic(); + debugln!("[Success]"); debug!(" Disabling PICs... "); interrupts::disable_pic(); @@ -145,7 +152,9 @@ pub fn boot() -> Result<(), &'static str> { x86_64::instructions::interrupts::enable(); debugln!("[Success]"); - UNWINDER.lock(); // Initialises the Unwinder once and only once :fingers_crossed:. + // Initialises the Unwinder once and only once because this makes a heap + // allocation. + UNWINDER.lock(); Ok(()) } diff --git a/kernel/src/prelude.rs b/kernel/src/prelude.rs new file mode 100644 index 0000000..f74b661 --- /dev/null +++ b/kernel/src/prelude.rs @@ -0,0 +1,7 @@ +pub use crate::{ + arch::x86_64::drivers::framebuffer::colour::Colour, + debug, debugln, eprint, eprintln, print, print_log, print_oneshot, println, + println_log, serial_print, serial_println, + std::debug::_debug, + std::io::{_print, _print_err, _print_log, _serial_write}, +}; diff --git a/kernel/src/std/elf/mod.rs b/kernel/src/std/elf/mod.rs index 92956b5..4b36c61 100644 --- a/kernel/src/std/elf/mod.rs +++ b/kernel/src/std/elf/mod.rs @@ -84,15 +84,13 @@ impl ElfReader { Ok(Self { elf }) } - #[warn(clippy::unwrap_used)] pub fn get_symbol_table( &self, ) -> Result< Option<(SymbolTable<'static, LittleEndian>, StringTable<'static>)>, ElfError, > { - // TODO: Remove .unwrap(). - Ok(self.elf.symbol_table().unwrap()) + Ok(self.elf.symbol_table()?) } /// Gets the section size of `section_name` in bytes. diff --git a/kernel/src/std/maths/geometry.rs b/kernel/src/std/maths/geometry.rs index 99b2133..e5dfd5f 100644 --- a/kernel/src/std/maths/geometry.rs +++ b/kernel/src/std/maths/geometry.rs @@ -42,6 +42,14 @@ impl Vec2 { pub const fn y(&self) -> T { self.y } + + pub const fn set_x(&mut self, x: T) { + self.x = x; + } + + pub const fn set_y(&mut self, y: T) { + self.y = y; + } } impl AddAssign for Vec2 { diff --git a/kernel/src/std/unwind/panic.rs b/kernel/src/std/unwind/panic.rs index a24863c..1c04791 100644 --- a/kernel/src/std/unwind/panic.rs +++ b/kernel/src/std/unwind/panic.rs @@ -16,8 +16,7 @@ use crate::{ #[panic_handler] /// A basic panic handler which can produce a helpful stack trace. pub fn panic_handler(info: &PanicInfo<'_>) -> ! { - println!("Kernel panic: {}", info); - serial_println!("Kernel panic: {}", info); + debugln!("Kernel panic: {}", info); let mut rip: u64; let mut rsp: u64; @@ -33,18 +32,18 @@ pub fn panic_handler(info: &PanicInfo<'_>) -> ! { while let Some(call_frame) = unwinder.next().unwrap_or_else(|err| { // If an unwind error occurred. - eprintln!("{:?}", err); + debugln!("{:?}", err); hcf() }) { serial_println!("Got frame: {:x?}", call_frame); let Some((symtab, strtab)) = ELF.get_symbol_table().unwrap_or_else(|e| { - serial_println!("{:?}", e); + debugln!("{:?}", e); hcf() }) else { // TODO: Omit symbol names but just print addresses. - serial_println!("Didn't find symtab and strtab!"); + debugln!("Didn't find symtab and strtab!"); hcf() }; diff --git a/kernel/src/std/unwind/unwinder.rs b/kernel/src/std/unwind/unwinder.rs index f64ef57..2eda690 100644 --- a/kernel/src/std/unwind/unwinder.rs +++ b/kernel/src/std/unwind/unwinder.rs @@ -126,7 +126,6 @@ impl FallibleIterator for Unwinder { Ok(Some(CallFrame { pc, symbol: 0 })) } - // fn next(&mut self) -> Option, UnwinderError>> {} } impl Unwinder { diff --git a/kernel/src/step.rs b/kernel/src/step.rs new file mode 100644 index 0000000..3b6ba4c --- /dev/null +++ b/kernel/src/step.rs @@ -0,0 +1,104 @@ +/* //! Defines the [Step] struct which may be used when initialising the system. +//! +//! # Warning +//! +//! Use of alloc is currently required, so early initialisation will need a +//! different method. We should possibly abstract some functionality so that +//! consumers don't need to worry or care about the difference. +use alloc::{borrow::ToOwned, boxed::Box, string::String, vec::Vec}; + +use crate::{arch::x86_64::drivers::ascii::WRITER, prelude::*}; + +/// Represents a [Step] when initialising the system. +/// +/// Handles printing various information to the framebuffer using steps. +/// +/// # Example +/// +/// ```rs +/// // Setup a Step for Foo. +/// let foo_step: Step = Step::new("foo", Foo::init()).register_tag(|foo| { foo.bar().to_string() }); +/// // This returns a Result +/// let foo = foo_step.run()?; +/// ``` +/// +/// # TODOs +/// +/// * If T = Option and None is returned, we print `"[Not found]"`, otherwise +/// `"[Success]`". +pub struct Step { + /// The initialisation function for the Step, usually this is a closure. + init_fn: fn() -> T, + /// A list of tag generator functions to display under the Step on success. + /// They are displayed like: + /// ``` + /// Initialising module... [Success] + /// => Some information here. + /// ``` + /// + /// # TODOs + /// + /// * Support tags without alloc being initialised. + tags: Vec String>>, + /// The name of the module being initialised. + task: &'static str, +} + +// This impl block might be so horrible it needs its own file. +impl Step { + pub fn new( + task: &'static str, + init_fn: fn() -> T, + tags: Vec String>>, + ) -> Self { + Self { + init_fn, + tags, + task, + } + } + + /// Runs the initialisation function and logs where required. + /// + /// This function is a little messy but it just handles pretty printing the + /// [Step] to the terminal for us. + pub fn run(self) -> T { + print_log!(" {}", self.task); + let mut success_position = WRITER.lock().pos(); + success_position.set_x(success_position.x() + 1); + + let ret = (self.init_fn)(); + + println!(); + + for tag in &self.tags[..] { + // Calls the closure on the return value. + println_log!(" => {}", (tag)(ret.clone())); + } + + // Print whether or not the step succeeded. + print_oneshot!( + success_position, + Colour::Green, + Colour::Black, + "[Success]" + ); + + ret + } + + /// Registers a 'tag' or note to display under the line which says + /// `Initialising module... [Success]`. + /// + /// # Notes + /// + /// The tag is a closure taking in a reference to T and returning a + /// presumably formatted String. + #[allow(unused)] + pub fn register_tag(&mut self, tag: Box String>) -> &Self { + self.tags.push(tag); + + self + } +} + */ diff --git a/rustfmt.toml b/rustfmt.toml index f661675..a9d34c6 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -2,3 +2,4 @@ wrap_comments = true max_width = 80 comment_width = 80 format_code_in_doc_comments = true +doc_comment_code_block_width = 80