Skip to content
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

Moving page table mapping into page fault handler #593

Closed
redamith2 opened this issue Apr 28, 2019 · 11 comments
Closed

Moving page table mapping into page fault handler #593

redamith2 opened this issue Apr 28, 2019 · 11 comments

Comments

@redamith2
Copy link

I am trying to move the mapping into the page fault handler, but i would like to create a pub static variables for both mapper and frame_allocator where I can pass it along to the mapping in the page fault handler

# in lib.rs
use memory::BootInfoFrameAllocator;
pub static mut FRAME_ALLOCATOR : Option<BootInfoFrameAllocator<Iterator<Item = PhysFrame>>> = None;
pub static mut MAPPER: Option<&'static mut RecursivePageTable> = None;

On doing so I get the error

doesn't have a size known at compile-time for Option<BootInfoFrameAllocator<Iterator<Item = PhysFrame>>>

@bjorn3
Copy link
Contributor

bjorn3 commented Apr 29, 2019

dyn Iterator<Item = PhysFrame> doesnt have a statically known size, as any type implementing Iterator<Item = PhysFrame> could be behind it.

@phil-opp
Copy link
Owner

That's a tricky issue that I didn't consider. My intuition would be to use BootFrameAllocator<impl Iterator<…>> since otherwise it is treated like a dynamic trait object dyn Iterator as @bjorn3 said. However, it turns out this doesn't work either because the Rust compiler can't (yet) infer the correct type when a static is initialized with None.

I opened rust-lang/rust#60367 in the Rust repository for this. Let's see what they say.

@andre-richter
Copy link
Contributor

In the meantime, I think you would need to pack the Iterator as a trait object, e.g. putting it in a Box.

pub struct BootInfoFrameAllocator<I> where I: Iterator<Item = PhysFrame> {
    frames: Box<I>,
}

@phil-opp
Copy link
Owner

@andre-richter That should work. The problem is that we don't have heap allocation on the blog yet.

@andre-richter
Copy link
Contributor

Ah, my bad 😅

@phil-opp
Copy link
Owner

Ah, my bad sweat_smile

Not at all. It is the best solution to this problem currently, independent of the state of the blog.

phil-opp added a commit that referenced this issue Apr 30, 2019
This makes it very hard to put the BootInfoFrameAllocator in a static, e.g. for accessing it from a page fault handler. See #593 for more information.
phil-opp added a commit that referenced this issue Apr 30, 2019
This makes it very hard to put the BootInfoFrameAllocator in a static, e.g. for accessing it from a page fault handler. See #593 for more information.
@phil-opp
Copy link
Owner

I opened #595 to change the implementation of BootInfoFrameAllocator. The new implementation is non generic, so it should be usable in a static without problems.

@phil-opp
Copy link
Owner

Investing further, it seems like we could solve the original problem with a named existential type in the future. However, the implementation of this feature is still in its early stages (some ICEs andlifetime problems, only placeholder syntax), so it's not a good fit for the blog right now.

@phil-opp
Copy link
Owner

@redamith2 Could you try the updated instructions at https://os.phil-opp.com/paging-implementation/#allocating-frames and see if it works?

@redamith2
Copy link
Author

redamith2 commented Apr 30, 2019

Thank you I after making those changes it worked.

@phil-opp
Copy link
Owner

Perfect, thanks for trying!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants