Skip to content

Add options for phys mem offset, kernel stack addr and size - #44

Closed
64 wants to merge 1 commit into
rust-osdev:masterfrom
64:extra-config
Closed

Add options for phys mem offset, kernel stack addr and size#44
64 wants to merge 1 commit into
rust-osdev:masterfrom
64:extra-config

Conversation

@64

@64 64 commented Aug 2, 2019

Copy link
Copy Markdown
Contributor

Allows you to specify these three options through Cargo.toml:

[package.metadata.bootimage]
physical-memory-offset = "0xffff800000000000"
kernel-stack-address = "0xffffff8000000000"
kernel-stack-size = 128

@phil-opp

phil-opp commented Aug 2, 2019

Copy link
Copy Markdown
Member

Thanks a lot for the pull request!

I know that I proposed flags like this in the gitter chat, but I'm no longer sure about this approach because it effectively locks bootimage to the specific bootloader crate. Perhaps it's better to let the bootloader crate parse their own options.

This could be implemented by passing a KERNEL_MANIFEST environment variable to the bootloader that points to the Cargo.toml of the kernel. The bootloader build script can then look for a package.metadata.bootloader configuration table in it. This would have the advantages that new options can be added to the bootloader without updating bootimage and that different bootloader crates can have different options. However it would be more work and slightly increase the compile times of the bootloader crate. What do you think?

@64

64 commented Aug 2, 2019

Copy link
Copy Markdown
Contributor Author

I know that I proposed flags like this in the gitter chat, but I'm no longer sure about this approach because it effectively locks bootimage to the specific bootloader crate.

Hm, I don’t exactly see how. If the bootloader doesn’t support these things then it will silently ignore the env vars.

This could be implemented by passing a KERNEL_MANIFEST environment variable to the bootloader that points to the Cargo.toml of the kernel.

Would this be done by bootimage?

Otherwise, it sounds like a good idea.

@phil-opp

phil-opp commented Aug 2, 2019

Copy link
Copy Markdown
Member

Hm, I don’t exactly see how. If the bootloader doesn’t support these things then it will silently ignore the env vars.

True, but an alternative bootloader implementation could not implement additional options without modifying bootimage, so that the two crates are still tied together. Also, silently ignoring the env variables means silently ignoring a configuration option, which doesn't seem like a good idea since it can lead to surprising behavior (e.g. the kernel stack remains at same place even though the user overrides the location). By letting the bootloader parse its own options, it can throw a proper error if something isn't supported.

This could be implemented by passing a KERNEL_MANIFEST environment variable to the bootloader that points to the Cargo.toml of the kernel.

Would this be done by bootimage?

Yes. Instead of passing each configuration option separately, bootimage would just tell the bootloader where the configuration can be found.

@64

64 commented Aug 2, 2019

Copy link
Copy Markdown
Contributor Author

Sounds good! Will cook up a PR at a later date.

@64 64 closed this Aug 2, 2019
@phil-opp

phil-opp commented Aug 2, 2019

Copy link
Copy Markdown
Member

Thanks a lot! Sorry for changing my opinion on this!

@64

64 commented Aug 2, 2019

Copy link
Copy Markdown
Contributor Author

No worries at all. I think this will need a bootloader breaking change, though, if we’re going to remove the env vars.

@phil-opp

phil-opp commented Aug 2, 2019

Copy link
Copy Markdown
Member

That's fine with me.

@phil-opp

phil-opp commented Aug 2, 2019

Copy link
Copy Markdown
Member

We could of course also keep the environment variables if you prefer.

@64
64 deleted the extra-config branch August 5, 2019 20:53
bors Bot added a commit that referenced this pull request Aug 5, 2019
45: Pass location of kernel's Cargo.toml to bootloader r=phil-opp a=64

As discussed in #44.

Co-authored-by: Matt Taylor <mstaveleytaylor@gmail.com>
MiscStuff839 pushed a commit to MiscStuff839/bootimage-wasm that referenced this pull request Nov 16, 2025
45: Pass location of kernel's Cargo.toml to bootloader r=phil-opp a=64

As discussed in rust-osdev#44.

Co-authored-by: Matt Taylor <mstaveleytaylor@gmail.com>
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.

2 participants