Added a whole lot of documentation

12 files changed, 1896 insertions, 136 deletions

diff --git a/StrixKernel/Cargo.toml b/StrixKernel/Cargo.toml
index ecdd964..0df1444 100644
--- a/StrixKernel/Cargo.toml
+++ b/StrixKernel/Cargo.toml
@@ -1,46 +1,111 @@
+# =============================================================================
+# Strix OS Kernel - Cargo Configuration
+# =============================================================================
+#
+# This file configures the Strix OS bare-metal kernel build. It includes
+# dependencies for no_std development and bootimage test configuration.
+
 [package]
 name = "strix_os"
 version = "0.1.0"
 edition = "2024"
 
+# =============================================================================
+# Integration Tests
+# =============================================================================
+#
+# Tests marked with `harness = false` manage their own execution flow and
+# don't use the standard Rust test harness. This is necessary for:
+# - Custom panic handlers (should_panic)
+# - Tests that need to control QEMU exit codes (stack_overflow)
+
 [[test]]
 name = "should_panic"
-harness = false
+harness = false  # Uses custom panic handler that signals success on panic
 
 [[test]]
 name = "stack_overflow"
-harness = false
+harness = false  # Uses custom IDT to catch double faults
+
+# =============================================================================
+# Dependencies
+# =============================================================================
 
 [dependencies]
+# Bootloader (v0.9): Creates bootable disk image and provides BootInfo
+# The `map_physical_memory` feature maps all physical memory at a fixed offset,
+# enabling easy physical-to-virtual address translation
 bootloader = { version = "0.9", features = ["map_physical_memory"] }
+
+# Volatile (v0.2.6): Prevents compiler from optimizing away memory-mapped I/O
+# Essential for VGA buffer writes that must not be elided
 volatile = "0.2.6"
+
+# Spin (v0.5.2): Spinlock implementation for no_std environments
+# Used to protect global mutable state (VGA buffer, serial port)
 spin = "0.5.2"
+
+# x86_64 (v0.14.2): CPU structures and instructions for x86-64 architecture
+# Provides: GDT, IDT, TSS, page tables, control registers, port I/O
 x86_64 = "0.14.2"
+
+# UART 16550 (v0.2.0): Serial port driver for debugging output
+# Outputs to COM1 (0x3F8), captured by QEMU's `-serial stdio`
 uart_16550 = "0.2.0"
+
+# PIC 8259 (v0.10.1): Programmable Interrupt Controller driver
+# Remaps hardware IRQs (timer, keyboard) to vectors 32-47
 pic8259 = "0.10.1"
+
+# PC Keyboard (v0.7.0): PS/2 keyboard scancode translation
+# Converts raw scancodes to ASCII characters
 pc-keyboard = "0.7.0"
 
+# Lazy Static (v1.0): Lazily initialized statics for no_std
+# The `spin_no_std` feature uses spinlocks instead of std::sync
 [dependencies.lazy_static]
 version = "1.0"
 features = ["spin_no_std"]
 
+# =============================================================================
+# Binary Configuration
+# =============================================================================
+
 [[bin]]
 name = "strix_os"
-test = true
-bench = false
+test = true   # Include this binary in `cargo test`
+bench = false # Don't include in benchmarks (not supported in no_std)
 
+# =============================================================================
+# Bootimage Configuration
+# =============================================================================
+#
+# The `bootimage` tool creates bootable disk images and runs tests in QEMU.
+# https://github.com/rust-osdev/bootimage
 
 [package.metadata.bootimage]
+# QEMU arguments for running tests
 test-args = [
-    "-device",
-    "isa-debug-exit,iobase=0xf4,iosize=0x04",
-    "-serial",
-    "stdio",
-    "-display",
-    "none",
+    # ISA debug exit device: writing to port 0xf4 exits QEMU
+    # Exit code = (value << 1) | 1, so 0x10 -> 33, 0x11 -> 35
+    "-device", "isa-debug-exit,iobase=0xf4,iosize=0x04",
+    # Serial output to stdio for test results
+    "-serial", "stdio",
+    # Disable graphical window during tests
+    "-display", "none",
 ]
-test-success-exit-code = 33 # (0x10 << 1) | 1
+# QEMU exit code that indicates test success
+# Calculated as: (0x10 << 1) | 1 = 33
+test-success-exit-code = 33
+
+# =============================================================================
+# Bootloader Configuration
+# =============================================================================
+#
+# Configuration passed to the bootloader crate at build time.
 
 [package.metadata.bootloader]
+# Virtual address offset where physical memory is mapped
+# Physical address 0x0 is accessible at virtual address 0x0000256000000000
+# This enables the OffsetPageTable to translate virtual <-> physical addresses
 physical-memory-offset = "0x0000256000000000"
-
diff --git a/StrixKernel/src/gdt.rs b/StrixKernel/src/gdt.rs
index e0c3d71..864dd57 100644
--- a/StrixKernel/src/gdt.rs
+++ b/StrixKernel/src/gdt.rs
@@ -1,54 +1,209 @@
-// src/gdt.rs
+//! # Global Descriptor Table (GDT) Module
+//!
+//! This module sets up the Global Descriptor Table and Task State Segment for the
+//! Strix OS kernel, providing the foundation for memory segmentation and interrupt
+//! handling on x86-64.
+//!
+//! ## x86-64 Background
+//!
+//! ### Global Descriptor Table (GDT)
+//!
+//! The GDT is a legacy x86 structure that was originally used for memory segmentation.
+//! In 64-bit long mode, segmentation is mostly disabled, but the GDT is still required for:
+//!
+//! - **Privilege Level Switching**: The code segment selector determines the Current
+//!   Privilege Level (CPL). Ring 0 (kernel) vs Ring 3 (user) transitions require
+//!   different code segments.
+//! - **Task State Segment (TSS)**: The GDT must contain a TSS descriptor for the
+//!   processor to locate the TSS.
+//!
+//! ### Task State Segment (TSS)
+//!
+//! The TSS in 64-bit mode serves two main purposes:
+//!
+//! 1. **Interrupt Stack Table (IST)**: Up to 7 separate stack pointers that can be
+//!    used by specific interrupts. This is critical for handling double faults,
+//!    which cannot use the normal kernel stack (it might be corrupted or overflowed).
+//!
+//! 2. **I/O Permission Bitmap**: Controls which I/O ports userspace can access
+//!    (not currently used in Strix OS).
+//!
+//! ## Module Structure
+//!
+//! - [`DOUBLE_FAULT_IST_INDEX`]: The IST index (0-6) used for the double fault handler
+//! - [`TSS`]: Static Task State Segment with the double fault stack configured
+//! - [`GDT`]: Static Global Descriptor Table with kernel code segment and TSS descriptor
+//! - [`init()`]: Loads the GDT and TSS into the CPU
+//!
+//! ## Safety Considerations
+//!
+//! The GDT and TSS must remain valid for the lifetime of the kernel. They are
+//! stored in `lazy_static` statics to ensure they have `'static` lifetime.
 
-use x86_64::VirtAddr;
-use x86_64::structures::tss::TaskStateSegment;
 use lazy_static::lazy_static;
+use x86_64::structures::gdt::{Descriptor, GlobalDescriptorTable, SegmentSelector};
+use x86_64::structures::tss::TaskStateSegment;
+use x86_64::VirtAddr;
 
+/// The Interrupt Stack Table index used for the double fault handler.
+///
+/// The IST is an array of 7 stack pointers (indices 0-6) in the TSS. When an
+/// interrupt or exception specifies an IST index in its IDT entry, the CPU
+/// automatically switches to that stack before invoking the handler.
+///
+/// We use index 0 for double faults. This ensures that even if the kernel
+/// stack overflows (causing a page fault that turns into a double fault),
+/// we have a valid stack to handle the exception.
 pub const DOUBLE_FAULT_IST_INDEX: u16 = 0;
 
-pub fn init() {
-    use x86_64::instructions::tables::load_tss;
-    use x86_64::instructions::segmentation::{CS, Segment};
-
-    GDT.0.load();
-
-    unsafe {
-        CS::set_reg(GDT.1.code_selector);
-        load_tss(GDT.1.tss_selector);
-    }
-
-}
-
-
 lazy_static! {
+    /// The Task State Segment for the kernel.
+    ///
+    /// The TSS contains the Interrupt Stack Table (IST), which provides separate
+    /// stacks for specific interrupt handlers. This is essential for handling
+    /// faults that might occur due to stack issues (like stack overflow).
+    ///
+    /// ## IST Entry 0 (Double Fault Stack)
+    ///
+    /// We allocate a 20 KiB stack (5 × 4096 bytes) for handling double faults.
+    /// The stack grows downward on x86-64, so we store the *end* address
+    /// (highest address) in the IST entry.
+    ///
+    /// ## Why a Separate Stack?
+    ///
+    /// Consider what happens during a stack overflow:
+    /// 1. Code pushes to the stack, exceeding the guard page
+    /// 2. CPU triggers a page fault
+    /// 3. CPU tries to push the interrupt frame... but the stack is full!
+    /// 4. CPU triggers a double fault
+    /// 5. CPU tries to push the double fault frame... still no stack!
+    /// 6. CPU triggers a triple fault → system reset
+    ///
+    /// With an IST entry, step 4 switches to a known-good stack, avoiding the
+    /// triple fault.
     static ref TSS: TaskStateSegment = {
         let mut tss = TaskStateSegment::new();
+
+        // Configure IST entry 0 for the double fault handler
         tss.interrupt_stack_table[DOUBLE_FAULT_IST_INDEX as usize] = {
+            // Stack size: 20 KiB (5 pages)
             const STACK_SIZE: usize = 4096 * 5;
+
+            // Allocate the stack as a static mutable array.
+            // SAFETY: This is only accessed here during TSS initialization,
+            // and the stack pointer is stored in the TSS for CPU use only.
             static mut STACK: [u8; STACK_SIZE] = [0; STACK_SIZE];
 
+            // Get the stack boundaries
+            // SAFETY: We're taking a raw pointer to the static, which is safe.
+            // The `&raw const` syntax avoids creating a reference to mutable static.
             let stack_start = VirtAddr::from_ptr(&raw const STACK);
-            let stack_end = stack_start + STACK_SIZE;
+
+            // Stacks grow downward on x86-64, so we need the end (top) address
+            let stack_end = stack_start + STACK_SIZE as u64;
+
             stack_end
         };
+
         tss
     };
 }
 
-use x86_64::structures::gdt::SegmentSelector;
-use x86_64::structures::gdt::GlobalDescriptorTable;
-use x86_64::structures::gdt::Descriptor;
-
 lazy_static! {
+    /// The Global Descriptor Table for the kernel.
+    ///
+    /// The GDT contains segment descriptors that define memory segments and their
+    /// access permissions. In 64-bit long mode, most segmentation features are
+    /// disabled, but we still need:
+    ///
+    /// 1. **Kernel Code Segment**: Required for the CPU to execute code in Ring 0.
+    ///    The segment selector is loaded into the CS register.
+    ///
+    /// 2. **TSS Segment**: A special system segment descriptor that points to the
+    ///    Task State Segment. Required for the CPU to find IST stacks.
+    ///
+    /// The GDT is stored alongside a [`Selectors`] struct containing the segment
+    /// selectors returned when adding entries. These selectors are loaded into
+    /// CPU segment registers during [`init()`].
     static ref GDT: (GlobalDescriptorTable, Selectors) = {
         let mut gdt = GlobalDescriptorTable::new();
+
+        // Add kernel code segment (required for Ring 0 execution)
         let code_selector = gdt.add_entry(Descriptor::kernel_code_segment());
+
+        // Add TSS segment (required for IST stack switching)
         let tss_selector = gdt.add_entry(Descriptor::tss_segment(&TSS));
+
         (gdt, Selectors { code_selector, tss_selector })
     };
 }
 
+/// Segment selectors for the GDT entries.
+///
+/// A segment selector is a 16-bit value containing:
+/// - Bits 0-1: Requested Privilege Level (RPL)
+/// - Bit 2: Table Indicator (0 = GDT, 1 = LDT)
+/// - Bits 3-15: Index into the descriptor table
+///
+/// These selectors are returned by [`GlobalDescriptorTable::add_entry()`] and
+/// must be loaded into the appropriate CPU registers.
 struct Selectors {
+    /// Selector for the kernel code segment.
+    /// Loaded into the CS (Code Segment) register.
     code_selector: SegmentSelector,
+
+    /// Selector for the Task State Segment.
+    /// Loaded via the `ltr` (Load Task Register) instruction.
     tss_selector: SegmentSelector,
 }
