Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions issues/kernel/kernel_stack_addr-names-an-offset-not-an-address.md
Original file line number Diff line number Diff line change
@@ -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.

This file was deleted.

37 changes: 33 additions & 4 deletions kernel/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> ! {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
Loading