diff --git a/issues/kernel/kernel_stack_addr-names-an-offset-not-an-address.md b/issues/kernel/kernel_stack_addr-names-an-offset-not-an-address.md new file mode 100644 index 00000000000..8bf1eca1ede --- /dev/null +++ b/issues/kernel/kernel_stack_addr-names-an-offset-not-an-address.md @@ -0,0 +1,19 @@ +--- +status: open +kind: defect +opened: 2026-09-28 +--- + +# `kernel_stack_addr` names an offset, not an address + +`toyos_abi::boot::KernelArgs::kernel_stack_addr` holds an offset into the +kernel's own image — `StackedImage::place`'s `stack` — never a physical +address. Both architectures' entry code already treat it as an offset +(`kernel/src/arch/aarch64/boot.rs` reads it into a local named +`stack_offset`); `kernel/src/main.rs` reads it under its `KernelArgs` name and +must not repeat the confusion the name invites. + +Owner: the author of PR #583 (`wt/toyos-loader1`), which is already changing +`KernelArgs`'s layout. +Exit: the field renamed to `kernel_stack_offset` (or equivalent), landed as +part of that change so `KernelArgs` moves once. diff --git a/issues/kernel/the-kernel-reserves-its-stack-offset-as-a-physical-region.md b/issues/kernel/the-kernel-reserves-its-stack-offset-as-a-physical-region.md deleted file mode 100644 index 64db710285a..00000000000 --- a/issues/kernel/the-kernel-reserves-its-stack-offset-as-a-physical-region.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-27 ---- - -# The kernel reserves its stack's offset as a physical region - -`KernelArgs::kernel_stack_addr` is an offset into the kernel's own allocation: -the loader sets it to `StackedImage::place`'s `stack` (`bootloader/src/main.rs`), -and both entries add it to `kernel_memory_addr` before loading the stack -pointer (`kernel/src/arch/x86_64/boot.rs`'s `_start`, -`kernel/src/arch/aarch64/boot.rs`). - -`kernel/src/main.rs`'s `reserved` list takes it as an address: -`mm::Region { start: kernel_stack_addr, end: kernel_stack_addr + -kernel_stack_size }`. On the rust-lld kernel that is physical -`0x411000..0xc11000`, low memory that is nobody's stack, withheld from the -allocator for the whole boot. The real stack is inside `kernel_memory_addr .. -+ kernel_memory_size`, which the first entry of the same list already reserves, -so nothing is handed out twice; the defect is 8 MiB of memory reserved for -nothing, at whatever physical range the image's size names. - -Exit: the region goes, or names `kernel_memory_addr + kernel_stack_addr`, and -a host-checkable decision or a boot assertion states which physical ranges the -reserved list covers. diff --git a/kernel/src/main.rs b/kernel/src/main.rs index ef64ca60a95..9adfc3da996 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -118,6 +118,7 @@ use arch::{cpu, percpu, smp}; pub(crate) use arch::hw; use drivers::{acpi, gop, nvme, pci, serial, virtio_console, virtio_gpu, virtio_sound, xhci}; use toyos_abi::boot::{KernelArgs, MemoryMapEntry}; +use toyos_rootimage::handoff::{held, Descriptor}; #[panic_handler] fn panic(info: &core::panic::PanicInfo) -> ! { @@ -304,7 +305,7 @@ pub(crate) unsafe extern "C" fn kernel_main(kernel_args: &KernelArgs) -> ! { // Split into six records: KernelArgs' derived Debug is the one message that exceeds the log's per-record bound. log!( - "boot: memory map {:#x}+{:#x}, kernel {:#x}+{:#x}, stack {:#x}+{:#x}", + "boot: memory map {:#x}+{:#x}, kernel {:#x}+{:#x}, stack image+{:#x}+{:#x}", kernel_args.memory_map_addr, kernel_args.memory_map_size, kernel_args.kernel_memory_addr, kernel_args.kernel_memory_size, kernel_args.kernel_stack_addr, kernel_args.kernel_stack_size @@ -348,11 +349,16 @@ pub(crate) unsafe extern "C" fn kernel_main(kernel_args: &KernelArgs) -> ! { ); let kernel_args = &kernel_args; - let reserved = [ + // `kernel_stack_addr` is an offset into the image, so the image's region is what keeps the stack. + assert!( + kernel_args.kernel_stack_addr.checked_add(kernel_args.kernel_stack_size) + .is_some_and(|end| end <= kernel_args.kernel_memory_size), + "boot: the loader put the stack at image+{:#x}+{:#x}, past the {:#x}-byte image", + kernel_args.kernel_stack_addr, kernel_args.kernel_stack_size, kernel_args.kernel_memory_size + ); + let loader = [ mm::Region { start: kernel_args.kernel_memory_addr, end: kernel_args.kernel_memory_addr + kernel_args.kernel_memory_size }, mm::Region { start: kernel_args.kernel_elf_addr, end: kernel_args.kernel_elf_addr + kernel_args.kernel_elf_size }, - mm::Region { start: kernel_args.kernel_stack_addr, end: kernel_args.kernel_stack_addr + kernel_args.kernel_stack_size }, - arch::boot::reserved(), // The loader's black-box page, which is ordinary `LoaderData` and so // memory the allocator would otherwise hand out. Empty on a boot whose // parameter line names none. @@ -361,6 +367,29 @@ pub(crate) unsafe extern "C" fn kernel_main(kernel_args: &KernelArgs) -> ! { // boot the loader handed none. root_image, ]; + // A region the loader did not allocate withholds memory nothing uses, so one the firmware map does not hold as `LoaderData` is refused. + // Block 1: the ELF region (`kernel_elf_addr`+`kernel_elf_size`) is not page-aligned. + for region in loader.iter().filter(|r| r.start < r.end) { + assert!( + held( + maps.iter().map(|e| Descriptor { ty: e.uefi_type, start: e.start, end: e.end }), + toyos_bootmap::EFI_LOADER_DATA, + region.start, + region.end - region.start, + 1, + ) + .is_some(), + "boot: reserving {:#x}..{:#x}, which no LoaderData descriptor in the firmware map holds", + region.start, region.end + ); + } + // The architecture's own page is not a loader allocation, so it is named + // here rather than folded into `loader` above. Destructuring `loader` by + // name, rather than indexing it, means a region added to `loader` fails + // to compile here instead of compiling and being silently dropped from + // what `mm::init` withholds. + let [image, elf, black_box, root] = loader; + let reserved = [image, elf, black_box, root, arch::boot::reserved()]; // The last point before the first hash container (`mm::init`'s address // space), and not earlier: seeding fails only by panicking, and a panic