+
+/// Initializes and loads the Global Descriptor Table and Task State Segment.
+///
+/// This function must be called early in the kernel boot process, before any
+/// interrupts are enabled. It performs the following steps:
+///
+/// 1. **Load GDT**: Uses the `lgdt` instruction to load the GDT register with
+///    the address and size of our GDT.
+///
+/// 2. **Reload CS Register**: The code segment register must be reloaded after
+///    changing the GDT. This is done with a far jump/return that loads the new
+///    selector.
+///
+/// 3. **Load TSS**: Uses the `ltr` (Load Task Register) instruction to tell the
+///    CPU where to find our TSS. This enables IST stack switching.
+///
+/// # Safety
+///
+/// After this function returns:
+/// - The CPU uses our GDT for all segment lookups
+/// - Interrupts can use IST stacks defined in the TSS
+/// - The GDT and TSS must remain valid for the kernel's lifetime
+///
+/// # Example
+///
+/// ```ignore
+/// // Called during kernel initialization
+/// gdt::init();
+/// interrupts::init_idt();  // IDT can now reference IST entries
+/// ```
+pub fn init() {
+    use x86_64::instructions::segmentation::{Segment, CS};
+    use x86_64::instructions::tables::load_tss;
+
+    // Load the GDT into the GDTR register
+    GDT.0.load();
+
+    // SAFETY: We're loading valid segment selectors from our GDT.
+    // The code_selector points to a valid kernel code segment.
+    // The tss_selector points to our TSS descriptor.
+    unsafe {
+        // Reload the code segment register with our kernel code selector.
+        // This is required because changing the GDT doesn't automatically
+        // update the hidden portion of segment registers.
+        CS::set_reg(GDT.1.code_selector);
+
+        // Load the task register with our TSS selector.
+        // This tells the CPU where to find IST stacks for interrupts.
+        load_tss(GDT.1.tss_selector);
+    }
+}
diff --git a/StrixKernel/src/interrupts.rs b/StrixKernel/src/interrupts.rs
index 9e245cc..ab974ac 100644
--- a/StrixKernel/src/interrupts.rs
+++ b/StrixKernel/src/interrupts.rs
@@ -1,46 +1,183 @@
-// interrupts.rs
+//! # Interrupt Handling Module
+//!
+//! This module sets up the Interrupt Descriptor Table (IDT) and provides handlers
+//! for CPU exceptions and hardware interrupts in the Strix OS kernel.
+//!
+//! ## x86-64 Interrupt Architecture
+//!
+//! ### Interrupt Vectors
+//!
+//! The x86-64 architecture supports 256 interrupt vectors (0-255):
+//!
+//! | Vector Range | Type                          |
+//! |--------------|-------------------------------|
+//! | 0-31         | CPU Exceptions (reserved)     |
+//! | 32-47        | Hardware Interrupts (IRQs)    |
+//! | 48-255       | Software/User-defined         |
+//!
+//! ### Interrupt Descriptor Table (IDT)
+//!
+//! The IDT maps each interrupt vector to a handler function. Each entry contains:
+//! - Handler function address
+//! - Code segment selector
+//! - Privilege level (DPL)
+//! - Interrupt Stack Table (IST) index (optional)
+//! - Gate type (interrupt gate disables interrupts, trap gate doesn't)
+//!
+//! ### 8259 Programmable Interrupt Controller (PIC)
+//!
+//! Legacy hardware uses the 8259 PIC to route hardware interrupts (IRQs) to the CPU.
+//! There are two PICs in a chained configuration:
+//!
+//! - **Primary PIC (PIC1)**: Handles IRQs 0-7 (timer, keyboard, etc.)
+//! - **Secondary PIC (PIC2)**: Handles IRQs 8-15 (RTC, mouse, etc.)
+//!
+//! By default, the PICs map IRQs 0-15 to vectors 0-15, which conflicts with CPU
+//! exceptions. We remap them to vectors 32-47 using [`PIC_1_OFFSET`] and [`PIC_2_OFFSET`].
+//!
+//! ## Implemented Handlers
+//!
+//! ### CPU Exceptions
+//! - **Breakpoint** (Vector 3): Triggered by `int3` instruction, used for debugging
+//! - **Double Fault** (Vector 8): Triggered when an exception occurs while handling
+//!   another exception
+//! - **Page Fault** (Vector 14): Triggered by invalid memory access
+//!
+//! ### Hardware Interrupts (IRQs)
+//! - **Timer** (IRQ 0, Vector 32): Periodic timer interrupt from the PIT
+//! - **Keyboard** (IRQ 1, Vector 33): PS/2 keyboard input
 
 use lazy_static::lazy_static;
-use x86_64::structures::idt::{InterruptDescriptorTable, InterruptStackFrame};
+use pic8259::ChainedPics;
+use spin::Mutex;
+use x86_64::structures::idt::{InterruptDescriptorTable, InterruptStackFrame, PageFaultErrorCode};
 
 use crate::gdt;
+use crate::hlt_loop;
 use crate::print;
 use crate::println;
 
-// EXCPETION INTERURPTS //
+// =============================================================================
+// Interrupt Descriptor Table
+// =============================================================================
 
 lazy_static! {
+    /// The Interrupt Descriptor Table for the kernel.
+    ///
+    /// This table maps interrupt vectors to their handler functions. The IDT is
+    /// loaded into the CPU's IDTR register by [`init_idt()`].
+    ///
+    /// ## Exception Handlers
+    ///
+    /// - **Breakpoint (vector 3)**: Debug breakpoint, non-fatal
+    /// - **Double Fault (vector 8)**: Fatal error, uses IST for stack safety
+    /// - **Page Fault (vector 14)**: Invalid memory access, currently fatal
+    ///
+    /// ## Hardware Interrupt Handlers
+    ///
+    /// - **Timer (vector 32)**: PIT timer tick
+    /// - **Keyboard (vector 33)**: PS/2 keyboard scancode received
     static ref IDT: InterruptDescriptorTable = {
         let mut idt = InterruptDescriptorTable::new();
-        idt.breakpoint.set_handler_fn(breakpoint_handler); // breakpoint exception handler
+
+        // CPU Exception Handlers
+        idt.breakpoint.set_handler_fn(breakpoint_handler);
+
+        // Double fault handler uses IST entry 0 for a separate stack.
+        // This is critical because double faults often occur due to stack
+        // overflow, so we can't use the normal kernel stack.
         unsafe {
-            idt.double_fault.set_handler_fn(double_fault_handler) // Double fault handler (when a fault occurs and the system cannot find the handler for said fault it causes a nother fault thus a double fault)
+            idt.double_fault
+                .set_handler_fn(double_fault_handler)
                 .set_stack_index(gdt::DOUBLE_FAULT_IST_INDEX);
         }
 
-        idt.page_fault.set_handler_fn(page_fault_handler); // Page fault handler (Caused when the kernel writes to non-existing/out of bounds virtual memory)
-
-        idt[InterruptIndex::Timer.as_usize()] // Timer interrupt handler. Sent every frame
-            .set_handler_fn(timer_interrupt_handler);
-
-        idt[InterruptIndex::Keyboard.as_usize()] // Keyboard interupt handler. Sent every time a keyboard input is recieved
-            .set_handler_fn(keyboard_interrupt_handler);
+        idt.page_fault.set_handler_fn(page_fault_handler);
 
+        // Hardware Interrupt Handlers (remapped via PIC)
+        idt[InterruptIndex::Timer.as_usize()].set_handler_fn(timer_interrupt_handler);
+        idt[InterruptIndex::Keyboard.as_usize()].set_handler_fn(keyboard_interrupt_handler);
 
         idt
     };
 }
 
+/// Loads the Interrupt Descriptor Table into the CPU.
+///
+/// This function must be called after [`crate::gdt::init()`] because the IDT
+/// references IST entries in the TSS (which is set up by the GDT initialization).
+///
+/// After calling this function, the CPU will invoke our handlers for the
+/// configured exception and interrupt vectors.
+///
+/// # Example
+///
+/// ```ignore
+/// gdt::init();      // Set up GDT and TSS first
+/// init_idt();       // Now safe to reference IST entries
+/// PICS.lock().initialize();  // Initialize the PICs
+/// x86_64::instructions::interrupts::enable();  // Enable interrupts
+/// ```
 pub fn init_idt() {
     IDT.load();
 }
 
-// Breakpoint exception
+// =============================================================================
+// CPU Exception Handlers
+// =============================================================================
+
+/// Handler for the Breakpoint exception (vector 3).
+///
+/// The breakpoint exception is triggered by the `int3` instruction, which is a
+/// single-byte opcode (0xCC) commonly used by debuggers to set breakpoints.
+///
+/// ## Exception Details
+///
+/// - **Vector**: 3
+/// - **Type**: Trap (RIP points to instruction after `int3`)
+/// - **Error Code**: None
+/// - **Fatal**: No
+///
+/// ## Behavior
+///
+/// This handler prints the interrupt stack frame (showing CPU state at the time
+/// of the breakpoint) and returns, allowing execution to continue.
 extern "x86-interrupt" fn breakpoint_handler(stack_frame: InterruptStackFrame) {
     println!("EXCEPTION: BREAKPOINT\n{:#?}", stack_frame);
 }
 
