From 2b2dafef0221330c2087e4728d236cd2f60d9c3b Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sun, 28 Dec 2025 22:48:09 +0000
Subject: [PATCH] Add detailed docstrings to bootloader and kernel header files

Co-authored-by: johndoe6345789 <224850594+johndoe6345789@users.noreply.github.com>
---
 bootloader/src/main.c              | 181 ++++++++++++++++++++++++---
 kernel/include/kernel/apic.h       | 105 +++++++++++++++-
 kernel/include/kernel/gdt.h        |  76 ++++++++++-
 kernel/include/kernel/interrupts.h | 127 ++++++++++++++++---
 kernel/include/kernel/memory.h     | 194 +++++++++++++++++++++++++++--
 kernel/include/kernel/pci.h        | 153 ++++++++++++++++++++---
 kernel/include/kernel/smp.h        | 132 ++++++++++++++++++--
 kernel/include/kernel/spinlock.h   |  96 +++++++++++++-
 kernel/include/kernel/timer.h      |  72 ++++++++++-
 9 files changed, 1047 insertions(+), 89 deletions(-)

diff --git a/bootloader/src/main.c b/bootloader/src/main.c
index 0575faf..daee81e 100644
--- a/bootloader/src/main.c
+++ b/bootloader/src/main.c
@@ -18,7 +18,23 @@
 static EFI_SYSTEM_TABLE* gST = NULL;
 static EFI_BOOT_SERVICES* gBS = NULL;
 
