RWTH-OS / eduOS-rs

A teaching operating system written in Rust
https://rwth-os.github.io/eduOS-rs/
Apache License 2.0
434 stars 27 forks source link

Unexpected behavior unmapping pages #35

Open TrebuchKill opened 1 year ago

TrebuchKill commented 1 year ago

Using the modules crate::arch::x86_64::mm::{ paging, virtualmem } results in unexpected behavior, assuming I understand the intentions of the code correctly.

What I expect:

After a call to paging::unmap, or at the very least virtualmem::deallocate, I'll expect paging::get_page_table_entry::<BasePageSize> to return None.

Actual behavior:

After each of the above calls, paging::get_page_table_entry::<BasePageSize> returns Some(0) for the first page. If page count > 1, following pages will be mapped to a multiple of BasePageSize::SIZE.

To reproduce the issue and making sure, it is not a misunderstanding on my part, here is an example function I put into main.rs and call from main.rs:main (after initialization of course), producing the output shown below:

Code:

// Helper function used in test(usize)
fn to_string(it: Option<crate::arch::x86_64::mm::paging::PageTableEntry>) -> String
{
    match it
    {
        None => "None".to_owned(),
        Some(it) => alloc::format!("Some(0x{:016x})", it.address())
    }
}

fn test(count: usize)
{
    use crate::arch::mm::{
        paging::{
            self,
            BasePageSize,
            PageSize,
            PageTableEntryFlags
        },
        virtualmem,
        physicalmem
    };

    let allocation_size = BasePageSize::SIZE * count;

    // Allocate Page & Frame
    let virt = virtualmem::allocate_aligned(allocation_size, BasePageSize::SIZE);
    let phys = physicalmem::allocate_aligned(allocation_size, BasePageSize::SIZE);

    println!("Trying:   0x{:016x} => 0x{:016x}", virt, phys);
    println!("Alloc:    0x{:016x} => {}", virt, to_string(paging::get_page_table_entry::<BasePageSize>(virt)));
    println!("          0x{:016x} => {}", virt + BasePageSize::SIZE, to_string(paging::get_page_table_entry::<BasePageSize>(virt + BasePageSize::SIZE)));

    // Map Page to Frame
    paging::map::<BasePageSize>(virt, phys, count, PageTableEntryFlags::empty());

    println!("Mapped:   0x{:016x} => {}", virt, to_string(paging::get_page_table_entry::<BasePageSize>(virt)));
    println!("          0x{:016x} => {}", virt + BasePageSize::SIZE, to_string(paging::get_page_table_entry::<BasePageSize>(virt + BasePageSize::SIZE)));

    // Unmap Page
    paging::unmap::<BasePageSize>(virt, count);

    println!("Unmapped: 0x{:016x} => {}", virt, to_string(paging::get_page_table_entry::<BasePageSize>(virt)));
    println!("          0x{:016x} => {}", virt + BasePageSize::SIZE, to_string(paging::get_page_table_entry::<BasePageSize>(virt + BasePageSize::SIZE)));

    // Deallocate Page & Frame
    virtualmem::deallocate(virt, allocation_size);
    physicalmem::deallocate(phys, allocation_size);

    println!("Dealloc:  0x{:016x} => {}", virt, to_string(paging::get_page_table_entry::<BasePageSize>(virt)));
    println!("          0x{:016x} => {}", virt + BasePageSize::SIZE, to_string(paging::get_page_table_entry::<BasePageSize>(virt + BasePageSize::SIZE)));
}

Output:

Building bootloader
    Finished release [optimized + debuginfo] target(s) in 0.03s
Running: `qemu-system-x86_64 -display none -smp 1 -m 128M -serial stdio -cpu qemu64,apic,fsgsbase,rdtscp,xsave,xsaveopt,fxsr -device isa-debug-exit,iobase=0xf4,iosize=0x04 -drive format=raw,file=target/x86_64-eduos/debug/bootimage-eduos-rs.bin`
[INFO] Memory size 127 MByte
init: paging             // Note: added println! into the respective init functions just to make sure
init: physicalmem
init: virtualmem
[INFO] Found mountable file at 0x251fb0 (len 0x3b90)
Hello from eduOS-rs!
[INFO] Print file system:
[INFO] /
[INFO]   bin (Directory)
[INFO]     demo (File)
[INFO]   dev (Directory)
[INFO] Creating task 1
[INFO] Creating task 2
[INFO] Creating task 3
Trying:   0x0000000080000000 => 0x0000000000af0000       // test(1)
Alloc:    0x0000000080000000 => None
          0x0000000080001000 => None
Mapped:   0x0000000080000000 => Some(0x0000000000af0000)
          0x0000000080001000 => None
Unmapped: 0x0000000080000000 => Some(0x0000000000000000) // <- Mapped to 0
          0x0000000080001000 => None
Dealloc:  0x0000000080000000 => Some(0x0000000000000000)
          0x0000000080001000 => None
Trying:   0x0000000080000000 => 0x0000000000af3000       // test(2)
Alloc:    0x0000000080000000 => Some(0x0000000000000000) // <- Some(0) from previous invocation of test
          0x0000000080001000 => None
Mapped:   0x0000000080000000 => Some(0x0000000000af3000)
          0x0000000080001000 => Some(0x0000000000af4000)
Unmapped: 0x0000000080000000 => Some(0x0000000000000000) // <- Mapped to 0
          0x0000000080001000 => Some(0x0000000000001000) // <- Mapped to 4096
Dealloc:  0x0000000080000000 => Some(0x0000000000000000)
          0x0000000080001000 => Some(0x0000000000001000)
hello from task 1
[INFO] finish task with id 1
hello from task 2
[INFO] finish task with id 2
[INFO] Hello from loader
Hello World from Linux!
[INFO] finish task with id 3
Shutdown system!
stlankes commented 1 year ago

Good point! This definitely wrong. I will fix it.