-// Double fault exception
+/// Handler for the Double Fault exception (vector 8).
+///
+/// A double fault occurs when the CPU encounters an exception while trying to
+/// handle a previous exception. Common causes include:
+///
+/// - Stack overflow: A page fault occurs, but pushing the exception frame
+///   causes another page fault
+/// - Invalid IDT entry: An exception occurs, but its IDT entry is invalid
+/// - Segment not present: An exception handler's code segment is invalid
+///
+/// ## Exception Details
+///
+/// - **Vector**: 8
+/// - **Type**: Abort (RIP may not be valid)
+/// - **Error Code**: Always 0
+/// - **Fatal**: Yes (cannot return)
+/// - **IST**: Uses [`gdt::DOUBLE_FAULT_IST_INDEX`] for a separate stack
+///
+/// ## Stack Safety
+///
+/// This handler is configured to use IST entry 0, which provides a separate
+/// 20 KiB stack. This is critical because:
+///
+/// 1. If a stack overflow caused the double fault, the normal stack is unusable
+/// 2. The IST stack is guaranteed to be valid
+/// 3. Without IST, a double fault during stack overflow would cause a triple
+///    fault (system reset)
+///
+/// ## Behavior
+///
+/// This handler panics with the exception information. The panic handler will
+/// attempt to print the error and halt. If running tests, it exits QEMU.
 extern "x86-interrupt" fn double_fault_handler(
     stack_frame: InterruptStackFrame,
     _error_code: u64,
@@ -48,10 +185,44 @@ extern "x86-interrupt" fn double_fault_handler(
     panic!("EXCEPTION: DOUBLE FAULT\n{:#?}", stack_frame);
 }
 
-// Page fault exception
-use crate::hlt_loop;
-use x86_64::structures::idt::PageFaultErrorCode;
-
+/// Handler for the Page Fault exception (vector 14).
+///
+/// A page fault occurs when code attempts to access memory in a way that
+/// violates the page table permissions. Common causes include:
+///
+/// - Accessing an unmapped (not present) page
+/// - Writing to a read-only page
+/// - Executing code on a non-executable page
+/// - User mode accessing a kernel page
+///
+/// ## Exception Details
+///
+/// - **Vector**: 14
+/// - **Type**: Fault (RIP points to faulting instruction)
+/// - **Error Code**: [`PageFaultErrorCode`] describing the violation
+/// - **Fatal**: Currently yes (no page fault recovery implemented)
+///
+/// ## CR2 Register
+///
+/// When a page fault occurs, the CPU stores the virtual address that caused the
+/// fault in the CR2 register. This handler reads CR2 to display the faulting address.
+///
+/// ## Error Code Flags
+///
+/// The error code contains flags describing the page fault:
+/// - `PROTECTION_VIOLATION`: Page was present but access violated permissions
+/// - `CAUSED_BY_WRITE`: Fault was caused by a write operation
+/// - `USER_MODE`: Fault occurred in user mode (CPL 3)
+/// - `MALFORMED_TABLE`: Reserved bit set in page table entry
+/// - `INSTRUCTION_FETCH`: Fault was caused by an instruction fetch
+///
+/// ## Behavior
+///
+/// Currently, all page faults are considered fatal. The handler prints diagnostic
+/// information and enters a halt loop. In the future, this could be extended to:
+/// - Handle copy-on-write pages
+/// - Implement demand paging
+/// - Map lazily-allocated pages
 extern "x86-interrupt" fn page_fault_handler(
     stack_frame: InterruptStackFrame,
     error_code: PageFaultErrorCode,
@@ -62,83 +233,216 @@ extern "x86-interrupt" fn page_fault_handler(
     println!("Accessed Address: {:?}", Cr2::read());
     println!("Error Code: {:?}", error_code);
     println!("{:#?}", stack_frame);
+
+    // Currently fatal - halt the CPU
     hlt_loop();
 }
 
-// HARDWARE INTERRUPTS //
-use pic8259::ChainedPics;
-use spin;
+// =============================================================================
+// Hardware Interrupts (8259 PIC)
+// =============================================================================
 
+/// Base interrupt vector for the primary PIC (PIC1).
+///
+/// The 8259 PIC by default maps IRQs 0-7 to interrupt vectors 0-7, which
+/// conflicts with CPU exceptions. We remap PIC1 to start at vector 32.
+///
+/// IRQ mapping after remapping:
+/// - IRQ 0 (Timer) → Vector 32
+/// - IRQ 1 (Keyboard) → Vector 33
+/// - IRQ 2 (Cascade to PIC2) → Vector 34
+/// - ...
+/// - IRQ 7 (Parallel Port) → Vector 39
 pub const PIC_1_OFFSET: u8 = 32;
+
+/// Base interrupt vector for the secondary PIC (PIC2).
+///
+/// PIC2 handles IRQs 8-15, mapped to vectors 40-47:
+/// - IRQ 8 (RTC) → Vector 40
+/// - IRQ 9 (ACPI) → Vector 41
+/// - ...
+/// - IRQ 15 (Secondary ATA) → Vector 47
 pub const PIC_2_OFFSET: u8 = PIC_1_OFFSET + 8;
 
-pub static PICS: spin::Mutex<ChainedPics> =
-    spin::Mutex::new(unsafe { ChainedPics::new(PIC_1_OFFSET, PIC_2_OFFSET) });
+/// The chained 8259 Programmable Interrupt Controllers.
+///
+/// This static provides access to the primary and secondary PICs, which are
+/// responsible for routing hardware interrupts to the CPU. The PICs must be:
+///
+/// 1. Initialized via [`ChainedPics::initialize()`] after loading the IDT
+/// 2. Notified after each interrupt via [`ChainedPics::notify_end_of_interrupt()`]
+///
+/// ## Thread Safety
+///
+/// The PICs are wrapped in a [`spin::Mutex`] because they may be accessed from
+/// both interrupt handlers and normal code. The spinlock ensures safe access
+/// without requiring heap-allocated synchronization primitives.
+///
+/// ## SAFETY
+///
+/// The `ChainedPics::new()` call is unsafe because incorrect offsets could
+/// cause the PICs to overlap with CPU exception vectors, leading to undefined
+/// behavior when hardware interrupts fire.
+pub static PICS: Mutex<ChainedPics> =
+    Mutex::new(unsafe { ChainedPics::new(PIC_1_OFFSET, PIC_2_OFFSET) });
 
+/// Enumeration of hardware interrupt indices.
+///
+/// This enum maps IRQ numbers to their corresponding interrupt vectors after
+/// PIC remapping. The values are used to:
+///
+/// 1. Index into the IDT to set handler functions
+/// 2. Acknowledge interrupts via the PIC's EOI (End of Interrupt) command
+///
+/// ## Available Interrupts
+///
+/// - [`Timer`](InterruptIndex::Timer): IRQ 0, fires ~18.2 times per second
+/// - [`Keyboard`](InterruptIndex::Keyboard): IRQ 1, fires on PS/2 keyboard events
 #[derive(Debug, Clone, Copy)]
 #[repr(u8)]
 pub enum InterruptIndex {
+    /// Timer interrupt (IRQ 0).
+    ///
+    /// The Programmable Interval Timer (PIT) fires this interrupt at a regular
+    /// interval (default ~18.2 Hz, or roughly every 55ms). This is used for:
+    /// - Timekeeping
+    /// - Preemptive multitasking (when implemented)
+    /// - Periodic system maintenance
     Timer = PIC_1_OFFSET,
+
+    /// Keyboard interrupt (IRQ 1).
+    ///
+    /// The PS/2 keyboard controller fires this interrupt when:
+    /// - A key is pressed (generates a "make" scancode)
+    /// - A key is released (generates a "break" scancode)
+    ///
+    /// The scancode must be read from I/O port 0x60.
     Keyboard,
 }
 
 impl InterruptIndex {
+    /// Converts the interrupt index to its raw `u8` vector number.
     fn as_u8(self) -> u8 {
         self as u8
     }
 
+    /// Converts the interrupt index to `usize` for IDT indexing.
     fn as_usize(self) -> usize {
         usize::from(self.as_u8())
     }
 }
 
+/// Handler for the Timer interrupt (IRQ 0, vector 32).
+///
+/// This handler is invoked approximately 18.2 times per second by the
+/// Programmable Interval Timer (PIT). Currently, it does nothing except
+/// acknowledge the interrupt.
+///
+/// ## Future Uses
+///
+/// - Increment a system tick counter
+/// - Trigger context switches for preemptive multitasking
+/// - Wake sleeping processes
+///
+/// ## End of Interrupt (EOI)
+///
+/// Every hardware interrupt handler MUST send an EOI signal to the PIC.
+/// Failure to do so will cause the PIC to stop sending that interrupt
+/// (and any lower-priority interrupts).
 extern "x86-interrupt" fn timer_interrupt_handler(_stack_frame: InterruptStackFrame) {
-    //print!(".");
+    // Timer tick - currently a no-op
+    // Uncomment the following line to see timer activity:
+    // print!(".");
 
+    // SAFETY: We must notify the PIC that we've handled the interrupt.
+    // This allows the PIC to send future interrupts of this type.
     unsafe {
         PICS.lock()
             .notify_end_of_interrupt(InterruptIndex::Timer.as_u8());
     }
 }
 
+/// Handler for the Keyboard interrupt (IRQ 1, vector 33).
+///
+/// This handler is invoked when the PS/2 keyboard controller has a scancode
+/// ready to be read. The scancode is read from I/O port 0x60 and translated
+/// to a character using the `pc-keyboard` crate.
+///
+/// ## Scancode Sets
+///
+/// PC keyboards use "scancode sets" to encode key events:
+/// - **Set 1**: Original XT scancodes (most common, used here)
+/// - **Set 2**: AT scancodes (default on AT keyboards, often translated to Set 1)
+/// - **Set 3**: PS/2 scancodes (rarely used)
+///
+/// ## Key Event Processing
+///
+/// 1. Read raw scancode from port 0x60
+/// 2. Feed scancode to the keyboard decoder
+/// 3. If a complete key event is decoded, process it
+/// 4. If the key event produces a character, print it
+///
+/// ## Thread Safety
+///
+/// The keyboard state machine ([`pc_keyboard::Keyboard`]) is wrapped in a
+/// spinlock because this interrupt handler may fire at any time.
 extern "x86-interrupt" fn keyboard_interrupt_handler(_stack_frame: InterruptStackFrame) {
-    use pc_keyboard::{DecodedKey, HandleControl, Keyboard, ScancodeSet1, layouts};
-    use spin::Mutex;
+    use pc_keyboard::{layouts, DecodedKey, HandleControl, Keyboard, ScancodeSet1};
     use x86_64::instructions::port::Port;
 
+    // Static keyboard state machine with US 104-key layout
     lazy_static! {
+        /// The PS/2 keyboard decoder and state machine.
+        ///
+        /// This maintains the state needed to decode multi-byte scancodes and
+        /// track modifier key states (Shift, Ctrl, Alt).
         static ref KEYBOARD: Mutex<Keyboard<layouts::Us104Key, ScancodeSet1>> =
             Mutex::new(Keyboard::new(
                 ScancodeSet1::new(),
                 layouts::Us104Key,
-                HandleControl::Ignore
+                HandleControl::Ignore  // Don't handle Ctrl+letter as control characters
             ));
     }
 
     let mut keyboard = KEYBOARD.lock();
-    let mut port = Port::new(0x60);
 
+    // Read the scancode from the PS/2 data port
+    let mut port = Port::new(0x60);
     let scancode: u8 = unsafe { port.read() };
+
+    // Feed the scancode to the decoder
     if let Ok(Some(key_event)) = keyboard.add_byte(scancode) {
+        // Process the key event (handles make/break and modifier tracking)
         if let Some(key) = keyboard.process_keyevent(key_event) {
             match key {
+                // Printable character - display it
                 DecodedKey::Unicode(character) => print!("{}", character),
-                DecodedKey::RawKey(_key) => {} //print!("{:?}", key), // Do not print controller key debug names like shift ctrl and super
-                                               // TODO: Add support for backspace
+                // Non-printable key (Shift, Ctrl, arrows, etc.) - ignore for now
+                // TODO: Handle special keys like backspace, arrow keys, etc.
+                DecodedKey::RawKey(_key) => {}
             }
         }
     }
 
+    // Acknowledge the interrupt to the PIC
+    // SAFETY: Required to re-enable keyboard interrupts
     unsafe {
         PICS.lock()
             .notify_end_of_interrupt(InterruptIndex::Keyboard.as_u8());
     }
 }
 
-// TESTS //
+// =============================================================================
+// Tests
+// =============================================================================
+
+/// Test that the breakpoint exception handler works correctly.
+///
+/// This test triggers a breakpoint exception using the `int3` instruction and
+/// verifies that the handler returns (doesn't crash the system). The breakpoint
+/// exception is a "trap", meaning it returns to the next instruction.
 #[test_case]
 fn test_breakpoint_exception() {
-    // invoke a breakpoint exception
+    // Invoke a breakpoint exception - should return normally
     x86_64::instructions::interrupts::int3();
 }
-
diff --git a/StrixKernel/src/lib.rs b/StrixKernel/src/lib.rs
index 445ee70..e3bcb7e 100644
--- a/StrixKernel/src/lib.rs
+++ b/StrixKernel/src/lib.rs
@@ -1,3 +1,35 @@
+//! # Strix OS Kernel Library
+//!
+//! This crate provides the core functionality for the Strix OS kernel, a bare-metal
+//! x86-64 operating system written in Rust. The kernel runs without the standard library
+//! (`no_std`) and provides its own implementations of essential OS primitives.
+//!
+//! ## Architecture Overview
+//!
+//! The kernel is organized into the following modules:
+//!
+//! - [`gdt`]: Global Descriptor Table and Task State Segment setup
+//! - [`interrupts`]: Interrupt Descriptor Table and interrupt handlers
+//! - [`memory`]: Page table management and frame allocation
+//! - [`serial`]: Serial port communication for debugging
+//! - [`vga_buffer`]: VGA text mode output for on-screen display
+//!
+//! ## Initialization
+//!
+//! The kernel initialization sequence is handled by [`init()`], which must be called
+//! early in the boot process. This function sets up:
+//!
+//! 1. The GDT with a kernel code segment and TSS
+//! 2. The IDT with exception and hardware interrupt handlers
+//! 3. The 8259 PIC for hardware interrupt routing
+//! 4. CPU interrupt enable flag
+//!
+//! ## Testing Framework
+//!
+//! This crate provides a custom test framework for running tests in the bare-metal
+//! environment. Tests are executed in QEMU and output results via the serial port.
+//! See [`Testable`] and [`test_runner()`] for details.
+
 #![no_std]
 #![cfg_attr(test, no_main)]
 #![feature(custom_test_frameworks)]
@@ -13,13 +45,69 @@ pub mod memory;
 pub mod serial;
 pub mod vga_buffer;
 
+/// Initializes the kernel's core subsystems.
+///
+/// This function must be called early in the boot process before any interrupts
+/// can be handled. It performs the following initialization steps in order:
+///
+/// 1. **GDT Initialization** ([`gdt::init()`]): Sets up the Global Descriptor Table
+///    with kernel code segment and Task State Segment (TSS). The TSS is required
+///    for handling interrupts that need a separate stack (e.g., double faults).
+///
+/// 2. **IDT Initialization** ([`interrupts::init_idt()`]): Loads the Interrupt
+///    Descriptor Table with handlers for CPU exceptions (breakpoint, page fault,
+///    double fault) and hardware interrupts (timer, keyboard).
+///
+/// 3. **PIC Initialization**: Initializes the 8259 Programmable Interrupt Controllers
+///    (primary and secondary) which route hardware interrupts to the CPU. The PICs
+///    are remapped to interrupt vectors 32-47 to avoid conflicts with CPU exceptions.
+///
+/// 4. **Enable Interrupts**: Sets the CPU's interrupt flag (IF) to allow the processor
+///    to respond to hardware interrupts.
+///
+/// # Panics
+///
+/// This function does not panic directly, but improper use (e.g., calling without
+/// proper bootloader setup) may result in undefined behavior.
+///
+/// # Example
+///
+/// ```ignore
+/// fn kernel_main(boot_info: &'static BootInfo) -> ! {
+///     strix_os::init();
+///     // Kernel is now ready to handle interrupts
+///     strix_os::hlt_loop();
+/// }
+/// ```
 pub fn init() {
     gdt::init();
     interrupts::init_idt();
+    // SAFETY: PICs must be initialized after IDT is loaded.
+    // The PICS static is protected by a spinlock for safe concurrent access.
     unsafe { interrupts::PICS.lock().initialize() };
     x86_64::instructions::interrupts::enable();
 }
+
+/// A trait for types that can be run as tests in the kernel test framework.
+///
+/// This trait is automatically implemented for all types that implement `Fn()`,
+/// allowing any zero-argument closure or function to be used as a test case.
+///
+/// The trait provides a [`run()`](Testable::run) method that:
+/// 1. Prints the test name to the serial port
+/// 2. Executes the test
+/// 3. Prints "[ok]" if the test completes without panicking
+///
+/// # Example
+///
+/// ```ignore
+/// #[test_case]
+/// fn my_test() {
+///     assert_eq!(2 + 2, 4);
+/// }
+/// ```
 pub trait Testable {
+    /// Runs the test and reports results to the serial port.
     fn run(&self) -> ();
 }
 
@@ -28,12 +116,37 @@ where
     T: Fn(),
 {
     fn run(&self) {
+        // Print the fully-qualified test name using Rust's type_name intrinsic
         serial_print!("{}...\t", core::any::type_name::<T>());
         self();
         serial_println!("[ok]");
     }
 }
 
+/// The test runner for the kernel's custom test framework.
+///
+/// This function is called by the test harness with a slice of all test cases
+/// marked with `#[test_case]`. It iterates through each test, runs it, and
+/// exits QEMU with a success code if all tests pass.
+///
+/// # Arguments
+///
+/// * `tests` - A slice of trait objects implementing [`Testable`]
+///
+/// # Test Output
+///
+/// Test results are written to the serial port (COM1) in the format:
+/// ```text
+/// Running 3 tests
+/// test_name_1...  [ok]
+/// test_name_2...  [ok]
+/// test_name_3...  [ok]
+/// ```
+///
+/// # Exit Behavior
+///
+/// After all tests pass, this function calls [`exit_qemu()`] with
+/// [`QemuExitCode::Success`] to signal test completion to the host.
 pub fn test_runner(tests: &[&dyn Testable]) {
     serial_println!("Running {} tests", tests.len());
     for test in tests {
@@ -42,6 +155,19 @@ pub fn test_runner(tests: &[&dyn Testable]) {
     exit_qemu(QemuExitCode::Success);
 }
 
+/// Handles panics during test execution.
+///
+/// This function is called when a test panics. It prints the failure message
+/// to the serial port and exits QEMU with a failure code.
+///
+/// # Arguments
+///
+/// * `info` - Panic information including the panic message and location
+///
+/// # Exit Behavior
+///
+/// Calls [`exit_qemu()`] with [`QemuExitCode::Failed`], then enters an
+/// infinite halt loop as a fallback (the QEMU exit should terminate first).
 pub fn test_panic_handler(info: &PanicInfo) -> ! {
     serial_println!("[failed]\n");
     serial_println!("Error: {}\n", info);
@@ -49,35 +175,117 @@ pub fn test_panic_handler(info: &PanicInfo) -> ! {
     hlt_loop();
 }
 
+/// Exit codes for communicating test results to QEMU.
+///
+/// These codes are written to the QEMU debug exit device (port 0xf4) to
+/// signal test completion. QEMU is configured to translate these into
+/// process exit codes via the `isa-debug-exit` device.
+///
+/// # QEMU Configuration
+///
+/// The bootimage runner configures QEMU with:
+/// ```text
+/// -device isa-debug-exit,iobase=0xf4,iosize=0x04
+/// ```
+///
+/// QEMU computes the actual exit code as `(value << 1) | 1`, so:
+/// - `Success` (0x10) → QEMU exit code 33
+/// - `Failed` (0x11) → QEMU exit code 35
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[repr(u32)]
 pub enum QemuExitCode {
+    /// Indicates all tests passed successfully.
     Success = 0x10,
+    /// Indicates one or more tests failed.
     Failed = 0x11,
 }
 
+/// Exits QEMU with the specified exit code.
+///
+/// This function writes to the QEMU debug exit device at I/O port 0xf4.
+/// The device is configured via QEMU's `-device isa-debug-exit` option.
+///
+/// # Arguments
+///
+/// * `exit_code` - The exit code to report to QEMU
+///
+/// # Safety
+///
+/// This function performs raw port I/O, which is only safe when:
+/// - Running in QEMU with the `isa-debug-exit` device configured
+/// - Port 0xf4 is not used by other hardware
+///
+/// # Example
+///
+/// ```ignore
+/// // Signal successful test completion
+/// exit_qemu(QemuExitCode::Success);
+/// ```
 pub fn exit_qemu(exit_code: QemuExitCode) {
     use x86_64::instructions::port::Port;
 
+    // SAFETY: We write to the QEMU debug exit device.
+    // This is safe because the port is specifically reserved for this purpose
+    // in our QEMU configuration.
     unsafe {
         let mut port = Port::new(0xf4);
         port.write(exit_code as u32);
     }
 }
 
+/// Enters an infinite loop that halts the CPU between interrupts.
+///
+/// This function is used as the kernel's idle loop. The `hlt` instruction
+/// puts the CPU into a low-power state until the next interrupt arrives,
+/// which is more efficient than a busy-wait loop.
+///
+/// # x86-64 HLT Instruction
+///
+/// The `hlt` (halt) instruction stops instruction execution and places the
+/// processor in a halt state. The processor leaves the halt state upon:
+/// - A non-maskable interrupt (NMI)
+/// - A System Management Interrupt (SMI)
+/// - A hardware interrupt (if interrupts are enabled)
+/// - A debug exception
+///
+/// # Returns
+///
+/// This function never returns (`-> !`).
+///
+/// # Example
+///
+/// ```ignore
+/// fn kernel_main(boot_info: &'static BootInfo) -> ! {
+///     strix_os::init();
+///     println!("Kernel initialized");
+///     strix_os::hlt_loop(); // Wait for interrupts forever
+/// }
+/// ```
 pub fn hlt_loop() -> ! {
     loop {
         x86_64::instructions::hlt();
     }
 }
 
+// ============================================================================
+// Test-only code
+// ============================================================================
+
 #[cfg(test)]
 use bootloader::{BootInfo, entry_point};
 
 #[cfg(test)]
 entry_point!(test_kernel_main);
 
-/// Entry point for `cargo xtest`
+/// Entry point for running library tests via `cargo test`.
+///
+/// This function is only compiled when running tests. It initializes the kernel
+/// subsystems and then invokes the test harness main function generated by
+/// `#[reexport_test_harness_main = "test_main"]`.
+///
+/// # Arguments
+///
+/// * `_boot_info` - Boot information from the bootloader (unused in tests)
 #[cfg(test)]
 fn test_kernel_main(_boot_info: &'static BootInfo) -> ! {
     init();
@@ -85,8 +293,11 @@ fn test_kernel_main(_boot_info: &'static BootInfo) -> ! {
     hlt_loop();
 }
 
+/// Panic handler for test mode.
+///
+/// Delegates to [`test_panic_handler()`] to report the failure and exit QEMU.
 #[cfg(test)]
 #[panic_handler]
 fn panic(info: &PanicInfo) -> ! {
     test_panic_handler(info)
-}
\ No newline at end of file
+}
diff --git a/StrixKernel/src/main.rs b/StrixKernel/src/main.rs
index 0b4cca4..7351a4e 100644
--- a/StrixKernel/src/main.rs
+++ b/StrixKernel/src/main.rs
@@ -1,31 +1,97 @@
+//! # Strix OS Kernel Entry Point
+//!
+//! This module contains the main entry point for the Strix OS kernel. The kernel
+//! is a bare-metal x86-64 operating system that boots via the `bootloader` crate.
+//!
+//! ## Boot Process
+//!
+//! 1. The bootloader loads the kernel ELF image into memory
+//! 2. The bootloader sets up identity-mapped page tables and a physical memory mapping
+//! 3. The bootloader jumps to `kernel_main` with a [`BootInfo`] struct containing:
+//!    - Physical memory offset (where all physical memory is mapped)
+//!    - Memory map describing available/reserved RAM regions
+//! 4. `kernel_main` initializes kernel subsystems and enters the idle loop
+//!
+//! ## Attributes
+//!
+//! - `#![no_std]`: No standard library (bare-metal environment)
+//! - `#![no_main]`: Custom entry point instead of `main()`
+//! - `#![feature(custom_test_frameworks)]`: Custom test runner for bare-metal testing
+
 #![no_std]
 #![no_main]
 #![feature(custom_test_frameworks)]
 #![test_runner(strix_os::test_runner)]
 #![reexport_test_harness_main = "test_main"]
 
-use strix_os::println;
+use bootloader::{entry_point, BootInfo};
 use core::panic::PanicInfo;
-use bootloader::{BootInfo, entry_point};
+use strix_os::println;
 
+// Register kernel_main as the entry point.
+// The bootloader crate uses this macro to generate the actual entry point code
+// with the correct calling convention and stack setup.
 entry_point!(kernel_main);
-fn kernel_main(boot_info: &'static BootInfo) -> ! {
 
+/// The main entry point of the Strix OS kernel.
+///
+/// This function is called by the bootloader after setting up the initial
+/// environment. It receives boot information containing the memory map and
+/// physical memory offset.
+///
+/// # Arguments
+///
+/// * `boot_info` - A reference to [`BootInfo`] provided by the bootloader, containing:
+///   - `memory_map`: Regions of physical memory and their types (usable, reserved, etc.)
+///   - `physical_memory_offset`: Virtual address where physical memory is mapped
+///   - Other boot-time information
+///
+/// # Boot Sequence
+///
+/// 1. Print boot messages to VGA buffer
+/// 2. Initialize kernel subsystems via [`strix_os::init()`]:
+///    - Global Descriptor Table (GDT)
+///    - Interrupt Descriptor Table (IDT)
+///    - Programmable Interrupt Controllers (PICs)
+/// 3. Run tests if compiled in test mode
+/// 4. Enter the idle halt loop
+///
+/// # Returns
+///
+/// This function never returns (`-> !`). The kernel runs indefinitely,
+/// processing interrupts in [`strix_os::hlt_loop()`].
+fn kernel_main(boot_info: &'static BootInfo) -> ! {
+    // Display boot messages on the VGA text buffer
     println!("Hello World{}", "!");
     println!("The Strix OS kernel is now online");
 
-    strix_os::init(); // Call the init function as declared in ./lib.rs
-
+    // Initialize all kernel subsystems (GDT, IDT, PICs, enable interrupts)
+    strix_os::init();
 
-    // Continue as normal
+    // In test mode, run the test harness instead of normal operation
     #[cfg(test)]
     test_main();
 
     println!("It did not crash{}", "!");
+
+    // Enter the idle loop - the kernel will respond to interrupts from here
     strix_os::hlt_loop();
 }
 
-/// This function is called on panic.
+/// Panic handler for normal (non-test) kernel execution.
+///
+/// This function is called when a panic occurs during normal kernel operation.
+/// It prints the panic information to the VGA buffer and halts the CPU.
+///
+/// # Arguments
+///
+/// * `info` - Panic information including the panic message and source location
+///
+/// # Behavior
+///
+/// Unlike the test panic handler, this does not exit QEMU. Instead, it:
+/// 1. Prints the panic info to the screen
+/// 2. Enters an infinite halt loop (the system is unrecoverable)
 #[cfg(not(test))]
 #[panic_handler]
 fn panic(info: &PanicInfo) -> ! {
@@ -33,13 +99,21 @@ fn panic(info: &PanicInfo) -> ! {
     strix_os::hlt_loop();
 }
 
+/// Panic handler for test mode.
+///
+/// Delegates to the library's [`strix_os::test_panic_handler()`] which
+/// prints the failure to the serial port and exits QEMU with a failure code.
 #[cfg(test)]
 #[panic_handler]
 fn panic(info: &PanicInfo) -> ! {
     strix_os::test_panic_handler(info)
 }
 
+/// A trivial test to verify the test framework is working.
+///
+/// This test simply asserts that 1 equals 1. If the test framework is
+/// functioning correctly, this test should always pass.
 #[test_case]
 fn trivial_assertion() {
     assert_eq!(1, 1);
-}
\ No newline at end of file
+}
diff --git a/StrixKernel/src/memory.rs b/StrixKernel/src/memory.rs
index f7264bf..d3a1972 100644
--- a/StrixKernel/src/memory.rs
+++ b/StrixKernel/src/memory.rs
@@ -1,63 +1,276 @@
+//! # Memory Management Module
+//!
+//! This module provides memory management primitives for the Strix OS kernel,
+//! including page table access and physical frame allocation.
+//!
+//! ## x86-64 Paging Overview
+//!
+//! x86-64 uses a 4-level page table hierarchy to translate virtual addresses
+//! to physical addresses:
+//!
+//! ```text
+//! Virtual Address (48-bit canonical):
+//! ┌─────────┬─────────┬─────────┬─────────┬─────────┬──────────────┐
+//! │ Sign    │ Level 4 │ Level 3 │ Level 2 │ Level 1 │    Offset    │
+//! │ Extend  │  Index  │  Index  │  Index  │  Index  │   (12 bits)  │
+//! │(16 bits)│ (9 bits)│ (9 bits)│ (9 bits)│ (9 bits)│              │
+//! └─────────┴─────────┴─────────┴─────────┴─────────┴──────────────┘
+//!
+//! Translation:
+//! CR3 Register → Level 4 Table → Level 3 Table → Level 2 Table → Level 1 Table → Physical Frame
+//! ```
+//!
+//! ### Page Table Levels
+//!
+//! | Level | Also Called | Each Entry Maps |
+//! |-------|-------------|-----------------|
+//! | 4     | PML4        | 512 GiB         |
+//! | 3     | PDPT        | 1 GiB           |
+//! | 2     | PD          | 2 MiB           |
+//! | 1     | PT          | 4 KiB (page)    |
+//!
+//! ### Physical Memory Mapping
+//!
+//! The bootloader maps all physical memory to virtual addresses starting at a
+//! fixed offset. This allows the kernel to access any physical address by adding
+//! this offset:
+//!
+//! ```text
+//! Virtual Address = Physical Address + physical_memory_offset
+//! ```
+//!
+//! This mapping is essential because page tables contain *physical* addresses,
+//! but the CPU can only access *virtual* addresses.
+//!
+//! ## Module Components
+//!
+//! - [`init()`]: Creates an [`OffsetPageTable`] for virtual memory management
+//! - [`BootInfoFrameAllocator`]: Allocates physical frames from the memory map
+//! - [`EmptyFrameAllocator`]: A no-op allocator for testing
+
 use bootloader::bootinfo::{MemoryMap, MemoryRegionType};
 use x86_64::{
+    structures::paging::{FrameAllocator, OffsetPageTable, PageTable, PhysFrame, Size4KiB},
     PhysAddr, VirtAddr,
-    structures::paging::{
-        FrameAllocator, Mapper, OffsetPageTable, Page, PageTable, PhysFrame, Size4KiB,
-    },
 };
 
-/// Initialize a new OffsetPageTable.
+/// Initializes the page table interface.
+///
+/// Creates an [`OffsetPageTable`] that can be used for virtual memory operations
+/// like mapping pages and translating addresses.
+///
+/// ## Physical Memory Offset
+///
+/// The bootloader maps all of physical memory to a contiguous region in virtual
+/// memory. The `physical_memory_offset` is the virtual address where physical
+/// address 0 is mapped. For example, if the offset is `0xFFFF_8000_0000_0000`:
+///
+/// - Physical `0x1000` → Virtual `0xFFFF_8000_0000_1000`
+/// - Physical `0x2000` → Virtual `0xFFFF_8000_0000_2000`
+///
+/// ## How It Works
+///
+/// 1. Read the CR3 register to get the physical address of the level 4 page table
+/// 2. Add the physical memory offset to get the virtual address
+/// 3. Create an `OffsetPageTable` that uses this offset for all translations
+///
+/// # Arguments
+///
+/// * `physical_memory_offset` - The virtual address where physical memory starts
+///
+/// # Returns
 ///
-/// This function is unsafe because the caller must guarantee that the
-/// complete physical memory is mapped to virtual memory at the passed
-/// `physical_memory_offset`. Also, this function must be only called once
-/// to avoid aliasing `&mut` references (which is undefined behavior).
+/// An [`OffsetPageTable`] that can translate virtual ↔ physical addresses and
+/// modify page table mappings.
+///
+/// # Safety
+///
+/// This function is unsafe because:
+///
+/// 1. The caller must ensure that the complete physical memory is mapped to
+///    virtual memory at the passed `physical_memory_offset`.
+///
+/// 2. This function must only be called once. Calling it multiple times would
+///    create multiple `&mut` references to the page tables, which is undefined
+///    behavior.
+///
+/// # Example
+///
+/// ```ignore
+/// let phys_mem_offset = VirtAddr::new(boot_info.physical_memory_offset);
+/// let mut mapper = unsafe { memory::init(phys_mem_offset) };
+///
+/// // Now you can use mapper to translate addresses or create mappings
+/// let phys = mapper.translate_addr(VirtAddr::new(0x1000));
+/// ```
 pub unsafe fn init(physical_memory_offset: VirtAddr) -> OffsetPageTable<'static> {
+    // SAFETY: Caller guarantees that physical memory is mapped at the offset
+    // and that this function is only called once.
     unsafe {
         let level_4_table = active_level_4_table(physical_memory_offset);
         OffsetPageTable::new(level_4_table, physical_memory_offset)
     }
 }
 
-/// Returns a mutable reference to the active level 4 table.
+/// Returns a mutable reference to the active level 4 page table.
+///
+/// This function reads the CR3 register to find the physical address of the
+/// currently active level 4 page table (PML4), then converts it to a virtual
+/// address using the physical memory offset.
+///
+/// ## CR3 Register
 ///
-/// This function is unsafe because the caller must guarantee that the
-/// complete physical memory is mapped to virtual memory at the passed
-/// `physical_memory_offset`. Also, this function must be only called once
-/// to avoid aliasing `&mut` references (which is undefined behavior).
+/// The CR3 (Control Register 3) contains:
+/// - Bits 12-51: Physical address of the level 4 page table (4 KiB aligned)
+/// - Bits 3-4: PCID (Process Context Identifier) flags
+/// - Bit 63: PCIDE (PCID Enable) flag
+///
+/// ## Why Level 4?
+///
+/// x86-64 uses a 4-level page table hierarchy. The level 4 table (PML4) is the
+/// root of this hierarchy and is pointed to by CR3. Each level 4 entry covers
+/// 512 GiB of virtual address space.
+///
+/// # Arguments
+///
+/// * `physical_memory_offset` - The virtual address where physical memory starts
+///
+/// # Returns
+///
+/// A mutable reference to the active level 4 page table.
+///
+/// # Safety
+///
+/// This function is unsafe because:
+///
+/// 1. The caller must ensure that physical memory is mapped at the given offset.
+///
+/// 2. This function must only be called once to avoid creating multiple
+///    mutable references to the same page table (undefined behavior).
 unsafe fn active_level_4_table(physical_memory_offset: VirtAddr) -> &'static mut PageTable {
     use x86_64::registers::control::Cr3;
 
-    let (level_4_table_frame, _) = Cr3::read();
+    // Read CR3 to get the physical address of the level 4 page table
+    let (level_4_table_frame, _flags) = Cr3::read();
 
+    // Convert the frame's physical start address to a virtual address
     let phys = level_4_table_frame.start_address();
     let virt = physical_memory_offset + phys.as_u64();
+
+    // Convert to a raw pointer and dereference
     let page_table_ptr: *mut PageTable = virt.as_mut_ptr();
 
+    // SAFETY: Caller guarantees this is called only once and physical memory
+    // is correctly mapped.
     unsafe { &mut *page_table_ptr }
 }
 
-/// A FrameAllocator that always returns `None`.
+/// A frame allocator that always fails to allocate.
+///
+/// This allocator is useful for testing scenarios where frame allocation
+/// should not occur, or as a placeholder before a real allocator is available.
+///
+/// ## Use Cases
+///
+/// - Testing page table code without actual memory allocation
+/// - Verifying that code handles allocation failures gracefully
+/// - Early boot before the memory map is available
+///
+/// # Example
+///
+/// ```ignore
+/// let mut allocator = EmptyFrameAllocator;
+/// assert!(allocator.allocate_frame().is_none());
+/// ```
 pub struct EmptyFrameAllocator;
 
+/// Safety: This allocator never returns a frame, so there's no risk of
+/// returning an invalid frame.
 unsafe impl FrameAllocator<Size4KiB> for EmptyFrameAllocator {
+    /// Always returns `None`, indicating allocation failure.
     fn allocate_frame(&mut self) -> Option<PhysFrame> {
         None
     }
 }
 
-/// A FrameAllocator that returns usable frames from the bootloader's memory map.
+/// A frame allocator that uses the bootloader's memory map.
+///
+/// This allocator provides physical memory frames (4 KiB each) from regions
+/// marked as "Usable" in the memory map. It's a simple bump allocator that
+/// never frees frames.
+///
+/// ## Memory Map Regions
+///
+/// The bootloader provides a memory map describing physical memory:
+///
+/// | Region Type       | Description                          |
+/// |-------------------|--------------------------------------|
+/// | `Usable`          | Available for kernel use ✓           |
+/// | `InUse`           | Used by bootloader/kernel code       |
+/// | `Reserved`        | Reserved by hardware/firmware        |
+/// | `AcpiReclaimable` | ACPI tables (usable after parsing)   |
+/// | `AcpiNvs`         | ACPI non-volatile storage            |
+/// | `BadMemory`       | Faulty memory regions                |
+/// | `BootloaderReclaimable` | Usable after bootloader cleanup |
+///
+/// ## Allocation Strategy
+///
+/// This is a simple bump allocator:
+/// 1. Maintain an index into the usable frames
+/// 2. On each allocation, return the frame at `index` and increment
+/// 3. Never free frames (no deallocation support)
+///
+/// ## Limitations
+///
+/// - **No Deallocation**: Frames cannot be freed. This is acceptable for early
+///   boot but will eventually need to be replaced with a proper allocator.
+/// - **Linear Scan**: Each allocation iterates through the memory map to find
+///   the nth usable frame, which is O(n).
+/// - **No Fragmentation Handling**: Does not coalesce or reuse freed memory.
 pub struct BootInfoFrameAllocator {
+    /// The bootloader-provided memory map.
     memory_map: &'static MemoryMap,
+
+    /// Index of the next frame to allocate.
+    /// This is incremented after each successful allocation.
     next: usize,
 }
 
 impl BootInfoFrameAllocator {
-    /// Create a FrameAllocator from the passed memory map.
+    /// Creates a new frame allocator from the bootloader's memory map.
+    ///
+    /// The allocator will return frames from memory regions marked as
+    /// [`MemoryRegionType::Usable`] in the memory map.
+    ///
+    /// # Arguments
+    ///
+    /// * `memory_map` - The memory map from the bootloader's [`BootInfo`]
+    ///
+    /// # Returns
+    ///
+    /// A new `BootInfoFrameAllocator` ready to allocate frames.
+    ///
+    /// # Safety
+    ///
+    /// The caller must guarantee that:
+    ///
+    /// 1. The memory map is valid and accurately describes physical memory.
+    ///
+    /// 2. All frames in `Usable` regions are actually unused. If the kernel
+    ///    or bootloader code resides in a region marked as `Usable`, allocating
+    ///    those frames could overwrite kernel code!
+    ///
+    /// 3. The allocator is only created once per memory map. Creating multiple
+    ///    allocators from the same map would cause them to return the same
+    ///    frames, leading to memory corruption.
+    ///
+    /// # Example
     ///
-    /// This function is unsafe because the caller must guarantee that the passed
-    /// memory map is valid. The main requirement is that all frames that are marked
-    /// as `USABLE` in it are really unused.
+    /// ```ignore
+    /// let frame_allocator = unsafe {
+    ///     BootInfoFrameAllocator::init(&boot_info.memory_map)
+    /// };
+    /// ```
     pub unsafe fn init(memory_map: &'static MemoryMap) -> Self {
         BootInfoFrameAllocator {
             memory_map,
@@ -65,25 +278,67 @@ impl BootInfoFrameAllocator {
         }
     }
 
-    /// Returns an iterator over the usable frames specified in the memory map.
+    /// Returns an iterator over all usable physical frames.
+    ///
+    /// This method filters the memory map for `Usable` regions, extracts
+    /// their address ranges, and converts them to frame iterators.
+    ///
+    /// ## Implementation Details
+    ///
+    /// 1. Filter memory map for `Usable` regions
+    /// 2. Extract the start and end addresses of each region
+    /// 3. Step through each region in 4 KiB increments
+    /// 4. Convert each address to a `PhysFrame`
+    ///
+    /// # Returns
+    ///
+    /// An iterator yielding [`PhysFrame`] objects for each 4 KiB frame in
+    /// usable memory regions.
     fn usable_frames(&self) -> impl Iterator<Item = PhysFrame> {
-        // get usable regions from memory map
+        // Get all regions from the memory map
         let regions = self.memory_map.iter();
+
+        // Filter to only usable regions
         let usable_regions = regions.filter(|r| r.region_type == MemoryRegionType::Usable);
-        // map each region to its address range
+
+        // Extract the address range from each region
         let addr_ranges = usable_regions.map(|r| r.range.start_addr()..r.range.end_addr());
-        // transform to an iterator of frame start addresses
+
+        // Step through each range in 4 KiB (page size) increments
         let frame_addresses = addr_ranges.flat_map(|r| r.step_by(4096));
-        // create `PhysFrame` types from the start addresses
+
+        // Convert each address to a PhysFrame
         frame_addresses.map(|addr| PhysFrame::containing_address(PhysAddr::new(addr)))
     }
 }
 
+/// Implementation of the [`FrameAllocator`] trait for [`BootInfoFrameAllocator`].
+///
+/// This allows the allocator to be used with the x86_64 crate's mapping functions.
 unsafe impl FrameAllocator<Size4KiB> for BootInfoFrameAllocator {
+    /// Allocates a single physical frame.
+    ///
+    /// Returns the next available frame from the usable memory regions, or
+    /// `None` if all frames have been allocated.
+    ///
+    /// # Performance Note
+    ///
+    /// This implementation uses `Iterator::nth()` to skip to the `next` frame,
+    /// which requires iterating through the memory map each time. For better
+    /// performance in production, consider caching the usable frames or using
+    /// a more sophisticated allocator.
+    ///
+    /// # Returns
+    ///
+    /// - `Some(frame)`: A physical frame that is now allocated
+    /// - `None`: No more frames available
     fn allocate_frame(&mut self) -> Option<PhysFrame> {
+        // Get the nth usable frame (where n = self.next)
         let frame = self.usable_frames().nth(self.next);
+
+        // Move to the next frame for the next allocation
         self.next += 1;
+
         frame
     }
 }
-
diff --git a/StrixKernel/src/serial.rs b/StrixKernel/src/serial.rs
index 65596c3..ca6514e 100644
--- a/StrixKernel/src/serial.rs
+++ b/StrixKernel/src/serial.rs
@@ -1,20 +1,118 @@
+//! # Serial Port Communication
+//!
+//! This module provides serial port output for debugging and test reporting.
+//! It implements [`serial_print!`] and [`serial_println!`] macros that write
+//! to the first serial port (COM1).
+//!
+//! ## Why Serial Output?
+//!
+//! Serial port output is essential for kernel development because:
+//!
+//! 1. **Host-readable**: QEMU can redirect serial output to the host terminal,
+//!    making it easy to capture kernel output.
+//!
+//! 2. **Test automation**: The test framework uses serial output to report
+//!    test results, which can be parsed by the test runner.
+//!
+//! 3. **Early boot debugging**: Serial works before VGA initialization.
+//!
+//! 4. **No interference**: Doesn't affect VGA output, allowing separate
+//!    debug and user-facing channels.
+//!
+//! ## UART 16550
+//!
+//! The serial port uses the UART 16550 chip (or compatible). This is a standard
+//! serial communication interface that has been part of PCs since the IBM PC/AT.
+//!
+//! ### COM Ports and I/O Addresses
+//!
+//! | Port | I/O Base | IRQ | Typical Use            |
+//! |------|----------|-----|------------------------|
+//! | COM1 | 0x3F8    | 4   | Serial console (used)  |
+//! | COM2 | 0x2F8    | 3   | Modem/auxiliary        |
+//! | COM3 | 0x3E8    | 4   | Varies                 |
+//! | COM4 | 0x2E8    | 3   | Varies                 |
+//!
+//! ## QEMU Configuration
+//!
+//! When running in QEMU, serial output is typically redirected to stdio:
+//!
+//! ```text
+//! qemu-system-x86_64 ... -serial stdio
+//! ```
+//!
+//! This allows kernel serial output to appear in the terminal running QEMU.
+//!
+//! ## Usage
+//!
+//! ```ignore
+//! use strix_os::{serial_print, serial_println};
+//!
+//! serial_println!("Debug: entering function foo");
+//! serial_print!("Value: ");
+//! serial_println!("{}", some_value);
+//! ```
+
 use lazy_static::lazy_static;
 use spin::Mutex;
 use uart_16550::SerialPort;
 
 lazy_static! {
+    /// The global serial port instance for COM1.
+    ///
+    /// This provides access to the first serial port (COM1) at I/O base address
+    /// 0x3F8. The port is wrapped in a [`spin::Mutex`] for thread-safe access
+    /// from interrupt handlers and concurrent code.
+    ///
+    /// ## Initialization
+    ///
+    /// The serial port is initialized using the `uart_16550` crate's `init()`
+    /// method, which configures the UART with standard settings:
+    /// - 38400 baud (default for the crate)
+    /// - 8 data bits
+    /// - 1 stop bit
+    /// - No parity
+    /// - FIFOs enabled
+    ///
+    /// ## Safety
+    ///
+    /// Creating the `SerialPort` is unsafe because:
+    /// - We're specifying a raw I/O port address
+    /// - The port must actually exist and be a UART 16550
+    /// - Only one `SerialPort` should exist per physical port
     pub static ref SERIAL1: Mutex<SerialPort> = {
+        // SAFETY: 0x3F8 is the standard COM1 port address.
+        // This is always present on x86 PCs (real or emulated).
         let mut serial_port = unsafe { SerialPort::new(0x3F8) };
+
+        // Initialize the UART (set baud rate, enable FIFOs, etc.)
         serial_port.init();
+
         Mutex::new(serial_port)
     };
 }
 
+/// Internal function used by the serial print macros.
+///
+/// Acquires the [`SERIAL1`] lock and writes the formatted arguments to the
+/// serial port. Interrupts are disabled during the write to prevent deadlocks.
+///
+/// # Arguments
+///
+/// * `args` - The pre-formatted arguments from `format_args!`
+///
+/// # Panics
+///
+/// Panics with the message "Printing to serial failed" if writing fails.
+/// This typically indicates a hardware problem or misconfiguration.
 #[doc(hidden)]
 pub fn _print(args: ::core::fmt::Arguments) {
     use core::fmt::Write;
     use x86_64::instructions::interrupts;
-    
+
+    // Disable interrupts while holding the lock to prevent deadlock.
+    // Without this, an interrupt handler trying to print while we hold
+    // the lock would spin forever on the same CPU.
     interrupts::without_interrupts(|| {
         SERIAL1
             .lock()
@@ -23,7 +121,23 @@ pub fn _print(args: ::core::fmt::Arguments) {
     });
 }
 
-/// Prints to the host through the serial interface.
+/// Prints formatted text to the serial port (without newline).
+///
+/// This macro works like the standard library's [`print!`](std::print), but
+/// outputs to the COM1 serial port instead of stdout.
+///
+/// ## Output Destination
+///
+/// When running in QEMU with `-serial stdio`, output appears in the terminal
+/// running QEMU. This is separate from VGA output shown in the QEMU window.
+///
+/// # Example
+///
+/// ```ignore
+/// serial_print!("Debug: ");
+/// serial_print!("value = {}", 42);
+/// // Serial output: Debug: value = 42
+/// ```
 #[macro_export]
 macro_rules! serial_print {
     ($($arg:tt)*) => {
@@ -31,7 +145,24 @@ macro_rules! serial_print {
     };
 }
 
-/// Prints to the host through the serial interface, appending a newline.
+/// Prints formatted text to the serial port with a trailing newline.
+///
+/// This macro works like the standard library's [`println!`](std::println), but
+/// outputs to the COM1 serial port instead of stdout.
+///
+/// ## Use Cases
+///
+/// - **Test output**: Report test pass/fail status
+/// - **Debugging**: Print variable values and execution flow
+/// - **Logging**: Record events for post-mortem analysis
+///
+/// # Example
+///
+/// ```ignore
+/// serial_println!("Kernel initialized");
+/// serial_println!("Memory: {} bytes available", free_memory);
+/// serial_println!();  // Just a newline
+/// ```
 #[macro_export]
 macro_rules! serial_println {
     () => ($crate::serial_print!("\n"));
diff --git a/StrixKernel/src/vga_buffer.rs b/StrixKernel/src/vga_buffer.rs
index f21a94e..8e9f46f 100644
--- a/StrixKernel/src/vga_buffer.rs
+++ b/StrixKernel/src/vga_buffer.rs
@@ -1,137 +1,350 @@
+//! # VGA Text Mode Buffer
+//!
+//! This module provides text output to the VGA text mode display, implementing
+//! the [`print!`] and [`println!`] macros for kernel console output.
+//!
+//! ## VGA Text Mode Overview
+//!
+//! VGA text mode is a legacy display mode that provides an 80×25 character
+//! grid (80 columns, 25 rows). Each cell contains:
+//!
+//! - An ASCII character (1 byte)
+//! - A color attribute (1 byte: 4 bits foreground + 4 bits background)
+//!
+//! The text buffer is memory-mapped at physical address `0xB8000`. Writing to
+//! this memory region immediately updates the display.
+//!
+//! ## Memory Layout
+//!
+//! ```text
+//! Address 0xB8000:
+//! ┌─────────┬────────────┬─────────┬────────────┬─────────┬────────────┐
+//! │ Char 0  │  Color 0   │ Char 1  │  Color 1   │   ...   │  Char 1999 │
+//! │ (1 byte)│  (1 byte)  │ (1 byte)│  (1 byte)  │         │  (1 byte)  │
+//! └─────────┴────────────┴─────────┴────────────┴─────────┴────────────┘
+//!    Row 0, Col 0          Row 0, Col 1            ...      Row 24, Col 79
+//! ```
+//!
+//! ## Color Attribute Byte
+//!
+//! ```text
+//! ┌───┬───┬───┬───┬───┬───┬───┬───┐
+//! │ 7 │ 6 │ 5 │ 4 │ 3 │ 2 │ 1 │ 0 │
+//! └───┴───┴───┴───┴───┴───┴───┴───┘
+//!   │   └───┴───┘   └───┴───┴───┘
+//!   │   Background     Foreground
+//!   Blink (or high intensity background)
+//! ```
+//!
+//! ## Usage
+//!
+//! This module exports [`print!`] and [`println!`] macros that work like their
+//! standard library counterparts:
+//!
+//! ```ignore
+//! use strix_os::println;
+//!
+//! println!("Hello, {}!", "World");
+//! println!("Number: {}", 42);
+//! ```
+//!
+//! ## Thread Safety
+//!
+//! The global [`WRITER`] instance is protected by a spinlock, making it safe
+//! to call `print!` and `println!` from interrupt handlers and concurrent code.
+
 use core::fmt;
 use lazy_static::lazy_static;
 use spin::Mutex;
 use volatile::Volatile;
 
 lazy_static! {
-    /// A global `Writer` instance that can be used for printing to the VGA text buffer.
+    /// The global VGA text buffer writer instance.
+    ///
+    /// This writer is used by the [`print!`] and [`println!`] macros to output
+    /// text to the screen. It's protected by a [`spin::Mutex`] for thread safety.
+    ///
+    /// ## Configuration
+    ///
+    /// - **Color**: Yellow text on black background (configurable)
+    /// - **Position**: Always writes to the last row (row 24), scrolling as needed
     ///
-    /// Used by the `print!` and `println!` macros.
+    /// ## Memory Location
+    ///
+    /// The buffer is mapped to physical address `0xB8000`, which is the standard
+    /// VGA text mode buffer address on x86 systems.
     pub static ref WRITER: Mutex<Writer> = Mutex::new(Writer {
         column_position: 0,
         color_code: ColorCode::new(Color::Yellow, Color::Black),
+        // SAFETY: 0xB8000 is the well-known VGA text buffer address.
+        // This memory is always present on x86 systems with VGA.
         buffer: unsafe { &mut *(0xb8000 as *mut Buffer) },
     });
 }
 
-/// The standard color palette in VGA text mode.
+/// The 16-color VGA text mode palette.
+///
+/// VGA text mode supports 16 colors for both foreground and background.
+/// Colors 8-15 are "bright" or "high intensity" versions of colors 0-7.
+///
+/// ## Color Indices
+///
+/// | Index | Color       | Index | Color         |
+/// |-------|-------------|-------|---------------|
+/// | 0     | Black       | 8     | Dark Gray     |
+/// | 1     | Blue        | 9     | Light Blue    |
+/// | 2     | Green       | 10    | Light Green   |
+/// | 3     | Cyan        | 11    | Light Cyan    |
+/// | 4     | Red         | 12    | Light Red     |
+/// | 5     | Magenta     | 13    | Pink          |
+/// | 6     | Brown       | 14    | Yellow        |
+/// | 7     | Light Gray  | 15    | White         |
 #[allow(dead_code)]
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[repr(u8)]
 pub enum Color {
+    /// Black (0x0)
     Black = 0,
+    /// Blue (0x1)
     Blue = 1,
+    /// Green (0x2)
     Green = 2,
+    /// Cyan (0x3)
     Cyan = 3,
+    /// Red (0x4)
     Red = 4,
+    /// Magenta (0x5)
     Magenta = 5,
+    /// Brown (0x6)
     Brown = 6,
+    /// Light Gray (0x7)
     LightGray = 7,
+    /// Dark Gray (0x8) - bright black
     DarkGray = 8,
+    /// Light Blue (0x9) - bright blue
     LightBlue = 9,
+    /// Light Green (0xA) - bright green
     LightGreen = 10,
+    /// Light Cyan (0xB) - bright cyan
     LightCyan = 11,
+    /// Light Red (0xC) - bright red
     LightRed = 12,
+    /// Pink (0xD) - bright magenta
     Pink = 13,
+    /// Yellow (0xE) - bright brown
     Yellow = 14,
+    /// White (0xF) - bright light gray
     White = 15,
 }
 
-/// A combination of a foreground and a background color.
+/// A combination of foreground and background colors.
+///
+/// The color code is stored as a single byte:
+/// - Bits 0-3: Foreground color (0-15)
+/// - Bits 4-6: Background color (0-7)
+/// - Bit 7: Blink (or bright background, depending on VGA mode)
+///
+/// ## Memory Layout
+///
+/// ```text
+/// Bits: 7 6 5 4 3 2 1 0
+///       │ └─┬─┘ └──┬──┘
+///       │   │      └── Foreground (0-15)
+///       │   └── Background (0-7)
+///       └── Blink/Bright
+/// ```
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[repr(transparent)]
 struct ColorCode(u8);
 
 impl ColorCode {
-    /// Create a new `ColorCode` with the given foreground and background colors.
+    /// Creates a new color code from foreground and background colors.
+    ///
+    /// # Arguments
+    ///
+    /// * `foreground` - The text color (0-15)
+    /// * `background` - The background color (0-7, or 0-15 with bright backgrounds)
+    ///
+    /// # Returns
+    ///
+    /// A color code with the foreground and background packed into a single byte.
+    ///
+    /// # Example
+    ///
+    /// ```ignore
+    /// let code = ColorCode::new(Color::White, Color::Blue);
+    /// // Result: 0x1F (white on blue)
+    /// ```
     fn new(foreground: Color, background: Color) -> ColorCode {
         ColorCode((background as u8) << 4 | (foreground as u8))
     }
 }
 
-/// A screen character in the VGA text buffer, consisting of an ASCII character and a `ColorCode`.
+/// A single character cell in the VGA text buffer.
+///
+/// Each cell consists of an ASCII character code and a color attribute byte.
+/// The struct uses C representation to ensure the memory layout matches
+/// the VGA hardware expectation.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[repr(C)]
 struct ScreenChar {
+    /// The ASCII character code to display.
+    /// Only printable ASCII (0x20-0x7E) renders correctly.
     ascii_character: u8,
+
+    /// The color attribute (foreground + background colors).
     color_code: ColorCode,
 }
 
-/// The height of the text buffer (normally 25 lines).
+/// The height of the VGA text buffer in rows.
 const BUFFER_HEIGHT: usize = 25;
-/// The width of the text buffer (normally 80 columns).
+
+/// The width of the VGA text buffer in columns.
 const BUFFER_WIDTH: usize = 80;
 
-/// A structure representing the VGA text buffer.
+/// The VGA text mode buffer.
+///
+/// This structure represents the 80×25 character grid. Each cell is wrapped
+/// in [`Volatile`] to prevent the compiler from optimizing away writes to
+/// memory-mapped I/O.
+///
+/// ## Why Volatile?
+///
+/// The VGA buffer is memory-mapped hardware. Without volatile access:
+/// 1. The compiler might optimize away "dead" writes
+/// 2. Multiple writes might be combined
+/// 3. Writes might be reordered
+///
+/// Volatile access ensures every write goes through to hardware in order.
 #[repr(transparent)]
 struct Buffer {
+    /// The 2D array of screen characters.
+    /// Indexed as `chars[row][column]`.
     chars: [[Volatile<ScreenChar>; BUFFER_WIDTH]; BUFFER_HEIGHT],
 }
 
-/// A writer type that allows writing ASCII bytes and strings to an underlying `Buffer`.
+/// A writer for the VGA text buffer.
 ///
-/// Wraps lines at `BUFFER_WIDTH`. Supports newline characters and implements the
-/// `core::fmt::Write` trait.
+/// The writer maintains a cursor position and provides methods for writing
+/// characters and strings. It always writes to the last row (row 24) and
+/// scrolls the buffer up when a new line is needed.
+///
+/// ## Writing Behavior
+///
+/// - Printable ASCII characters are written at the current cursor position
+/// - Newline (`\n`) causes the buffer to scroll up
+/// - Non-printable characters are rendered as `■` (0xFE)
+/// - When the cursor reaches the end of a row, it wraps to the next line
 pub struct Writer {
+    /// Current column position (0-79).
     column_position: usize,
+
+    /// Current color for new characters.
     color_code: ColorCode,
+
+    /// Reference to the VGA buffer.
     buffer: &'static mut Buffer,
 }
 
 impl Writer {
-    /// Writes an ASCII byte to the buffer.
+    /// Writes a single ASCII byte to the buffer.
+    ///
+    /// This method handles:
+    /// - **Newline** (`\n`): Scrolls the buffer and resets column to 0
+    /// - **Printable characters**: Written at current position, column advances
+    /// - **End of row**: Automatically wraps to the next line
+    ///
+    /// # Arguments
+    ///
+    /// * `byte` - The ASCII byte to write
+    ///
+    /// # Example
     ///
-    /// Wraps lines at `BUFFER_WIDTH`. Supports the `\n` newline character.
+    /// ```ignore
+    /// writer.write_byte(b'H');
+    /// writer.write_byte(b'\n');
+    /// ```
     pub fn write_byte(&mut self, byte: u8) {
         match byte {
             b'\n' => self.new_line(),
             byte => {
+                // Wrap to new line if we've reached the end of the row
                 if self.column_position >= BUFFER_WIDTH {
                     self.new_line();
                 }
 
+                // Always write to the last row
                 let row = BUFFER_HEIGHT - 1;
                 let col = self.column_position;
 
                 let color_code = self.color_code;
+
+                // Write the character to the buffer using volatile access
                 self.buffer.chars[row][col].write(ScreenChar {
                     ascii_character: byte,
                     color_code,
                 });
+
                 self.column_position += 1;
             }
         }
     }
 
-    /// Writes the given ASCII string to the buffer.
+    /// Writes an ASCII string to the buffer.
+    ///
+    /// Iterates through each byte of the string and writes it using
+    /// [`write_byte()`](Writer::write_byte). Non-printable characters
+    /// (outside 0x20-0x7E range, except newline) are replaced with `■` (0xFE).
+    ///
+    /// # Arguments
     ///
-    /// Wraps lines at `BUFFER_WIDTH`. Supports the `\n` newline character. Does **not**
-    /// support strings with non-ASCII characters, since they can't be printed in the VGA text
-    /// mode.
+    /// * `s` - The string slice to write (must be valid UTF-8, but only ASCII
+    ///   characters will display correctly)
+    ///
+    /// # Note
+    ///
+    /// VGA text mode only supports ASCII. Multi-byte UTF-8 characters will
+    /// render as the placeholder character `■`.
     fn write_string(&mut self, s: &str) {
         for byte in s.bytes() {
             match byte {
-                // printable ASCII byte or newline
+                // Printable ASCII byte or newline - write as-is
                 0x20..=0x7e | b'\n' => self.write_byte(byte),
-                // not part of printable ASCII range
+                // Non-printable byte - write placeholder (filled square)
                 _ => self.write_byte(0xfe),
             }
         }
     }
 
-    /// Shifts all lines one line up and clears the last row.
+    /// Scrolls the buffer up by one line and clears the last row.
+    ///
+    /// This method:
+    /// 1. Copies each row up by one (row 1 → row 0, row 2 → row 1, etc.)
+    /// 2. Clears the last row (row 24) with spaces
+    /// 3. Resets the column position to 0
+    ///
+    /// Row 0 is lost in the process (scrolled off the top).
     fn new_line(&mut self) {
+        // Move all rows up by one
         for row in 1..BUFFER_HEIGHT {
             for col in 0..BUFFER_WIDTH {
                 let character = self.buffer.chars[row][col].read();
                 self.buffer.chars[row - 1][col].write(character);
             }
         }
+
+        // Clear the last row
         self.clear_row(BUFFER_HEIGHT - 1);
+
+        // Reset cursor to beginning of last row
         self.column_position = 0;
     }
 
-    /// Clears a row by overwriting it with blank characters.
+    /// Clears a row by filling it with space characters.
+    ///
+    /// # Arguments
+    ///
+    /// * `row` - The row index to clear (0-24)
     fn clear_row(&mut self, row: usize) {
         let blank = ScreenChar {
             ascii_character: b' ',
@@ -143,42 +356,93 @@ impl Writer {
     }
 }
 
+/// Implementation of [`core::fmt::Write`] for the VGA writer.
+///
+/// This allows the writer to be used with Rust's formatting macros via
+/// `write!` and `writeln!`.
 impl fmt::Write for Writer {
+    /// Writes a string slice to the VGA buffer.
+    ///
+    /// Delegates to [`Writer::write_string()`].
     fn write_str(&mut self, s: &str) -> fmt::Result {
         self.write_string(s);
         Ok(())
     }
 }
 
-/// Like the `print!` macro in the standard library, but prints to the VGA text buffer.
+/// Prints formatted text to the VGA buffer (without newline).
+///
+/// This macro works like the standard library's [`print!`](std::print), but
+/// outputs to the VGA text buffer instead of stdout.
+///
+/// # Example
+///
+/// ```ignore
+/// print!("Hello");
+/// print!(", {}!", "World");
+/// // Output: Hello, World!
+/// ```
 #[macro_export]
 macro_rules! print {
     ($($arg:tt)*) => ($crate::vga_buffer::_print(format_args!($($arg)*)));
 }
 
-/// Like the `println!` macro in the standard library, but prints to the VGA text buffer.
+/// Prints formatted text to the VGA buffer with a trailing newline.
+///
+/// This macro works like the standard library's [`println!`](std::println), but
+/// outputs to the VGA text buffer instead of stdout.
+///
+/// # Example
+///
+/// ```ignore
+/// println!("Hello, World!");
+/// println!("The answer is {}", 42);
+/// ```
 #[macro_export]
 macro_rules! println {
     () => ($crate::print!("\n"));
     ($($arg:tt)*) => ($crate::print!("{}\n", format_args!($($arg)*)));
 }
 
-/// Prints the given formatted string to the VGA text buffer through the global `WRITER` instance.
+/// Internal function used by the `print!` and `println!` macros.
+///
+/// Acquires the global [`WRITER`] lock and writes the formatted arguments
+/// to the VGA buffer. Interrupts are disabled during the write to prevent
+/// deadlocks (an interrupt handler could try to acquire the same lock).
+///
+/// # Arguments
+///
+/// * `args` - The pre-formatted arguments from `format_args!`
+///
+/// # Panics
+///
+/// Panics if writing to the buffer fails (should never happen with VGA).
 #[doc(hidden)]
 pub fn _print(args: fmt::Arguments) {
     use core::fmt::Write;
     use x86_64::instructions::interrupts;
 
+    // Disable interrupts while holding the lock to prevent deadlock.
+    // If an interrupt tried to print while we hold the lock, it would
+    // wait forever (spinlock + same CPU = deadlock).
     interrupts::without_interrupts(|| {
         WRITER.lock().write_fmt(args).unwrap();
     });
 }
 
+// =============================================================================
+// Tests
+// =============================================================================
+
+/// Test that a simple println works without panicking.
 #[test_case]
 fn test_println_simple() {
     println!("test_println_simple output");
 }
 
+/// Test that many println calls work without panicking or overflowing.
+///
+/// This test writes 200 lines, which forces multiple screen scrolls.
 #[test_case]
 fn test_println_many() {
     for _ in 0..200 {
@@ -186,19 +450,32 @@ fn test_println_many() {
     }
 }
 
+/// Test that println actually writes the correct characters to the buffer.
+///
+/// This test:
+/// 1. Prints a test string followed by a newline
+/// 2. Reads back the characters from the buffer
+/// 3. Verifies each character matches the original string
 #[test_case]
 fn test_println_output() {
     use core::fmt::Write;
     use x86_64::instructions::interrupts;
 
     let s = "Some test string that fits on a single line";
+
+    // Disable interrupts and hold the lock for the entire test
     interrupts::without_interrupts(|| {
         let mut writer = WRITER.lock();
+
+        // Write newline first to ensure we're on a fresh line,
+        // then write the test string
         writeln!(writer, "\n{}", s).expect("writeln failed");
+
+        // Verify each character was written correctly
+        // After the newline, the string is on row BUFFER_HEIGHT - 2
         for (i, c) in s.chars().enumerate() {
             let screen_char = writer.buffer.chars[BUFFER_HEIGHT - 2][i].read();
             assert_eq!(char::from(screen_char.ascii_character), c);
         }
     });
 }
-
diff --git a/StrixKernel/tests/basic_boot.rs b/StrixKernel/tests/basic_boot.rs
index 3707458..254c6aa 100644
--- a/StrixKernel/tests/basic_boot.rs
+++ b/StrixKernel/tests/basic_boot.rs
@@ -1,3 +1,37 @@
+//! # Basic Boot Integration Test
+//!
+//! This integration test verifies that the kernel boots successfully and that
+//! fundamental kernel functionality works correctly in a minimal environment.
+//!
+//! ## Purpose
+//!
+//! Unlike unit tests which test individual components in isolation, this integration
+//! test runs as a completely separate kernel instance. This catches issues that might
+//! only appear during actual boot, such as:
+//!
+//! - Boot process failures
+//! - Initialization order problems
+//! - VGA buffer functionality in a real boot context
+//! - Interrupt setup issues
+//!
+//! ## How It Works
+//!
+//! 1. The bootloader loads this test as a standalone kernel
+//! 2. `_start()` is called as the entry point
+//! 3. The test framework runs all `#[test_case]` functions
+//! 4. Results are output via serial port to the host
+//! 5. QEMU exits with success (0x10) or failure (0x11) code
+//!
+//! ## Test Cases
+//!
+//! - `test_println`: Verifies the VGA buffer and `println!` macro work correctly
+//!
+//! ## Running This Test
+//!
+//! ```bash
+//! cargo test --test basic_boot
+//! ```
+
 #![no_std]
 #![no_main]
 #![feature(custom_test_frameworks)]
@@ -19,6 +53,18 @@ fn panic(info: &PanicInfo) -> ! {
     strix_os::test_panic_handler(info)
 }
 
+/// Tests that the `println!` macro works correctly after boot.
+///
+/// This is a smoke test that verifies:
+/// - The VGA buffer is properly initialized
+/// - The `WRITER` static is accessible
+/// - String formatting works in the kernel environment
+/// - No panics occur during text output
+///
+/// If this test fails, it indicates a fundamental problem with either:
+/// - VGA buffer memory mapping
+/// - Spinlock implementation
+/// - The lazy_static initialization
 #[test_case]
 fn test_println() {
     println!("test_println output");
diff --git a/StrixKernel/tests/breakpoint_exception.rs b/StrixKernel/tests/breakpoint_exception.rs
new file mode 100644
index 0000000..750c1d1
--- /dev/null
+++ b/StrixKernel/tests/breakpoint_exception.rs
@@ -0,0 +1,85 @@
+//! # Breakpoint Exception Integration Test
+//!
+//! This integration test verifies that the breakpoint exception handler (vector 3)
+//! works correctly in a real boot context.
+//!
+//! ## Purpose
+//!
+//! The breakpoint exception is triggered by the `int3` instruction and is commonly
+//! used by debuggers. Unlike most exceptions, it is non-fatal - execution should
+//! continue after the handler returns.
+//!
+//! This test verifies:
+//! - The IDT is properly set up with a breakpoint handler
+//! - The handler executes without crashing
+//! - Execution continues normally after the exception
+//!
+//! ## Exception Details
+//!
+//! - **Vector**: 3
+//! - **Type**: Trap (RIP points to instruction after `int3`)
+//! - **Error Code**: None
+//! - **Fatal**: No
+//!
+//! ## Running This Test
+//!
+//! ```bash
+//! cargo test --test breakpoint_exception
+//! ```
+
+#![no_std]
+#![no_main]
+#![feature(custom_test_frameworks)]
+#![test_runner(strix_os::test_runner)]
+#![reexport_test_harness_main = "test_main"]
+
+use core::panic::PanicInfo;
+use strix_os::serial_println;
+
+#[unsafe(no_mangle)]
+pub extern "C" fn _start() -> ! {
+    strix_os::init();
+    test_main();
+
+    loop {}
+}
+
+#[panic_handler]
+fn panic(info: &PanicInfo) -> ! {
+    strix_os::test_panic_handler(info)
+}
+
+/// Tests that the breakpoint exception handler works correctly.
+///
+/// This test triggers a breakpoint exception using the `int3` instruction and
+/// verifies that:
+/// 1. The exception handler is invoked
+/// 2. The handler returns (doesn't panic or halt)
+/// 3. Execution continues normally after the breakpoint
+///
+/// If this test passes, it confirms that:
+/// - The IDT is correctly initialized with a breakpoint handler
+/// - The handler is properly registered as a trap (not fault)
+/// - The kernel can recover from breakpoint exceptions
+#[test_case]
+fn test_breakpoint_exception() {
+    serial_println!("Triggering breakpoint exception...");
+
+    // Invoke a breakpoint exception - should return normally
+    x86_64::instructions::interrupts::int3();
+
+    serial_println!("Execution continued after breakpoint!");
+}
+
+/// Tests that multiple breakpoint exceptions can be handled in sequence.
+///
+/// This ensures the IDT remains valid after handling an exception and that
+/// the handler can be invoked multiple times without issues.
+#[test_case]
+fn test_multiple_breakpoints() {
+    for i in 0..3 {
+        serial_println!("Breakpoint iteration {}...", i);
+        x86_64::instructions::interrupts::int3();
+    }
+    serial_println!("All breakpoints handled successfully!");
+}
diff --git a/StrixKernel/tests/should_panic.rs b/StrixKernel/tests/should_panic.rs
index ad58af2..4a236de 100644
--- a/StrixKernel/tests/should_panic.rs
+++ b/StrixKernel/tests/should_panic.rs
@@ -1,22 +1,90 @@
+//! # Should Panic Integration Test
+//!
+//! This integration test verifies that the kernel's panic handler works correctly.
+//! It's an "inverse" test: success means the code panicked, failure means it didn't.
+//!
+//! ## Purpose
+//!
+//! Testing panic behavior is critical for a kernel because:
+//! - Panics in kernel code must be handled gracefully
+//! - The panic handler is our last line of defense against undefined behavior
+//! - We need to verify panics produce useful output (via serial port)
+//!
+//! ## How It Works
+//!
+//! Unlike normal tests where panic = failure, this test inverts the logic:
+//!
+//! | Outcome | Test Result |
+//! |---------|-------------|
+//! | Code panics | **Success** (panic handler works) |
+//! | Code returns normally | **Failure** (panic didn't trigger) |
+//!
+//! The custom panic handler in this file calls `exit_qemu(Success)` instead of
+//! the normal failure exit.
+//!
+//! ## Test Flow
+//!
+//! ```text
+//! _start() → should_fail() → assert_eq!(0, 1)
+//!                                  ↓
+//!                            Assertion fails
+//!                                  ↓
+//!                              panic!()
+//!                                  ↓
+//!                   Custom panic handler called
+//!                                  ↓
+//!                    exit_qemu(Success) ✓
+//! ```
+//!
+//! ## Why `harness = false`
+//!
+//! This test uses `harness = false` in Cargo.toml because:
+//! - It needs a custom panic handler (not the test framework's)
+//! - It manages its own success/failure signaling
+//! - The standard test harness would interfere with panic testing
+//!
+//! ## Running This Test
+//!
+//! ```bash
+//! cargo test --test should_panic
+//! ```
+
 #![no_std]
 #![no_main]
 
 use strix_os::{QemuExitCode, exit_qemu, serial_print, serial_println};
 use core::panic::PanicInfo;
 
+/// Test entry point.
+///
+/// Calls `should_fail()` which is expected to panic. If we reach the lines
+/// after the call, the test has failed because no panic occurred.
 #[unsafe(no_mangle)]
 pub extern "C" fn _start() -> ! {
     should_fail();
+    // If we get here, the assertion didn't panic - that's a test failure
     serial_println!("[test did not panic]");
     exit_qemu(QemuExitCode::Failed);
     loop {}
 }
 
+/// A function that deliberately panics.
+///
+/// Uses `assert_eq!(0, 1)` to trigger a panic with a clear assertion
+/// failure message. This verifies that:
+/// - The `assert_eq!` macro works in kernel context
+/// - Panic messages are properly formatted
+/// - The panic handler receives control
 fn should_fail() {
     serial_print!("should_panic::should_fail...\t");
     assert_eq!(0, 1);
 }
 
+/// Custom panic handler that signals test success.
+///
+/// This inverts normal panic behavior: panicking means the test passed
+/// because we expected the code to panic. We print "[ok]" to match the
+/// output format of other tests and exit with success.
 #[panic_handler]
 fn panic(_info: &PanicInfo) -> ! {
     serial_println!("[ok]");
diff --git a/StrixKernel/tests/stack_overflow.rs b/StrixKernel/tests/stack_overflow.rs
index 66f52a3..65d50dd 100644
--- a/StrixKernel/tests/stack_overflow.rs
+++ b/StrixKernel/tests/stack_overflow.rs
@@ -1,3 +1,68 @@
+//! # Stack Overflow Integration Test
+//!
+//! This integration test verifies that the kernel correctly handles stack overflow
+//! conditions using the Interrupt Stack Table (IST) mechanism.
+//!
+//! ## Background: The Stack Overflow Problem
+//!
+//! When a stack overflow occurs, the CPU attempts to push an exception stack frame
+//! onto the same stack that just overflowed. This causes a page fault (guard page
+//! violation), which itself requires pushing another stack frame... leading to a
+//! **triple fault** and system reset.
+//!
+//! ## The Solution: Interrupt Stack Table (IST)
+//!
+//! The x86-64 Task State Segment (TSS) contains the Interrupt Stack Table, which
+//! provides up to 7 separate stack pointers for specific interrupts. By configuring
+//! the double fault handler to use IST entry 0, we guarantee it always has a valid
+//! stack to run on, even when the main stack is corrupted.
+//!
+//! ## What This Test Verifies
+//!
+//! 1. Stack overflow triggers a page fault (guard page hit)
+//! 2. Page fault on corrupted stack triggers double fault
+//! 3. Double fault handler successfully executes on IST stack
+//! 4. Kernel gracefully handles the error (exits with success code)
+//!
+//! ## Test Flow
+//!
+//! ```text
+//! _start() → stack_overflow() → [recursive calls]
+//!                                      ↓
+//!                            Stack guard page hit
+//!                                      ↓
+//!                            Page Fault (vector 14)
+//!                                      ↓
+//!                     Can't push frame (stack corrupted)
+//!                                      ↓
+//!                           Double Fault (vector 8)
+//!                                      ↓
+//!                    IST switches to known-good stack
+//!                                      ↓
+//!                       test_double_fault_handler()
+//!                                      ↓
+//!                         exit_qemu(Success) ✓
+//! ```
+//!
+//! ## Why We Need a Custom IDT
+//!
+//! This test uses its own IDT (`TEST_IDT`) rather than the kernel's IDT because:
+//! - We need the double fault handler to signal test success
+//! - The kernel's handler panics, which would mark the test as failed
+//! - We still use the kernel's GDT/TSS for the IST stack
+//!
+//! ## Running This Test
+//!
+//! ```bash
+//! cargo test --test stack_overflow
+//! ```
+//!
+//! ## Note
+//!
+//! This test intentionally causes undefined behavior (stack overflow) to verify
+//! that the kernel's safety mechanisms work correctly. The `harness = false`
+//! setting in Cargo.toml indicates this test manages its own execution flow.
+
 #![no_std]
 #![no_main]
 #![feature(abi_x86_interrupt)]
@@ -7,6 +72,11 @@ use core::panic::PanicInfo;
 use lazy_static::lazy_static;
 use x86_64::structures::idt::{InterruptDescriptorTable, InterruptStackFrame};
 
+/// Test entry point.
+///
+/// Initializes only the minimal components needed (GDT for IST, custom IDT),
+/// then deliberately triggers a stack overflow. If we reach the panic at the
+/// end, the test has failed because the overflow wasn't caught.
 #[unsafe(no_mangle)]
 pub extern "C" fn _start() -> ! {
     serial_print!("stack_overflow::stack_overflow...\t");
@@ -14,19 +84,30 @@ pub extern "C" fn _start() -> ! {
     strix_os::gdt::init();
     init_test_idt();
 
-    // trigger a stack overflow
+    // Trigger a stack overflow - this should NOT return
     stack_overflow();
 
     panic!("Execution continued after stack overflow");
 }
 
+/// Causes a stack overflow through infinite recursion.
+///
+/// Each recursive call pushes a return address (8 bytes on x86-64) onto the stack.
+/// Eventually this exhausts the stack space and hits the guard page.
+///
+/// The volatile read prevents the compiler from optimizing this into a tail call,
+/// which would reuse the same stack frame and never overflow.
 #[allow(unconditional_recursion)]
 fn stack_overflow() {
-    stack_overflow(); // for each recursion, the return address is pushed
-    volatile::Volatile::new(0).read(); // prevent tail recursion optimizations
+    stack_overflow(); // Each recursion pushes 8-byte return address
+    volatile::Volatile::new(0).read(); // Prevent tail-call optimization
 }
 
 lazy_static! {
+    /// Test-specific Interrupt Descriptor Table.
+    ///
+    /// Only configures the double fault handler, using the kernel's IST stack.
+    /// The handler signals success instead of panicking.
     static ref TEST_IDT: InterruptDescriptorTable = {
         let mut idt = InterruptDescriptorTable::new();
         unsafe {
@@ -34,15 +115,23 @@ lazy_static! {
                 .set_handler_fn(test_double_fault_handler)
                 .set_stack_index(strix_os::gdt::DOUBLE_FAULT_IST_INDEX);
         }
-
         idt
     };
 }
 
+/// Loads the test IDT into the CPU.
 pub fn init_test_idt() {
     TEST_IDT.load();
 }
 
+/// Double fault handler for testing.
+///
+/// If we reach this handler, the IST mechanism worked correctly:
+/// - The corrupted stack was detected
+/// - The CPU switched to the IST stack
+/// - This handler is now running on a valid stack
+///
+/// We signal success and exit QEMU.
 extern "x86-interrupt" fn test_double_fault_handler(
     _stack_frame: InterruptStackFrame,
     _error_code: u64,