-// Helper: Compare GUIDs
+/**
+ * @brief Compares two UEFI GUIDs for equality
+ * 
+ * GUIDs (Globally Unique Identifiers) are 128-bit values used by UEFI to identify
+ * protocols, tables, and other system resources. This function performs a field-by-field
+ * comparison of two GUID structures.
+ * 
+ * @param a Pointer to the first GUID to compare
+ * @param b Pointer to the second GUID to compare
+ * @return 1 if the GUIDs are equal, 0 if they differ
+ * 
+ * @note The GUID structure consists of:
+ *       - Data1: 32-bit value
+ *       - Data2: 16-bit value
+ *       - Data3: 16-bit value
+ *       - Data4: 8-byte array
+ */
 static int guid_compare(const EFI_GUID* a, const EFI_GUID* b) {
     if (a->Data1 != b->Data1) return 0;
     if (a->Data2 != b->Data2) return 0;
@@ -29,14 +45,42 @@ static int guid_compare(const EFI_GUID* a, const EFI_GUID* b) {
     return 1;
 }
 
-// Helper: Memory set
+/**
+ * @brief Sets a block of memory to a specified value
+ * 
+ * This is a simple implementation of memset that works in the UEFI environment.
+ * It fills the first n bytes of the memory area pointed to by s with the constant
+ * byte c. This is used for initializing structures and buffers.
+ * 
+ * @param s Pointer to the memory block to fill
+ * @param c Value to set (converted to unsigned char)
+ * @param n Number of bytes to set
+ * @return Pointer to the memory block s
+ * 
+ * @note This function operates byte-by-byte, so it's not optimized for large blocks.
+ *       However, for bootloader usage with small structures, this is sufficient.
+ */
 static VOID* efi_memset(VOID* s, int c, UINTN n) {
     unsigned char* p = s;
     while (n--) *p++ = (unsigned char)c;
     return s;
 }
 
-// Helper: Memory copy
+/**
+ * @brief Copies a block of memory from source to destination
+ * 
+ * This is a simple implementation of memcpy that works in the UEFI environment.
+ * It copies n bytes from memory area src to memory area dest. The memory areas
+ * must not overlap (use memmove if they might overlap).
+ * 
+ * @param dest Pointer to the destination memory block
+ * @param src Pointer to the source memory block
+ * @param n Number of bytes to copy
+ * @return Pointer to the destination memory block dest
+ * 
+ * @warning The memory areas must not overlap. If they do, the behavior is undefined.
+ * @note This function operates byte-by-byte for simplicity and UEFI compatibility.
+ */
 static VOID* efi_memcpy(VOID* dest, const VOID* src, UINTN n) {
     unsigned char* d = dest;
     const unsigned char* s = src;
@@ -44,8 +88,18 @@ static VOID* efi_memcpy(VOID* dest, const VOID* src, UINTN n) {
     return dest;
 }
 
-/*
- * Print a string to the UEFI console
+/**
+ * @brief Prints a UTF-16 string to the UEFI console output
+ * 
+ * Uses the UEFI Simple Text Output Protocol to display a string on the console.
+ * This is the primary method for user feedback during the boot process.
+ * 
+ * @param str Pointer to a null-terminated UTF-16 string to display
+ * 
+ * @note UEFI uses UTF-16 encoding (CHAR16*) rather than ASCII or UTF-8.
+ *       Literal strings should be prefixed with 'u' (e.g., u"Hello").
+ * @note This function does nothing if the console output protocol is unavailable,
+ *       which may happen in headless systems.
  */
 void print_string(const CHAR16* str) {
     if (gST && gST->ConOut) {
@@ -53,8 +107,18 @@ void print_string(const CHAR16* str) {
     }
 }
 
-/*
- * Print operation status
+/**
+ * @brief Prints an operation description followed by its status result
+ * 
+ * This helper function displays the result of a UEFI operation in a consistent
+ * format: the operation description followed by either " [OK]" or " [FAILED]"
+ * depending on the status code.
+ * 
+ * @param operation UTF-16 string describing the operation that was performed
+ * @param status UEFI status code returned by the operation (EFI_SUCCESS or error)
+ * 
+ * @note EFI_SUCCESS (0) indicates success; any other value indicates failure.
+ * @see print_string() for the underlying output mechanism
  */
 void print_status(const CHAR16* operation, EFI_STATUS status) {
     print_string(operation);
@@ -65,8 +129,28 @@ void print_status(const CHAR16* operation, EFI_STATUS status) {
     }
 }
 
-/*
- * Initialize graphics output protocol
+/**
+ * @brief Initializes the graphics output and retrieves framebuffer information
+ * 
+ * This function locates the UEFI Graphics Output Protocol (GOP) and extracts
+ * framebuffer details needed by the kernel for direct graphics rendering.
+ * The GOP provides a linear framebuffer that can be used for pixel-based graphics.
+ * 
+ * The function stores the following information in boot_info:
+ * - framebuffer_base: Physical address of the framebuffer in memory
+ * - framebuffer_width: Horizontal resolution in pixels
+ * - framebuffer_height: Vertical resolution in pixels
+ * - framebuffer_pitch: Bytes per scanline (width * bytes_per_pixel, may include padding)
+ * - framebuffer_bpp: Bits per pixel (assumed 32-bit BGRA format)
+ * 
+ * @param ImageHandle Handle to the bootloader image (currently unused)
+ * @param boot_info Pointer to BootInfo structure to receive framebuffer information
+ * @return EFI_SUCCESS if GOP was located and framebuffer info was retrieved,
+ *         or an error code if GOP is unavailable
+ * 
+ * @note This function uses the current graphics mode without attempting to change it.
+ *       The resolution is whatever UEFI firmware has already configured.
+ * @note The framebuffer format is assumed to be 32-bit (4 bytes per pixel).
  */
 EFI_STATUS initialize_graphics(EFI_HANDLE ImageHandle, BootInfo* boot_info) {
     (void)ImageHandle;
@@ -93,8 +177,28 @@ EFI_STATUS initialize_graphics(EFI_HANDLE ImageHandle, BootInfo* boot_info) {
     return EFI_SUCCESS;
 }
 
-/*
- * Load kernel from disk
+/**
+ * @brief Loads the kernel binary from the boot disk into memory
+ * 
+ * This function performs the following steps to load the kernel:
+ * 1. Obtains the Loaded Image Protocol to identify the boot device
+ * 2. Opens the Simple File System Protocol on the boot device
+ * 3. Opens the root directory of the file system
+ * 4. Reads the kernel file "metalos.bin" from the root directory
+ * 5. Allocates a temporary buffer and reads the kernel into it
+ * 6. Copies the kernel to its final load address (KERNEL_LOAD_ADDRESS = 0x100000 = 1MB)
+ * 7. Stores kernel location and size in boot_info for the kernel's use
+ * 
+ * @param ImageHandle Handle to the bootloader image, used to find the boot device
+ * @param boot_info Pointer to BootInfo structure to receive kernel location and size
+ * @return EFI_SUCCESS if kernel was loaded successfully, or an error code on failure
+ * 
+ * @note The kernel file must be named "metalos.bin" and located in the root directory
+ *       of the boot device (typically the EFI System Partition).
+ * @note The kernel is copied to physical address 0x100000 (1MB), which is a standard
+ *       location above the legacy BIOS area and below the 16MB mark.
+ * @note This function allocates memory using UEFI's AllocatePool, which is only valid
+ *       until ExitBootServices is called. The kernel is copied to a permanent location.
  */
 EFI_STATUS load_kernel(EFI_HANDLE ImageHandle, BootInfo* boot_info) {
     EFI_STATUS status;
@@ -181,8 +285,26 @@ EFI_STATUS load_kernel(EFI_HANDLE ImageHandle, BootInfo* boot_info) {
     return EFI_SUCCESS;
 }
 
-/*
- * Get ACPI RSDP (Root System Description Pointer)
+/**
+ * @brief Retrieves the ACPI RSDP (Root System Description Pointer) from UEFI
+ * 
+ * The RSDP is the entry point to ACPI (Advanced Configuration and Power Interface)
+ * tables, which provide information about the system hardware, including:
+ * - Multiple APIC Description Table (MADT) for SMP initialization
+ * - PCI routing tables
+ * - Power management configuration
+ * - Hardware description
+ * 
+ * This function searches the UEFI Configuration Table for the ACPI 2.0+ table GUID
+ * and returns a pointer to the RSDP structure if found.
+ * 
+ * @return Pointer to the RSDP structure if found, NULL if not available
+ * 
+ * @note ACPI 2.0+ is preferred over ACPI 1.0 because it uses 64-bit addresses.
+ * @note The RSDP pointer remains valid after ExitBootServices is called since it
+ *       points to firmware-provided tables in reserved memory.
+ * @note The kernel can use this to locate ACPI tables for multicore initialization
+ *       and hardware discovery.
  */
 void* get_rsdp(void) {
     EFI_GUID acpi_20_guid = EFI_ACPI_20_TABLE_GUID;
@@ -197,8 +319,37 @@ void* get_rsdp(void) {
     return NULL;
 }
 
-/*
- * Main entry point for bootloader
+/**
+ * @brief Main entry point for the UEFI bootloader
+ * 
+ * This is the entry point called by UEFI firmware when the bootloader is loaded.
+ * It performs the following steps in order:
+ * 
+ * 1. Initialize UEFI services and boot_info structure
+ * 2. Display boot banner
+ * 3. Initialize graphics and get framebuffer information
+ * 4. Load the kernel binary from disk
+ * 5. Get ACPI RSDP for hardware information
+ * 6. Get UEFI memory map
+ * 7. Exit UEFI boot services (point of no return - transfers control from firmware)
+ * 8. Jump to the kernel entry point
+ * 
+ * After ExitBootServices is called:
+ * - UEFI Boot Services are no longer available
+ * - UEFI Runtime Services remain available
+ * - The kernel takes full control of the system
+ * - No more firmware calls can be made except runtime services
+ * 
+ * @param ImageHandle Handle to this bootloader image, used for protocol access
+ * @param SystemTable Pointer to UEFI System Table containing all UEFI services
+ * @return EFI_SUCCESS on successful boot (should never return),
+ *         or an error code if boot fails
+ * 
+ * @note If ExitBootServices fails on the first attempt, the memory map may have
+ *       changed. This function automatically retries once with an updated memory map.
+ * @note The kernel entry point is assumed to be at KERNEL_LOAD_ADDRESS (0x100000).
+ * @note The kernel receives a pointer to the BootInfo structure containing all
+ *       information needed to initialize the system.
  */
 EFI_STATUS efi_main(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE* SystemTable) {
     EFI_STATUS status;
diff --git a/kernel/include/kernel/apic.h b/kernel/include/kernel/apic.h
index dab4832..45f05e4 100644
--- a/kernel/include/kernel/apic.h
+++ b/kernel/include/kernel/apic.h
@@ -20,32 +20,131 @@
 #define APIC_IPI_STARTUP      0x600
 
 #ifdef __cplusplus
-// C++ APIC class
+/**
+ * @class APIC
+ * @brief Advanced Programmable Interrupt Controller manager for multicore systems
+ * 
+ * The APIC (Advanced Programmable Interrupt Controller) replaces the legacy 8259 PIC
+ * in modern x86-64 systems. Each CPU core has its own Local APIC that:
+ * - Receives interrupts from devices and other CPUs
+ * - Sends End-Of-Interrupt (EOI) signals
+ * - Sends Inter-Processor Interrupts (IPIs) to other cores
+ * - Manages per-core timer and performance monitoring interrupts
+ * 
+ * The Local APIC is memory-mapped at physical address 0xFEE00000 by default.
+ * All APIC operations are performed by reading/writing 32-bit registers at
+ * specific offsets from this base address.
+ * 
+ * Key APIC concepts:
+ * - Local APIC: One per CPU core, handles that core's interrupts
+ * - APIC ID: Unique identifier for each Local APIC (each CPU core)
+ * - IPI (Inter-Processor Interrupt): Interrupt sent from one CPU to another
+ * - EOI (End-Of-Interrupt): Signal that interrupt handling is complete
+ * - ICR (Interrupt Command Register): Used to send IPIs to other cores
+ */
 class APIC {
 private:
-    volatile uint32_t* apicBase;
+    volatile uint32_t* apicBase; ///< Pointer to APIC memory-mapped registers (0xFEE00000)
     
+    /**
+     * @brief Read a 32-bit value from an APIC register
+     * @param offset Register offset from APIC base (e.g., APIC_REG_ID)
+     * @return 32-bit register value
+     */
     uint32_t read(uint32_t offset) const;
+    
+    /**
+     * @brief Write a 32-bit value to an APIC register
+     * @param offset Register offset from APIC base
+     * @param value 32-bit value to write
+     */
     void write(uint32_t offset, uint32_t value);
 
 public:
+    /** @brief Constructor - initializes APIC base address to 0xFEE00000 */
     APIC();
     
+    /**
+     * @brief Check if APIC is available on this CPU
+     * @return true if APIC is present and can be used, false otherwise
+     * @note Uses CPUID instruction to check for APIC support (bit 9 of EDX)
+     */
     bool isAvailable() const;
+    
+    /**
+     * @brief Initialize the Local APIC for this CPU core
+     * @note Enables the APIC by setting bit 8 of the Spurious Interrupt Vector Register
+     * @note Sets Task Priority Register to 0 to accept all interrupts
+     * @note Must be called on each CPU core that will use the APIC
+     */
     void init();
+    
+    /**
+     * @brief Get the APIC ID of the current CPU core
+     * @return 8-bit APIC ID (unique identifier for this core)
+     * @note APIC IDs may not be sequential (e.g., 0, 2, 4, 6 on hyperthreaded systems)
+     */
     uint8_t getId() const;
+    
+    /**
+     * @brief Send End-Of-Interrupt signal to acknowledge interrupt completion
+     * @note Must be called at the end of every interrupt handler that uses the APIC
+     * @note Writing any value to the EOI register acknowledges the interrupt
+     */
     void sendEOI();
+    
+    /**
+     * @brief Send an Inter-Processor Interrupt to another CPU core
+     * @param destApicId APIC ID of the destination CPU core
+     * @param vector Interrupt vector number to deliver (0-255)
+     * @param deliveryMode Delivery mode flags (e.g., APIC_IPI_INIT, APIC_IPI_STARTUP)
+     * 
+     * IPIs are used for:
+     * - Starting application processors (APs) during SMP initialization
+     * - TLB shootdowns when changing page tables
+     * - Sending signals between cores
+     * - Emergency core wakeup or halt
+     * 
+     * @note Waits for previous IPI to complete before sending new one
+     */
     void sendIPI(uint8_t destApicId, uint8_t vector, uint32_t deliveryMode);
 };
 
 extern "C" {
 #endif
 
-// C-compatible APIC functions
+/* APIC - C-compatible interface */
+
+/**
+ * @brief Check if Local APIC is available on this system
+ * @return true if APIC is present, false otherwise
+ */
 bool apic_is_available(void);
+
+/**
+ * @brief Initialize the Local APIC for the current CPU core
+ * @note Must be called on each core before using APIC functionality
+ */
 void apic_init(void);
+
+/**
+ * @brief Get APIC ID of the current CPU core
+ * @return 8-bit APIC ID
+ */
 uint8_t apic_get_id(void);
+
+/**
+ * @brief Send End-Of-Interrupt signal
+ * @note Call at end of interrupt handlers that use APIC
+ */
 void apic_send_eoi(void);
+
+/**
+ * @brief Send Inter-Processor Interrupt to another core
+ * @param dest_apic_id Target core's APIC ID
+ * @param vector Interrupt vector number
+ * @param delivery_mode IPI delivery mode (APIC_IPI_INIT, APIC_IPI_STARTUP, etc.)
+ */
 void apic_send_ipi(uint8_t dest_apic_id, uint8_t vector, uint32_t delivery_mode);
 
 #ifdef __cplusplus
diff --git a/kernel/include/kernel/gdt.h b/kernel/include/kernel/gdt.h
index 5d29d04..86789b5 100644
--- a/kernel/include/kernel/gdt.h
+++ b/kernel/include/kernel/gdt.h
@@ -3,7 +3,25 @@
 
 #include <stdint.h>
 
-// GDT Entry structure
+/**
+ * @struct gdt_entry_t
+ * @brief Global Descriptor Table entry structure (8 bytes)
+ * 
+ * The GDT defines memory segments in protected/long mode. Each entry describes
+ * a segment with base address, limit (size), and access rights. In 64-bit long mode,
+ * segmentation is mostly disabled, but the GDT is still required for:
+ * - Code/Data segment selectors
+ * - Privilege level enforcement (Ring 0 kernel, Ring 3 user)
+ * - System call/interrupt handling
+ * 
+ * Each GDT entry is 8 bytes with the following layout:
+ * - limit_low (16 bits): Lower 16 bits of segment limit
+ * - base_low (16 bits): Lower 16 bits of base address
+ * - base_middle (8 bits): Middle 8 bits of base address
+ * - access (8 bits): Access flags (present, privilege, type)
+ * - granularity (8 bits): Upper 4 bits of limit + flags (granularity, size)
+ * - base_high (8 bits): Upper 8 bits of base address
+ */
 typedef struct {
     uint16_t limit_low;
     uint16_t base_low;
@@ -13,31 +31,77 @@ typedef struct {
     uint8_t base_high;
 } __attribute__((packed)) gdt_entry_t;
 
-// GDT Pointer structure
+/**
+ * @struct gdt_ptr_t
+ * @brief GDT pointer structure used by LGDT instruction
+ * 
+ * This structure is loaded into the CPU using the LGDT (Load GDT) instruction.
+ * It tells the CPU where the GDT is located in memory and how large it is.
+ * 
+ * - limit: Size of GDT in bytes minus 1
+ * - base: 64-bit linear address of the GDT
+ */
 typedef struct {
     uint16_t limit;
     uint64_t base;
 } __attribute__((packed)) gdt_ptr_t;
 
 #ifdef __cplusplus
-// C++ GDT class
+/**
+ * @class GDT
+ * @brief Global Descriptor Table manager for x86-64 long mode
+ * 
+ * The GDT (Global Descriptor Table) is a data structure used by x86 processors
+ * to define memory segments. Although segmentation is mostly flat in 64-bit mode,
+ * the GDT is still required for:
+ * - Defining code and data segments
+ * - Setting privilege levels (CPL 0 for kernel, CPL 3 for user)
+ * - Enabling system calls and interrupts
+ * 
+ * MetalOS uses a minimal 5-entry GDT:
+ * 0. Null descriptor (required by CPU, never used)
+ * 1. Kernel code segment (64-bit, ring 0, executable)
+ * 2. Kernel data segment (64-bit, ring 0, writable)
+ * 3. User code segment (64-bit, ring 3, executable)
+ * 4. User data segment (64-bit, ring 3, writable)
+ * 
+ * In 64-bit mode, segment bases and limits are mostly ignored, but the access
+ * rights (privilege level, executable flag) are still enforced.
+ */
 class GDT {
 private:
-    gdt_entry_t entries[5];
-    gdt_ptr_t gdtPtr;
+    gdt_entry_t entries[5]; ///< Array of 5 GDT entries
+    gdt_ptr_t gdtPtr;       ///< GDT pointer for LGDT instruction
     
+    /**
+     * @brief Set a GDT entry with specified parameters
+     * @param num Entry index (0-4)
+     * @param base Base address (mostly ignored in 64-bit mode)
+     * @param limit Segment limit (mostly ignored in 64-bit mode)
+     * @param access Access byte (present, DPL, type flags)
+     * @param gran Granularity byte (upper limit bits + flags)
+     */
     void setGate(int num, uint32_t base, uint32_t limit, uint8_t access, uint8_t gran);
 
 public:
+    /** @brief Constructor - initializes GDT pointer structure */
     GDT();
     
+    /**
+     * @brief Initialize the GDT and load it into the CPU
+     * @note Sets up 5 descriptors: null, kernel code/data, user code/data
+     * @note Calls gdt_flush() assembly function to load GDT using LGDT
+     */
     void init();
 };
 
 extern "C" {
 #endif
 
-// C-compatible GDT function
+/**
+ * @brief Initialize the Global Descriptor Table
+ * @note Must be called early in kernel initialization before enabling interrupts
+ */
 void gdt_init(void);
 
 #ifdef __cplusplus
diff --git a/kernel/include/kernel/interrupts.h b/kernel/include/kernel/interrupts.h
index f353b3e..449a45a 100644
--- a/kernel/include/kernel/interrupts.h
+++ b/kernel/include/kernel/interrupts.h
@@ -3,53 +3,140 @@
 
 #include <stdint.h>
 
-// IDT Entry structure
+/**
+ * @struct idt_entry_t
+ * @brief Interrupt Descriptor Table entry structure (16 bytes in 64-bit mode)
+ * 
+ * Each IDT entry describes how to handle a specific interrupt or exception.
+ * The IDT contains 256 entries for interrupt vectors 0-255:
+ * - Vectors 0-31: CPU exceptions (divide by zero, page fault, etc.)
+ * - Vectors 32-47: Hardware IRQs (after PIC remap)
+ * - Vectors 48-255: Available for software interrupts
+ * 
+ * In 64-bit mode, IDT entries are 16 bytes and point to interrupt handler functions.
+ */
 typedef struct {
-    uint16_t offset_low;    // Offset bits 0-15
-    uint16_t selector;      // Code segment selector
-    uint8_t ist;            // Interrupt Stack Table offset
-    uint8_t type_attr;      // Type and attributes
-    uint16_t offset_mid;    // Offset bits 16-31
-    uint32_t offset_high;   // Offset bits 32-63
-    uint32_t zero;          // Reserved
+    uint16_t offset_low;    ///< Lower 16 bits of handler function address
+    uint16_t selector;      ///< Code segment selector (typically 0x08 for kernel code)
+    uint8_t ist;            ///< Interrupt Stack Table offset (0 = use current stack)
+    uint8_t type_attr;      ///< Type and attributes (present, DPL, gate type)
+    uint16_t offset_mid;    ///< Middle 16 bits of handler function address
+    uint32_t offset_high;   ///< Upper 32 bits of handler function address
+    uint32_t zero;          ///< Reserved, must be zero
 } __attribute__((packed)) idt_entry_t;
 
-// IDT Pointer structure
+/**
+ * @struct idt_ptr_t
+ * @brief IDT pointer structure used by LIDT instruction
+ * 
+ * This structure is loaded into the CPU using the LIDT (Load IDT) instruction.
+ * It tells the CPU where the IDT is located in memory and how large it is.
+ */
 typedef struct {
-    uint16_t limit;
-    uint64_t base;
+    uint16_t limit;  ///< Size of IDT in bytes minus 1
+    uint64_t base;   ///< 64-bit linear address of the IDT
 } __attribute__((packed)) idt_ptr_t;
 
-// CPU registers state (for interrupt handlers)
+/**
+ * @struct registers_t
+ * @brief CPU register state saved during interrupt handling
+ * 
+ * This structure represents the complete CPU state at the time an interrupt occurred.
+ * It is pushed onto the stack by the interrupt handler assembly code and passed to
+ * the C interrupt handler. It includes:
+ * - General purpose registers (rax, rbx, rcx, etc.)
+ * - Interrupt number and error code
+ * - Execution state (rip, rsp, rflags) automatically pushed by CPU
+ */
 typedef struct {
-    uint64_t r15, r14, r13, r12, r11, r10, r9, r8;
-    uint64_t rbp, rdi, rsi, rdx, rcx, rbx, rax;
-    uint64_t int_no, err_code;
-    uint64_t rip, cs, rflags, rsp, ss;
+    uint64_t r15, r14, r13, r12, r11, r10, r9, r8;  ///< Extended GP registers
+    uint64_t rbp, rdi, rsi, rdx, rcx, rbx, rax;     ///< Standard GP registers
+    uint64_t int_no, err_code;                       ///< Interrupt number and error code
+    uint64_t rip, cs, rflags, rsp, ss;              ///< CPU state (pushed by CPU)
 } __attribute__((packed)) registers_t;
 
 #ifdef __cplusplus
-// C++ InterruptManager class
+/**
+ * @class InterruptManager
+ * @brief Manages the Interrupt Descriptor Table and interrupt handling
+ * 
+ * The InterruptManager class is responsible for:
+ * - Setting up the IDT with 256 interrupt vectors
+ * - Installing interrupt handler functions
+ * - Remapping the legacy 8259 PIC (Programmable Interrupt Controller)
+ * - Dispatching interrupts to appropriate handlers
+ * - Sending End-Of-Interrupt signals to PIC or APIC
+ * 
+ * Key concepts:
+ * - ISR (Interrupt Service Routine): Handles CPU exceptions (0-31)
+ * - IRQ (Interrupt Request): Handles hardware interrupts (32-47 after remap)
+ * - PIC (Programmable Interrupt Controller): Legacy 8259 chip that manages IRQs
+ * - APIC (Advanced PIC): Modern interrupt controller for multicore systems
+ * 
+ * The PIC is remapped because the default IRQ vectors (0-15) conflict with
+ * CPU exception vectors. We remap IRQs to vectors 32-47.
+ */
 class InterruptManager {
 private:
-    idt_entry_t idt[256];
-    idt_ptr_t idtPtr;
+    idt_entry_t idt[256];  ///< Array of 256 IDT entries
+    idt_ptr_t idtPtr;      ///< IDT pointer for LIDT instruction
     
+    /**
+     * @brief Set an IDT entry to point to an interrupt handler
+     * @param num Interrupt vector number (0-255)
+     * @param handler Address of interrupt handler function
+     * @param selector Code segment selector (0x08 for kernel code)
+     * @param flags Type and attribute flags (present, DPL, gate type)
+     */
     void setGate(uint8_t num, uint64_t handler, uint16_t selector, uint8_t flags);
+    
+    /**
+     * @brief Remap the 8259 PIC to use different IRQ vectors
+     * @note Remaps IRQ 0-7 to vectors 32-39, IRQ 8-15 to vectors 40-47
+     * @note Masks all IRQs initially; they must be explicitly unmasked to receive
+     */
     void remapPIC();
 
 public:
+    /** @brief Constructor - initializes IDT pointer structure */
     InterruptManager();
     
+    /**
+     * @brief Initialize the IDT and enable interrupts
+     * @note Sets up all 256 IDT entries
+     * @note Installs exception handlers (ISR 0-31)
+     * @note Remaps and configures the PIC
+     * @note Installs IRQ handlers (IRQ 0-15 → vectors 32-47)
+     * @note Loads IDT using LIDT instruction
+     * @note Enables interrupts using STI instruction
+     */
     void init();
+    
+    /**
+     * @brief Handle an interrupt by dispatching to appropriate handler
+     * @param regs Pointer to saved CPU register state
+     * @note Calls specific handlers based on interrupt number
+     * @note Sends EOI to PIC or APIC after handling
+     */
     void handleInterrupt(registers_t* regs);
 };
 
 extern "C" {
 #endif
 
-// C-compatible functions
+/* Interrupt Management - C-compatible interface */
+
+/**
+ * @brief Initialize the Interrupt Descriptor Table and enable interrupts
+ * @note Must be called after GDT initialization
+ */
 void idt_init(void);
+
+/**
+ * @brief Main interrupt handler dispatcher (called from assembly ISRs)
+ * @param regs Pointer to saved CPU register state
+ * @note Should only be called from interrupt context
+ */
 void interrupt_handler(registers_t* regs);
 
 #ifdef __cplusplus
diff --git a/kernel/include/kernel/memory.h b/kernel/include/kernel/memory.h
index c34b7f4..209760c 100644
--- a/kernel/include/kernel/memory.h
+++ b/kernel/include/kernel/memory.h
@@ -9,58 +9,230 @@
 #define PAGE_SIZE 4096
 
 #ifdef __cplusplus
-// C++ PhysicalMemoryManager class
+/**
+ * @class PhysicalMemoryManager
+ * @brief Manages physical memory pages using a bitmap-based allocator
+ * 
+ * This class implements a simple physical memory manager that tracks available
+ * 4KB pages using a bitmap. Each bit in the bitmap represents one page:
+ * - 0 = page is free and available for allocation
+ * - 1 = page is in use
+ * 
+ * The bitmap supports up to 128MB of physical memory (32768 bytes * 8 bits/byte
+ * * 4KB per page = 128MB). Memory is assumed to start at physical address 0x01000000
+ * (16MB) to avoid conflicts with legacy hardware and the kernel itself.
+ * 
+ * This is a very simple allocator suitable for a minimal kernel. It does not:
+ * - Handle memory regions with different properties
+ * - Support allocation of multiple contiguous pages at once
+ * - Track memory usage per process (single application OS)
+ */
 class PhysicalMemoryManager {
 private:
-    uint8_t pageBitmap[32768];  // Supports up to 128MB with 4KB pages
-    uint64_t totalPages;
-    uint64_t usedPages;
+    uint8_t pageBitmap[32768];  ///< Bitmap tracking page allocation (128MB / 4KB pages)
+    uint64_t totalPages;         ///< Total number of pages managed
+    uint64_t usedPages;          ///< Number of pages currently allocated
 
 public:
+    /** @brief Constructor - initializes bitmap to all zeros (all pages free) */
     PhysicalMemoryManager();
     
+    /**
+     * @brief Initialize the physical memory manager with boot information
+     * @param bootInfo Boot information from bootloader (currently unused, assumes 128MB)
+     * @note Currently hardcoded to manage 128MB starting at 16MB physical address
+     */
     void init(BootInfo* bootInfo);
+    
+    /**
+     * @brief Allocate a single 4KB page from physical memory
+     * @return Physical address of allocated page, or nullptr if out of memory
+     * @note Returns first available page found in bitmap (no specific allocation strategy)
+     */
     void* allocPage();
+    
+    /**
+     * @brief Free a previously allocated page, returning it to the available pool
+     * @param page Physical address of the page to free
+     * @note Does nothing if the page address is invalid or out of managed range
+     */
     void freePage(void* page);
+    
+    /**
+     * @brief Get total amount of physical memory managed (in bytes)
+     * @return Total memory in bytes (totalPages * PAGE_SIZE)
+     */
     uint64_t getTotalMemory() const;
+    
+    /**
+     * @brief Get amount of free physical memory available (in bytes)
+     * @return Free memory in bytes ((totalPages - usedPages) * PAGE_SIZE)
+     */
     uint64_t getFreeMemory() const;
 };
 
-// C++ HeapAllocator class
+/**
+ * @class HeapAllocator
+ * @brief Simple bump allocator for kernel heap memory
+ * 
+ * This class implements a very simple "bump" or "arena" allocator. Memory is
+ * allocated by simply incrementing a pointer (heapCurrent) forward. This is
+ * extremely fast but has limitations:
+ * - Cannot free individual allocations (free() is a no-op)
+ * - Memory is only reclaimed when the entire heap is reset
+ * - Can fragment if many small allocations are made
+ * 
+ * For a single-application OS that doesn't need complex memory management,
+ * this simple allocator is sufficient and has minimal code size.
+ * 
+ * All allocations are aligned to 16-byte boundaries for performance and
+ * compatibility with SSE/AVX instructions.
+ */
 class HeapAllocator {
 private:
-    uint8_t* heapStart;
-    uint8_t* heapCurrent;
-    uint8_t* heapEnd;
+    uint8_t* heapStart;   ///< Start address of the heap region
+    uint8_t* heapCurrent; ///< Current allocation pointer (bump pointer)
+    uint8_t* heapEnd;     ///< End address of the heap region (exclusive)
 
 public:
+    /** @brief Constructor - initializes all pointers to null */
     HeapAllocator();
     
+    /**
+     * @brief Initialize the heap allocator with a memory region
+     * @param start Starting address of heap memory region
+     * @param size Size of heap region in bytes
+     * @note The memory region should be obtained from the physical memory manager
+     */
     void init(void* start, size_t size);
+    
+    /**
+     * @brief Allocate memory from the heap
+     * @param size Number of bytes to allocate
+     * @return Pointer to allocated memory, or nullptr if out of heap space
+     * @note Allocation is aligned to 16-byte boundaries
+     * @note This is a bump allocator - simply moves heapCurrent forward
+     */
     void* alloc(size_t size);
+    
+    /**
+     * @brief Allocate and zero-initialize an array of objects
+     * @param num Number of elements
+     * @param size Size of each element in bytes
+     * @return Pointer to allocated and zeroed memory, or nullptr if out of space
+     * @note Equivalent to alloc(num * size) followed by memset to zero
+     */
     void* calloc(size_t num, size_t size);
+    
+    /**
+     * @brief Free allocated memory (no-op in bump allocator)
+     * @param ptr Pointer to memory to free (ignored)
+     * @note This function does nothing - bump allocators cannot free individual allocations
+     * @todo Replace with a proper allocator if individual free() is needed
+     */
     void free(void* ptr);
 };
 
 extern "C" {
 #endif
 
-// C-compatible physical memory manager
+/* Physical Memory Manager - C-compatible interface */
+
+/**
+ * @brief Initialize the physical memory manager
+ * @param boot_info Boot information from bootloader (contains memory map)
+ * @note This must be called early in kernel initialization, before any memory allocation
+ */
 void pmm_init(BootInfo* boot_info);
+
+/**
+ * @brief Allocate a single 4KB physical memory page
+ * @return Physical address of allocated page, or NULL if out of memory
+ * @note Returns first available page from the page bitmap
+ */
 void* pmm_alloc_page(void);
+
+/**
+ * @brief Free a previously allocated physical memory page
+ * @param page Physical address of the page to free
+ */
 void pmm_free_page(void* page);
+
+/**
+ * @brief Get total amount of physical memory managed by PMM
+ * @return Total memory in bytes
+ */
 uint64_t pmm_get_total_memory(void);
+
+/**
+ * @brief Get amount of free physical memory currently available
+ * @return Free memory in bytes
+ */
 uint64_t pmm_get_free_memory(void);
 
-// C-compatible kernel heap allocator
+/* Kernel Heap Allocator - C-compatible interface */
+
+/**
+ * @brief Initialize the kernel heap allocator
+ * @param start Starting address of heap memory region
+ * @param size Size of heap region in bytes
+ * @note The memory region should be allocated from physical memory manager first
+ */
 void heap_init(void* start, size_t size);
+
+/**
+ * @brief Allocate memory from kernel heap (like malloc)
+ * @param size Number of bytes to allocate
+ * @return Pointer to allocated memory, or NULL if out of heap space
+ * @note Memory is 16-byte aligned
+ * @note Cannot be freed individually (bump allocator limitation)
+ */
 void* kmalloc(size_t size);
+
+/**
+ * @brief Allocate and zero-initialize array memory from kernel heap (like calloc)
+ * @param num Number of elements
+ * @param size Size of each element in bytes
+ * @return Pointer to allocated and zeroed memory, or NULL if out of space
+ */
 void* kcalloc(size_t num, size_t size);
+
+/**
+ * @brief Free kernel heap memory (no-op in current implementation)
+ * @param ptr Pointer to memory to free
+ * @note This function currently does nothing - bump allocator cannot free individual allocations
+ */
 void kfree(void* ptr);
 
-// Memory utility functions
+/* Memory Utility Functions */
+
+/**
+ * @brief Fill memory with a constant byte value
+ * @param dest Pointer to memory block to fill
+ * @param val Value to set (converted to unsigned char)
+ * @param count Number of bytes to set
+ * @return Pointer to dest
+ * @note Simple byte-by-byte implementation
+ */
 void* memset(void* dest, int val, size_t count);
+
+/**
+ * @brief Copy memory from source to destination
+ * @param dest Pointer to destination buffer
+ * @param src Pointer to source buffer
+ * @param count Number of bytes to copy
+ * @return Pointer to dest
+ * @warning Memory regions must not overlap
+ */
 void* memcpy(void* dest, const void* src, size_t count);
+
+/**
+ * @brief Compare two memory blocks
+ * @param s1 Pointer to first memory block
+ * @param s2 Pointer to second memory block
+ * @param count Number of bytes to compare
+ * @return 0 if equal, negative if s1 < s2, positive if s1 > s2
+ */
 int memcmp(const void* s1, const void* s2, size_t count);
 
 #ifdef __cplusplus
diff --git a/kernel/include/kernel/pci.h b/kernel/include/kernel/pci.h
index 5cc160d..67a006c 100644
--- a/kernel/include/kernel/pci.h
+++ b/kernel/include/kernel/pci.h
@@ -10,22 +10,42 @@
 // Maximum devices we'll track
 #define MAX_PCI_DEVICES 256
 
-// PCI Device Structure
+/**
+ * @struct pci_device_t
+ * @brief Represents a PCI device discovered during bus enumeration
+ * 
+ * PCI (Peripheral Component Interconnect) is the standard bus for connecting
+ * hardware devices like GPUs, network cards, storage controllers, etc.
+ * 
+ * Each PCI device is identified by:
+ * - Bus, device, function numbers (BDF): Physical location
+ * - Vendor ID and Device ID: Identifies manufacturer and model
+ * - Class code, subclass, prog_if: Type of device (GPU, storage, etc.)
+ * - Base Address Registers (BARs): Memory/IO regions used by device
+ * 
+ * Example vendor IDs:
+ * - 0x1002: AMD/ATI (used by Radeon GPUs)
+ * - 0x10DE: NVIDIA
+ * - 0x8086: Intel
+ */
 typedef struct {
-    uint8_t bus;
-    uint8_t device;
-    uint8_t function;
-    uint16_t vendor_id;
-    uint16_t device_id;
-    uint8_t class_code;
-    uint8_t subclass;
-    uint8_t prog_if;
-    uint8_t revision_id;
-    uint32_t bar[6];  // Base Address Registers
+    uint8_t bus;         ///< PCI bus number (0-255)
+    uint8_t device;      ///< Device number on bus (0-31)
+    uint8_t function;    ///< Function number within device (0-7)
+    uint16_t vendor_id;  ///< Vendor identifier (e.g., 0x1002 for AMD)
+    uint16_t device_id;  ///< Device identifier (specific model)
+    uint8_t class_code;  ///< Device class (0x03 = display controller)
+    uint8_t subclass;    ///< Device subclass (0x00 = VGA compatible)
+    uint8_t prog_if;     ///< Programming interface
+    uint8_t revision_id; ///< Device revision
+    uint32_t bar[6];     ///< Base Address Registers (memory/IO regions)
 } pci_device_t;
 
 #ifdef __cplusplus
-// C++ PCIDevice class-compatible structure
+/**
+ * @struct PCIDevice
+ * @brief C++ equivalent of pci_device_t for type compatibility
+ */
 struct PCIDevice {
     uint8_t bus;
     uint8_t device;
@@ -39,32 +59,133 @@ struct PCIDevice {
     uint32_t bar[6];
 };
 
-// C++ PCIManager class
+/**
+ * @class PCIManager
+ * @brief Manages PCI bus enumeration and device configuration
+ * 
+ * The PCIManager scans the PCI bus hierarchy to discover all connected devices.
+ * PCI configuration is done through two I/O ports:
+ * - 0xCF8 (CONFIG_ADDRESS): Specifies which device and register to access
+ * - 0xCFC (CONFIG_DATA): Reads/writes the configuration register
+ * 
+ * PCI devices are organized hierarchically:
+ * - Up to 256 buses
+ * - Up to 32 devices per bus
+ * - Up to 8 functions per device (most devices have only function 0)
+ * 
+ * Each device has 256 bytes of configuration space containing:
+ * - Device identification (vendor/device ID)
+ * - Command and status registers
+ * - Base Address Registers (BARs) for memory-mapped I/O
+ * - Interrupt configuration
+ * - Device-specific registers
+ */
 class PCIManager {
 private:
-    PCIDevice devices[MAX_PCI_DEVICES];
-    uint32_t deviceCount;
+    PCIDevice devices[MAX_PCI_DEVICES];  ///< Array of discovered devices
+    uint32_t deviceCount;                ///< Number of devices found
     
+    /**
+     * @brief Probe a specific PCI function and add to device list
+     * @param bus Bus number
+     * @param device Device number
+     * @param function Function number
+     * @note Reads device identification and stores in devices array
+     */
     void probeDevice(uint8_t bus, uint8_t device, uint8_t function);
 
 public:
+    /** @brief Constructor - initializes device count to 0 */
     PCIManager();
     
+    /**
+     * @brief Scan all PCI buses and enumerate devices
+     * @note Scans all 256 buses, 32 devices per bus, up to 8 functions per device
+     * @note Skips non-existent devices (vendor ID 0xFFFF)
+     */
     void init();
+    
+    /**
+     * @brief Read a 32-bit value from PCI configuration space
+     * @param bus Bus number
+     * @param device Device number
+     * @param function Function number
+     * @param offset Register offset (must be 4-byte aligned)
+     * @return 32-bit configuration register value
+     */
     uint32_t readConfig(uint8_t bus, uint8_t device, uint8_t function, uint8_t offset);
+    
+    /**
+     * @brief Write a 32-bit value to PCI configuration space
+     * @param bus Bus number
+     * @param device Device number
+     * @param function Function number
+     * @param offset Register offset (must be 4-byte aligned)
+     * @param value Value to write
+     */
     void writeConfig(uint8_t bus, uint8_t device, uint8_t function, uint8_t offset, uint32_t value);
+    
+    /**
+     * @brief Find a PCI device by vendor and device ID
+     * @param vendor_id Vendor identifier (e.g., 0x1002 for AMD)
+     * @param device_id Device identifier
+     * @return Pointer to PCIDevice if found, nullptr if not found
+     * @note Useful for finding specific hardware (e.g., "find AMD RX 6600 GPU")
+     */
     PCIDevice* findDevice(uint16_t vendor_id, uint16_t device_id);
+    
+    /**
+     * @brief Enable bus mastering for a PCI device
+     * @param dev Pointer to PCI device
+     * @note Bus mastering allows device to perform DMA (Direct Memory Access)
+     * @note Required for most devices to function properly
+     */
     void enableBusMastering(PCIDevice* dev);
 };
 
 extern "C" {
 #endif
 
-// C-compatible PCI functions
+/* PCI - C-compatible interface */
+
+/**
+ * @brief Initialize PCI subsystem and enumerate all devices
+ * @note Should be called during kernel initialization
+ */
 void pci_init(void);
+
+/**
+ * @brief Read from PCI configuration space
+ * @param bus Bus number
+ * @param device Device number
+ * @param function Function number
+ * @param offset Register offset
+ * @return 32-bit register value
+ */
 uint32_t pci_read_config(uint8_t bus, uint8_t device, uint8_t function, uint8_t offset);
+
+/**
+ * @brief Write to PCI configuration space
+ * @param bus Bus number
+ * @param device Device number
+ * @param function Function number
+ * @param offset Register offset
+ * @param value Value to write
+ */
 void pci_write_config(uint8_t bus, uint8_t device, uint8_t function, uint8_t offset, uint32_t value);
+
+/**
+ * @brief Find a PCI device by vendor and device ID
+ * @param vendor_id Vendor identifier
+ * @param device_id Device identifier
+ * @return Pointer to device structure if found, NULL otherwise
+ */
 pci_device_t* pci_find_device(uint16_t vendor_id, uint16_t device_id);
+
+/**
+ * @brief Enable bus mastering for a PCI device
+ * @param dev Pointer to PCI device structure
+ */
 void pci_enable_bus_mastering(pci_device_t* dev);
 
 #ifdef __cplusplus
diff --git a/kernel/include/kernel/smp.h b/kernel/include/kernel/smp.h
index 03d8b1b..7d50b90 100644
--- a/kernel/include/kernel/smp.h
+++ b/kernel/include/kernel/smp.h
@@ -7,46 +7,158 @@
 // Maximum number of CPUs we support
 #define MAX_CPUS 16
 
-// Per-CPU data structure
+/**
+ * @struct cpu_info_t
+ * @brief Per-CPU information structure
+ * 
+ * Stores information about each CPU core in a multicore system.
+ */
 typedef struct {
-    uint8_t cpu_id;
-    uint8_t apic_id;
-    bool online;
-    uint64_t kernel_stack;
+    uint8_t cpu_id;       ///< Logical CPU ID (0, 1, 2, ...)
+    uint8_t apic_id;      ///< Physical APIC ID (may not be sequential)
+    bool online;          ///< True if CPU is initialized and running
+    uint64_t kernel_stack; ///< Kernel stack pointer for this CPU (future use)
 } cpu_info_t;
 
 #ifdef __cplusplus
-// C++ SMPManager class
+/**
+ * @class SMPManager
+ * @brief Symmetric Multi-Processing manager for multicore CPU initialization
+ * 
+ * SMP (Symmetric Multi-Processing) allows an operating system to use multiple
+ * CPU cores. In x86-64 systems:
+ * - One core (BSP - Bootstrap Processor) starts first and initializes the system
+ * - Other cores (APs - Application Processors) must be explicitly started by the BSP
+ * 
+ * The SMP initialization process:
+ * 1. BSP initializes its own Local APIC
+ * 2. BSP discovers other cores (from ACPI tables or by probing)
+ * 3. BSP sends INIT IPI to each AP (reset the core)
+ * 4. BSP sends SIPI (Startup IPI) with entry point address to each AP
+ * 5. AP starts executing at entry point, initializes itself
+ * 6. AP marks itself as online
+ * 
+ * The SIPI protocol requires:
+ * - A small "trampoline" code in low memory (below 1MB) in real mode
+ * - Two SIPI messages sent with specific timing (per Intel spec)
+ * - Waiting for AP to signal it's online
+ * 
+ * Once all cores are initialized, they can be used for parallel processing,
+ * though MetalOS currently only uses the BSP for application execution.
+ */
 class SMPManager {
 private:
-    cpu_info_t cpuInfo[MAX_CPUS];
-    uint8_t cpuCount;
-    bool smpEnabled;
+    cpu_info_t cpuInfo[MAX_CPUS];  ///< Information for each CPU core
+    uint8_t cpuCount;               ///< Total number of CPU cores detected
+    bool smpEnabled;                ///< True if multiple cores are available
     
+    /**
+     * @brief Initialize CPU info structure for a core
+     * @param cpuId Logical CPU ID
+     * @param apicId Physical APIC ID
+     */
     void initCPU(uint8_t cpuId, uint8_t apicId);
+    
+    /**
+     * @brief Busy-wait delay (approximate)
+     * @param microseconds Delay duration in microseconds
+     * @note Not precise, used for timing during AP startup
+     */
     void delay(uint32_t microseconds);
+    
+    /**
+     * @brief Start an Application Processor using SIPI protocol
+     * @param apicId APIC ID of the AP to start
+     * @return true if AP started successfully, false on timeout
+     * @note Sends INIT IPI followed by two SIPI messages
+     */
     bool startAP(uint8_t apicId);
 
 public:
+    /** @brief Constructor - initializes BSP as first CPU */
     SMPManager();
     
+    /**
+     * @brief Initialize SMP subsystem and start all available CPU cores
+     * @note Checks for APIC availability
+     * @note Initializes BSP's APIC
+     * @note Attempts to start up to MAX_CPUS cores
+     * @note Falls back to single-core mode if APIC unavailable
+     */
     void init();
+    
+    /**
+     * @brief Get total number of CPU cores detected
+     * @return Number of CPU cores (at least 1)
+     */
     uint8_t getCPUCount() const;
+    
+    /**
+     * @brief Get logical ID of the currently executing CPU core
+     * @return CPU ID (0 for BSP)
+     */
     uint8_t getCurrentCPU() const;
+    
+    /**
+     * @brief Check if SMP is enabled (multiple cores available)
+     * @return true if multiple cores detected, false if single-core
+     */
     bool isEnabled() const;
+    
+    /**
+     * @brief Get information structure for a specific CPU
+     * @param cpuId Logical CPU ID
+     * @return Pointer to cpu_info_t structure, or nullptr if invalid ID
+     */
     cpu_info_t* getCPUInfo(uint8_t cpuId);
+    
+    /**
+     * @brief Mark a CPU as online (called by AP during initialization)
+     * @param cpuId Logical CPU ID
+     */
     void markCPUOnline(uint8_t cpuId);
 };
 
 extern "C" {
 #endif
 
-// C-compatible SMP functions
+/* SMP - C-compatible interface */
+
+/**
+ * @brief Initialize SMP subsystem and start all CPU cores
+ * @note Should be called after APIC initialization
+ */
 void smp_init(void);
+
+/**
+ * @brief Get number of CPU cores detected
+ * @return Number of CPU cores
+ */
 uint8_t smp_get_cpu_count(void);
+
+/**
+ * @brief Get ID of current CPU core
+ * @return Logical CPU ID
+ */
 uint8_t smp_get_current_cpu(void);
+
+/**
+ * @brief Check if SMP is enabled
+ * @return true if multiple cores available, false if single-core
+ */
 bool smp_is_enabled(void);
+
+/**
+ * @brief Get CPU information structure
+ * @param cpu_id Logical CPU ID
+ * @return Pointer to cpu_info_t structure or NULL
+ */
 cpu_info_t* smp_get_cpu_info(uint8_t cpu_id);
+
+/**
+ * @brief Mark CPU as online (called by AP during startup)
+ * @param cpu_id Logical CPU ID
+ */
 void smp_cpu_online(uint8_t cpu_id);
 
 #ifdef __cplusplus
diff --git a/kernel/include/kernel/spinlock.h b/kernel/include/kernel/spinlock.h
index dd9a3ca..9f7f3e0 100644
--- a/kernel/include/kernel/spinlock.h
+++ b/kernel/include/kernel/spinlock.h
@@ -5,34 +5,120 @@
 #include <stdbool.h>
 
 #ifdef __cplusplus
-// C++ Spinlock class
+/**
+ * @class Spinlock
+ * @brief Simple spinlock for multicore synchronization
+ * 
+ * A spinlock is a synchronization primitive used to protect shared data in
+ * multicore systems. Unlike a mutex that blocks (sleeps), a spinlock "spins"
+ * in a tight loop waiting for the lock to become available.
+ * 
+ * Spinlocks are appropriate when:
+ * - Critical sections are very short (a few instructions)
+ * - You're in interrupt context (can't sleep)
+ * - Lock contention is rare
+ * 
+ * Spinlocks should NOT be used when:
+ * - Critical section is long (wastes CPU cycles)
+ * - Lock might be held for unpredictable time
+ * - You can use a sleeping lock instead
+ * 
+ * Implementation uses x86 XCHG instruction which is:
+ * - Atomic (indivisible operation)
+ * - Automatically locked (works across multiple cores)
+ * - Sequentially consistent (no memory reordering issues)
+ * 
+ * The PAUSE instruction in the spin loop:
+ * - Reduces power consumption while spinning
+ * - Improves performance on hyperthreaded CPUs
+ * - Signals to CPU that we're in a spin-wait loop
+ */
 class Spinlock {
 private:
-    volatile uint32_t lock;
+    volatile uint32_t lock;  ///< Lock state: 0 = unlocked, 1 = locked
 
 public:
+    /** @brief Constructor - initializes lock to unlocked state */
     Spinlock();
     
+    /**
+     * @brief Initialize spinlock (set to unlocked)
+     * @note Can be used to re-initialize an existing spinlock
+     */
     void init();
+    
+    /**
+     * @brief Acquire the spinlock, spinning until available
+     * @note Will loop indefinitely if lock is never released
+     * @note Disables interrupts on this CPU while spinning (not implemented here)
+     */
     void acquire();
+    
+    /**
+     * @brief Try to acquire spinlock without blocking
+     * @return true if lock was acquired, false if already locked
+     * @note Returns immediately without spinning
+     */
     bool tryAcquire();
+    
+    /**
+     * @brief Release the spinlock
+     * @note Must only be called by the CPU that acquired the lock
+     * @note Includes memory barrier to ensure all writes are visible
+     */
     void release();
+    
+    /**
+     * @brief Check if spinlock is currently locked
+     * @return true if locked, false if unlocked
+     * @note Result may be stale immediately after checking
+     */
     bool isLocked() const;
 };
 
 extern "C" {
 #endif
 
-// C-compatible spinlock structure
+/**
+ * @struct spinlock_t
+ * @brief C-compatible spinlock structure
+ */
 typedef struct {
-    volatile uint32_t lock;
+    volatile uint32_t lock;  ///< Lock state: 0 = unlocked, 1 = locked
 } spinlock_t;
 
-// C-compatible functions
+/* Spinlock - C-compatible interface */
+
+/**
+ * @brief Initialize a spinlock
+ * @param lock Pointer to spinlock structure
+ */
 void spinlock_init(spinlock_t* lock);
+
+/**
+ * @brief Acquire a spinlock (spin until available)
+ * @param lock Pointer to spinlock structure
+ */
 void spinlock_acquire(spinlock_t* lock);
+
+/**
+ * @brief Try to acquire spinlock without blocking
+ * @param lock Pointer to spinlock structure
+ * @return true if acquired, false if already locked
+ */
 bool spinlock_try_acquire(spinlock_t* lock);
+
+/**
+ * @brief Release a spinlock
+ * @param lock Pointer to spinlock structure
+ */
 void spinlock_release(spinlock_t* lock);
+
+/**
+ * @brief Check if spinlock is locked
+ * @param lock Pointer to spinlock structure
+ * @return true if locked, false if unlocked
+ */
 bool spinlock_is_locked(spinlock_t* lock);
 
 #ifdef __cplusplus
diff --git a/kernel/include/kernel/timer.h b/kernel/include/kernel/timer.h
index 2dc83c4..1af0f96 100644
--- a/kernel/include/kernel/timer.h
+++ b/kernel/include/kernel/timer.h
@@ -7,27 +7,93 @@
 #define TIMER_FREQUENCY 1000  // 1ms per tick
 
 #ifdef __cplusplus
-// C++ Timer class
+/**
+ * @class Timer
+ * @brief Programmable Interval Timer (PIT) manager for system timing
+ * 
+ * The PIT (Programmable Interval Timer), also known as the 8253/8254 chip,
+ * generates periodic timer interrupts at a configurable frequency. It's used for:
+ * - System timekeeping (tracking elapsed time)
+ * - Scheduling and preemption (in multi-tasking systems)
+ * - Delays and timeouts
+ * 
+ * The PIT has a base frequency of 1.193182 MHz. By programming a divisor,
+ * we can generate interrupts at lower frequencies. For example:
+ * - Divisor 1193 → ~1000 Hz (1ms per tick) ← MetalOS uses this
+ * - Divisor 119318 → ~10 Hz (100ms per tick)
+ * 
+ * Timer interrupts are delivered as IRQ0, which is mapped to interrupt vector 32
+ * after the PIC is remapped.
+ * 
+ * @note The PIT is legacy hardware but still commonly available. Modern systems
+ *       may use HPET (High Precision Event Timer) or APIC timer instead.
+ */
 class Timer {
 private:
-    volatile uint64_t ticks;
+    volatile uint64_t ticks; ///< Number of timer interrupts received since initialization
 
 public:
+    /** @brief Constructor - initializes tick counter to 0 */
     Timer();
     
+    /**
+     * @brief Initialize the PIT with specified interrupt frequency
+     * @param frequency Desired interrupt frequency in Hz (e.g., 1000 for 1ms ticks)
+     * @note Calculates divisor = 1193182 / frequency and programs the PIT
+     * @note Unmasks IRQ0 in the PIC to enable timer interrupts
+     */
     void init(uint32_t frequency);
+    
+    /**
+     * @brief Get the current tick count
+     * @return Number of timer ticks since initialization
+     * @note At 1000 Hz, each tick represents 1 millisecond
+     */
     uint64_t getTicks() const;
+    
+    /**
+     * @brief Wait for a specified number of timer ticks
+     * @param waitTicks Number of ticks to wait
+     * @note Uses HLT instruction to save power while waiting
+     * @note Blocking wait - CPU will be idle during this time
+     */
     void wait(uint32_t waitTicks) const;
+    
+    /**
+     * @brief Handle timer interrupt (called from IRQ0 handler)
+     * @note Increments the tick counter
+     * @note Should be called from interrupt context only
+     */
     void handleInterrupt();
 };
 
 extern "C" {
 #endif
 
-// C-compatible timer functions
+/* Timer - C-compatible interface */
+
+/**
+ * @brief Initialize the Programmable Interval Timer
+ * @param frequency Interrupt frequency in Hz (typically 1000 for 1ms ticks)
+ */
 void timer_init(uint32_t frequency);
+
+/**
+ * @brief Get current timer tick count
+ * @return Number of ticks since timer initialization
+ */
 uint64_t timer_get_ticks(void);
+
+/**
+ * @brief Wait for specified number of timer ticks
+ * @param ticks Number of ticks to wait
+ */
 void timer_wait(uint32_t ticks);
+
+/**
+ * @brief Timer interrupt handler (called from IRQ0)
+ * @note Should only be called from interrupt context
+ */
 void timer_handler(void);
 
 #ifdef __cplusplus