Skip to content

Last page of stack memory is not used #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
phil-opp opened this issue Dec 2, 2022 · 2 comments · Fixed by #335
Closed

Last page of stack memory is not used #294

phil-opp opened this issue Dec 2, 2022 · 2 comments · Fixed by #335
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@phil-opp
Copy link
Member

phil-opp commented Dec 2, 2022

We set the stack_top to the start address of the stack_end page here:

stack_top: mappings.stack_end.start_address(),

However, it looks like stack_end is an inclusive bound:

let stack_end = {
let end_addr = stack_start_addr + config.kernel_stack_size;
Page::containing_address(end_addr - 1u64)
};

So I think the stack pointer should be initialized with (stack_end + 1).start_address() instead.

Reported by klim on the Discord of Theseus OS.

@phil-opp phil-opp added bug Something isn't working good first issue Good for newcomers labels Dec 2, 2022
tsoutsman added a commit to theseus-os/bootloader that referenced this issue Dec 4, 2022
Summary of changes:
- Add size to boot info
- Add modules to boot info
- Add kernel elf sections to boot info
- Remove BIOS building from build script
- Make kernel stack page aligned
- Modify stack to use last page of memory (fixes rust-osdev#294)
- Add stack guard page
- Remove push operation from context switch
- Modify frame allocator to only allocate frames above 0x10000

Signed-off-by: Klim Tsoutsman <[email protected]>
tsoutsman added a commit to theseus-os/bootloader that referenced this issue Dec 4, 2022
Summary of changes:
- Add size to boot info
- Add modules to boot info
- Add kernel elf sections to boot info
- Remove BIOS building from build script
- Make kernel stack page aligned
- Modify stack to use last page of memory (fixes rust-osdev#294)
- Add stack guard page
- Remove push operation from context switch
- Modify frame allocator to only allocate frames above 0x10000

Signed-off-by: Klim Tsoutsman <[email protected]>
tsoutsman added a commit to theseus-os/bootloader that referenced this issue Dec 4, 2022
Summary of changes:
- Add size to boot info
- Add modules to boot info
- Add kernel elf sections to boot info
- Remove BIOS building from build script
- Make kernel stack page aligned
- Modify stack to use last page of memory (fixes rust-osdev#294)
- Add stack guard page
- Remove push operation from context switch
- Modify frame allocator to only allocate frames above 0x10000
- Add support for arbitrary files in UEFI image

Signed-off-by: Klim Tsoutsman <[email protected]>
tsoutsman added a commit to theseus-os/bootloader that referenced this issue Dec 4, 2022
Summary of changes:
- Add size to boot info
- Add modules to boot info
- Add kernel elf sections to boot info
- Remove BIOS building from build script
- Make kernel stack page aligned
- Modify stack to use last page of memory (fixes rust-osdev#294)
- Add stack guard page
- Remove push operation from context switch
- Modify frame allocator to only allocate frames above 0x10000
- Add support for arbitrary files in UEFI image

Signed-off-by: Klim Tsoutsman <[email protected]>
@inhibitor1217
Copy link

It would be a lot of help if this issue is fixed soon.

I'm currently working with a setup which assigns a single page (4 KiB) for the stack of each thread, including the kernel thread. Currently, last page (which is, the only page) of the stack is not used, causing the page fault to occur immediately after we jump to the kernel.

inhibitor1217 added a commit to inhibitor1217/bootloader that referenced this issue Jan 27, 2023
…om of kernel stack

mappings.stack_end is set inclusive

fix rust-osdev#294
@phil-opp
Copy link
Member Author

@inhibitor1217 Thanks for the reminder! I'll prepare a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants