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

Example with x86_64 crate? #78

Closed
Qubasa opened this issue Nov 7, 2020 · 8 comments · Fixed by rust-osdev/x86_64#204
Closed

Example with x86_64 crate? #78

Qubasa opened this issue Nov 7, 2020 · 8 comments · Fixed by rust-osdev/x86_64#204

Comments

@Qubasa
Copy link

Qubasa commented Nov 7, 2020

Hello!

I tried to use this library but miserably failed to do so while battling with the rust compiler.
Could you add a little example while using the x86_64 crates FrameAllocator & OffsetPageTable?
I would deeply appreciate it :)
I tried using a struct like this:

#[derive(Clone)]
pub struct MyAcpiHandler<'a> {
    mapper: Rc<RefCell<&'a mut OffsetPageTable<'a>>>,
    frame_allocator: Rc<RefCell<&'a mut dyn FrameAllocator<Size4KiB>>>,
}

but this failed with the error:

.map_to(page, frame, flags, self.frame_allocator.get_mut())
    |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `FrameAllocator<Size4KiB>` is not implemented for `&mut dyn FrameAllocator<Size4KiB>`
@Restioson
Copy link
Member

Restioson commented Nov 7, 2020

This seems like an x86_64 issue since acpi has no FrameAllocator trait.

In any case, which map_to fn are you using? This one needs A: FrameAllocator with an implicit Sized bound which would prevent one from passing A: dyn FrameAllocator<Size4KiB>. Maybe this could be relaxed though.

phil-opp added a commit to rust-osdev/x86_64 that referenced this issue Nov 7, 2020
This makes it possible to use the method with trait objects.

Resolves rust-osdev/acpi#78
@phil-opp
Copy link
Member

phil-opp commented Nov 7, 2020

Maybe this could be relaxed though.

Yeah, I think we can relax this. I opened rust-osdev/x86_64#204 for this.

@IsaacWoods
Copy link
Member

Hi, always nice to hear of people using the library! I'm afraid I don't have much experience with the x86_64 crate as my kernel doesn't use it. Those types are pretty complicated though, it might be worth making MyAcpiHandler generic over the type of FrameAllocator instead of using a trait object (you'll probably only be monomorphising one copy of the code anyway).

EDIT: ah looks like Phil's on it :) hope you get this working @luis-hebendanz!

@Qubasa
Copy link
Author

Qubasa commented Nov 7, 2020

Damn you guys are fast! Thanks for the help :D

@phil-opp
Copy link
Member

phil-opp commented Nov 7, 2020

Try using borrow_mut instead of get_mut. The latter requires a &mut reference to the RefCell which is not possible for Rc<RefCell<_>>.

@Qubasa
Copy link
Author

Qubasa commented Nov 7, 2020

Wow it worked. I have no idea why but it worked! Now I have to sit down and find out why and how :)

@phil-opp
Copy link
Member

phil-opp commented Nov 7, 2020

Let me try to explain this in a bit more detail:

The last error had nothing to do with the x86_64 crate (it was just masked by the earlier error). The error also occurs when you try self.frame_allocator.get_mut() independently.

When you use the borrow_mut() method you get a mutable reference to the type wrapped in RefCell, which is &mut dyn FrameAllocator<Size4KiB> in your case. So the resulting type is &mut &mut dyn FrameAllocator<Size4KiB>. To remove the double reference, you need to dereference once.

All generic parameters in Rust have an implicit Sized bound by default. This means that the size of the parameter needs to be known at compile time. Since the map_to method takes only references to the FrameAllocator as argument, this implicit bound isn't needed, so we can opt-out using the ?Sized bound (that's what the linked PR does). This way, the dynamically-sized dyn FrameAllocator type is now usable with the method.

I hope this makes things clearer!

@Qubasa
Copy link
Author

Qubasa commented Nov 7, 2020

You are awesome!

phil-opp added a commit to rust-osdev/x86_64 that referenced this issue Dec 28, 2020
This makes it possible to use the method with trait objects.

Resolves rust-osdev/acpi#78
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

Successfully merging a pull request may close this issue.

4 participants