Development #40
Development #40
Conversation
WalkthroughThis update refactors the kernel initialization process by splitting system setup into bootstrap and higher-half stages, introduces explicit page table mapping routines, and adds new assembly helpers for paging. It also improves memory reservation logic, defines kernel memory boundaries in the linker script, and introduces new macros and function prototypes for virtual memory management. Changes
Sequence Diagram(s)sequenceDiagram
participant Bootloader
participant KernelMain
participant Memory
participant VMem
participant AsmHelpers
participant KernelMainHigherHalf
Bootloader->>KernelMain: Jump to KernelMain
KernelMain->>Memory: MemoryInit()
KernelMain->>VMem: VMemInit()
KernelMain->>VMem: VMemGetPML4PhysAddr()
KernelMain->>KernelMain: BootstrapMapPage() (map kernel and low memory)
KernelMain->>AsmHelpers: EnablePagingAndJump(pml4_phys, KernelMainHigherHalf)
AsmHelpers-->>KernelMainHigherHalf: Jump to higher-half
KernelMainHigherHalf->>KernelMainHigherHalf: CoreInit()
KernelMainHigherHalf->>KernelMainHigherHalf: Start kernel scheduler loop
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
kernel/memory/VMem.h (1)
91-92: Consider clarifying the distinction between KERNEL_VIRTUAL_BASE and KERNEL_VIRTUAL_OFFSET.Both macros define kernel virtual addresses but serve different purposes.
KERNEL_VIRTUAL_BASE(0xFFFF800000000000ULL) appears to be for general kernel virtual space, whileKERNEL_VIRTUAL_OFFSET(0xFFFFFFFF80000000ULL) is specifically for the higher-half kernel mapping. Consider adding comments to clarify their distinct uses.+// Higher-half kernel mapping offset for physical-to-virtual conversion #define KERNEL_VIRTUAL_OFFSET 0xFFFFFFFF80000000ULL +// Page table address mask for extracting physical addresses #define PT_ADDR_MASK 0x000FFFFFFFFFF000ULL
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
drivers/Pic.h(1 hunks)kernel/core/Kernel.c(5 hunks)kernel/memory/AsmHelpers.asm(1 hunks)kernel/memory/AsmHelpers.h(1 hunks)kernel/memory/Memory.c(3 hunks)kernel/memory/VMem.c(3 hunks)kernel/memory/VMem.h(2 hunks)linker.ld(2 hunks)meson.build(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
kernel/memory/Memory.c (2)
kernel/core/Kernel.h (2)
PrintKernel(5-5)PrintKernelInt(7-7)kernel/core/Kernel.c (2)
PrintKernel(135-146)PrintKernelInt(194-218)
kernel/core/Kernel.c (8)
kernel/memory/Memory.c (2)
AllocPage(139-150)MemoryInit(40-136)kernel/memory/Memory.h (2)
AllocPage(10-10)MemoryInit(9-9)kernel/core/Panic.h (1)
Panic(7-7)kernel/memory/MemOps.h (1)
FastZeroPage(9-9)kernel/memory/MemOps.c (1)
FastZeroPage(63-77)kernel/memory/VMem.h (2)
VMemInit(94-94)VMemGetPML4PhysAddr(115-115)kernel/memory/VMem.c (2)
VMemInit(41-61)VMemGetPML4PhysAddr(401-403)kernel/memory/AsmHelpers.h (1)
EnablePagingAndJump(3-3)
🔇 Additional comments (19)
drivers/Pic.h (1)
4-4: Verify the impact of reducing timer frequency by 10x.The timer interrupt frequency has been reduced from 1000 Hz to 100 Hz, which significantly affects system timing granularity. This change could impact:
- Scheduler time slicing and process responsiveness
- Time-sensitive kernel operations
- System call timing expectations
Please ensure this frequency reduction aligns with the system's timing requirements and doesn't negatively impact user experience.
linker.ld (2)
10-10: LGTM: Kernel physical start boundary correctly defined.The placement of
_kernel_phys_startimmediately after setting the base address to 1MB is correct for marking the beginning of the kernel's physical memory region.
39-39: LGTM: Kernel physical end boundary correctly defined.The placement of
_kernel_phys_endafter the BSS section correctly marks the end of the kernel's physical memory region, capturing the entire kernel image.meson.build (1)
59-59: LGTM: Assembly helper correctly added to build configuration.The addition of
AsmHelpers.asmto the assembly sources list ensures the new paging helper function is properly compiled and linked into the kernel.kernel/memory/AsmHelpers.h (1)
1-3: LGTM: Header correctly declares the assembly helper function.The function declaration and includes are correct. The header properly defines the interface for the paging enablement helper.
kernel/memory/VMem.h (2)
99-99: Function prototype looks correct for kernel memory mapping.The
VMemMapKernelfunction signature properly takes physical start and end addresses, which aligns with the refactoring goal of mapping kernel memory into the higher-half virtual address space.
115-115: Function prototype correctly supports the new paging initialization flow.The
VMemGetPML4PhysAddrfunction provides the physical address of the PML4 table, which is essential for the assembly helperEnablePagingAndJumpto enable paging.kernel/memory/Memory.c (4)
10-11: Linker symbol declarations are correctly defined.The extern declarations for
_kernel_phys_startand_kernel_phys_endproperly reference the linker script symbols asuint8_t[]arrays, which is the standard approach for linker-provided boundaries.
105-108: Improved memory reservation with proper documentation.The explicit reservation of the first 1MB with informational logging is a good practice for debugging and system transparency.
110-125: Excellent improvement to kernel memory reservation precision.Using linker-provided symbols
_kernel_phys_startand_kernel_phys_endinstead of estimating kernel size is much more accurate and robust. The informational logging helps with debugging.
127-133: Good practice to reserve multiboot info memory.Explicitly reserving the multiboot information structure prevents it from being overwritten during normal memory allocation operations.
kernel/memory/VMem.c (3)
339-363: Well-implemented kernel memory mapping function.The
VMemMapKernelfunction correctly:
- Aligns addresses to page boundaries
- Maps physical kernel memory to higher-half virtual addresses using
KERNEL_VIRTUAL_OFFSET- Uses appropriate page flags (
PAGE_WRITABLE)- Provides informational logging
- Handles mapping failures with a panic
The implementation integrates well with the two-stage initialization flow.
398-398: Missing closing brace fix is correct.The addition of the closing brace for the
VMemGetStatsfunction resolves a syntax error.
401-403: Simple and correct PML4 address retrieval.The
VMemGetPML4PhysAddrfunction properly returns the physical address of the kernel's PML4 table, which is essential for the assembly helperEnablePagingAndJump.kernel/core/Kernel.c (5)
16-21: Proper includes and declarations for the refactoring.The addition of
AsmHelpers.h,MemOps.h, and the linker symbol declarations correctly support the new two-stage initialization flow.
312-357: Well-implemented bootstrap page table setup function.The
BootstrapMapPagefunction correctly implements the x86-64 page table hierarchy:
- Properly calculates indices for each level (PML4, PDPT, PD, PT)
- Allocates and zeros new page tables as needed
- Uses appropriate flags (
PAGE_PRESENT | PAGE_WRITABLE)- Handles memory allocation failures with panic
The direct pointer dereferencing is acceptable during bootstrap since identity mapping is available.
359-385: Good separation of concerns with CoreInit function.Moving the core subsystem initialization (GDT, IDT, syscalls, PIC, process management) into a separate function improves code organization and supports the two-stage initialization flow.
402-425: Excellent implementation of the bootstrap stage.The bootstrap initialization correctly:
- Initializes memory management first
- Sets up virtual memory and gets PML4 address
- Maps kernel memory to higher-half virtual addresses
- Identity maps low memory for VGA and other hardware
- Enables paging and jumps to higher-half kernel
The sequence is logical and necessary for proper virtual memory setup.
427-450: Proper higher-half initialization sequence.The
KernelMainHigherHalffunction correctly:
- Calls
CoreInit()to initialize core subsystems- Creates the security manager process
- Enables interrupts
- Enters the main scheduling loop
The two-stage approach ensures that core subsystems are initialized after virtual memory is fully operational.
| global EnablePagingAndJump | ||
|
|
||
| EnablePagingAndJump: | ||
| mov rdi, cr3 |
There was a problem hiding this comment.
Critical bug: Incorrect CR3 register operation.
The assembly code is moving the current CR3 value to RDI instead of setting CR3 with the provided PML4 physical address. This completely breaks the paging enablement functionality.
Apply this fix:
- mov rdi, cr3
+ mov cr3, rdiThe function should set CR3 to the first parameter (pml4_phys_addr in RDI), not read from CR3.
🤖 Prompt for AI Agents
In kernel/memory/AsmHelpers.asm at line 5, the code incorrectly moves the CR3
register value into RDI instead of setting CR3. To fix this, replace the
instruction so that CR3 is set to the value in RDI (the first parameter
pml4_phys_addr), ensuring the paging enablement works correctly.
1 participant
Summary by CodeRabbit
New Features
Refactor
Chores
Style