Skip to content

Version 0.2.0 - #20

Merged
phil-opp merged 5 commits into
masterfrom
version-0.2.0
Jul 20, 2018
Merged

Version 0.2.0#20
phil-opp merged 5 commits into
masterfrom
version-0.2.0

Conversation

@phil-opp

Copy link
Copy Markdown
Member

Given that we were in alpha state for a long time and there were no real bugs reported, I think it's time to finally release version 0.2.0. Another big reason for doing it is that the current versions of bootimage only work with the latest alpha versions, so everybody was forced to use the alpha versions anyway.

There are a few improvements in this PR:

  • Add a `lib.rs` that re-exports the `os_bootinfo` crate as `bootinfo`. This works nicely with the [new version of `bootimage`](https://github.com/Rewrite bootimage to make bootloader a "normal" dependency bootimage#16). It means that you no longer need to add a `os_bootinfo` dependency and instead add the `bootloader` as dependency directly. This solves the potential version mismatch problem (i.e. kernel and bootloader use different `os_bootinfo` versions).
  • Add an `entry_point` macro which allows to specify the entry point like this:
    #[macro_use]
    extern crate bootloader;
    
    #[cfg(not(test))]
    entry_point!(kernel_main);
    
    fn kernel_main(boot_info: &'static bootloader::bootinfo::BootInfo) -> ! {}

    The advantage of using this macro is that the function signature is type-checked, i.e. it is an error if the argument type is wrong. You also no longer need no_mangle, extern "C", etc since the macro handles this for us.

  • - Disable interrupts instead of masking them. This solves Don't mask IRQs #14.

This PR is already pulished as 0.2.0-beta. Please test it and give feedback!

@IsaacWoods

Copy link
Copy Markdown
Member

Why not completely move os_bootinfo into bootloader, now that it's a dependency? We're already causing breakage, and having the one crate simplifies the dependency graph a little

@phil-opp

Copy link
Copy Markdown
Member Author

Having it as a separate crate allows multiple bootloader implementations to reuse the same bootinfo struct, so we might want to keep it separate. However, I completely agree that we should hide the fact that this is a common crate to be less constrained by backwards compatibility in the future.

I think the best way is to create a bootinfo module in the bootloader and reexport all os_bootinfo types from it. This way we can move the implementation into the bootloader without a breaking change later (if we want to). The user never sees the os_bootinfo crate.

This allows us to merge the os_bootinfo crate into the bootloader without breakage (in case we want to do that someday). It also hides the os_bootinfo crate as an implementation detail.
@phil-opp

Copy link
Copy Markdown
Member Author

I pushed a new version that does not expose os_bootinfo, but instead reexports its types as part of a public bootinfo module. This should allow us to move the contents of os_bootinfo into bootloader without causing breakage, in case we want to do that in the future. What do you think @IsaacWoods?

Oh and one thing I wanted to note:

We're already causing breakage

We're causing breakage by the bootimage update, but this bootloader PR should be backwards compatible. (The only change that could be breaking is that interrupts are now disabled instead of masked, but I don't think that anyone relied on this.)

@IsaacWoods

Copy link
Copy Markdown
Member

Ah that makes sense - we haven't discussed how we're going to approach multiple bootloaders in the future (we'll need 2-3 for x86 alone eventually - the current BIOS HDD image, EFI and maybe El-Torito). I'd like to see if we can actually support them all within the one crate, and just use cfg to mix the code we need into each version, which would be configurable via bootimage, so the dependency would always be this crate. Until that comes to fruition however, keeping os_bootinfo around doesn't hurt, and I support not re-exporting it :)

Overall tho, this is great work and much easier to understand what's going on, thanks Phil!

@lachlansneff

lachlansneff commented Jul 19, 2018

Copy link
Copy Markdown
Contributor

To add onto what @IsaacWoods said, we'll also need bootloaders for risc-v and the various flavors of arm eventually. It would be nice to contain them all in one crate and use cfg features or even just the compiler target to enable to correct one.

In fact, we could have a number of sub-crates that each export an inlined start function. And then, the bootloader would include all of them and gate based on cfg features and compile targets.

@phil-opp

Copy link
Copy Markdown
Member Author

@IsaacWoods

I'd like to see if we can actually support them all within the one crate, and just use cfg to mix the code we need into each version, which would be configurable via bootimage, so the dependency would always be this crate.

Now that the bootloader is a normal dependency, we could even do this through normal cargo features, e.g. an bios feature, an efi feature, etc.

Overall tho, this is great work and much easier to understand what's going on, thanks Phil!

Glad to hear that, thank you!

@phil-opp

Copy link
Copy Markdown
Member Author

@lachlansneff Sounds good!

@phil-opp
phil-opp merged commit cf33ef8 into master Jul 20, 2018
@phil-opp
phil-opp deleted the version-0.2.0 branch July 20, 2018 08:17
@phil-opp

Copy link
Copy Markdown
Member Author

Published as version 0.2.0

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 this pull request may close these issues.

3 participants