-
Notifications
You must be signed in to change notification settings - Fork 9
Implement Linux bootable image support #20
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
qwandor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I like the idea of supporting the Linux boot protocol (so far I've only used this for binaries loaded at a fixed address, like a bootloader), but I'm not sure that copying to a fixed address will always work. You'll also need to sign the CLA before I can accept anything.
cb2893c to
36296ef
Compare
|
Was busy with work but had time to look into this just now. I got the relocate solution working, with the following caveats:
Also fixed the CLA check, and guarded the relocate logic behind a EDIT: I made an example by copying the |
3cae637 to
60e7a0f
Compare
|
One thing missing is that the |
|
Rebased against |
|
I am still not sure if this is the correct way to go. It definitely works, but forcing the emission of relocation entries seems hacky. Maybe we should just emit only the boot header instead, and force the user of the library to map their actual physical load address to their expected load virtual address. I think that's what the linux kernel is doing (?). That way, no relocations are needed. I'm not super familiar with MMU programming and page tables though. @qwandor WDYT? |
|
Yes, I got confirmation from a kernel developer. It maps its code to a virtual address with an offset from the physical address. Then, for MMIO/devices it maps them to dynamic virtual addresses such as ones created with vmalloc. So with that in mind, while self-relocation seems like a good solution to avoid all that, it seems it should be separate from the linux boot header stuff. |
This is an interesting idea. So I guess you'd need to reserve space for an initial pagetable, but fill in the entries before enabling it based on the load address? |
src/relocate.rs
Outdated
| r_info, | ||
| r_addend, | ||
| } = unsafe { *rela }; | ||
| if elf64_r_type!(r_info) == R_AARCH64_RELATIVE { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you know it is safe to ignore relocation entries of other types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alternative is RELR which are opt-in as a linker option, since they are a very recent feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mean RELA vs. RELR, I mean R_AARCH64_RELATIVE vs. other entries. I'm not very familiar with the details of how relocation works, but as far as I can see there are a whole lot of different types e.g. R_AARCH64_ABS64, R_AARCH64_MOVW_UABS_G0 and so on, how do we know that we can ignore all the rest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these are not emitted for pie executables.
| image : ORIGIN = 0x0, LENGTH = 2M | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document that the relocate feature is incompatible with the initial-pagetable feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not necessarily, you can use initial-pagetable to map everything to normal memory and after relocating, you need to set up new page tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, let's document that then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that actually valid? Talking to some folks at Arm, it sounds like this might not work, because if a device is incorrectly mapped as normal memory then the CPU might speculatively access it, which can cause the device to perform arbitrary unwanted operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, will write it as incompatible then. Thanks!
930e8ff to
991a2f9
Compare
|
Cc @ardbiesheuvel this is the PR I mentioned to you. |
Follow the https://docs.kernel.org/arch/arm64/booting.html protocol to allow building a bootable linux-like image. To allow for any load address, perform relocations before jumping to the rust entrypoint. Guard relocation logic behind "relocation" Cargo feature. Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
991a2f9 to
3bbdec3
Compare
| "ldr x10, [x9, #16]", | ||
| // let r_offset = unsafe { *rela }.r_offset; | ||
| "ldr x11, [x9]", | ||
| // new_ptr += offset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can simplify this a bit, and remove the need for the jump at the start of the loop, by doing something like
ldp x10, x11, [x9], #24
ldr x12, [x9, #-8]Note that R_AARCH64_RELATIVE is the only RELA type that can reasonably expected to occur here, so performing the load of the offset and addend fields unconditionally will not make any difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we know that no other RELA types can occur?
|
One general question about this code: who is supposed to perform the cache invalidation? No I-cache maintenance should be required, as R_AARCH64_RELATIVE relocations can only refer to data, not code. However, under KVM, where the VMM is in charge of ensuring that the entire bootable image is clean to the point of coherency (PoC), some extra work is needed when the relocation code itself happens to execute before the MMU and caches are enabled. Otherwise, the updated memory contents might be shadowed by stale, clean cachelines, resulting in odd behavior when the code enables the MMU. |
|
@ardbiesheuvel What is your suggestion? if we want to ensure correct use of this API, would it make sense to do the cache flush ourselves after performing the relocation? I.e. |
|
Flushing the I-cache should not be needed, only the D-cache needs maintenance. The issue here is that it depends on
Assuming that this code only executes with the MMU off, it should be sufficient to perform a D-cache invalidate for either the whole region, or simply for every store - the latter is much simpler but performs redundant work if there are many adjacent locations being relocated. If the code may execute with the MMU on as well, the invalidate should be gated on this condition, and a clean performed instead (unless the code never executes with the MMU off, even on secondaries). |
| rustflags = [ | ||
| "-C", "relocation-model=pie", | ||
| "-C", "link-args=-z notext", | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Higher up in this README we recommend passing the linker script in build.rs via cargo:rustc-link-arg=, so let's suggest the same thing here for these flags if that's possible.
|
@epilys Did you have a chance to look at the outstanding comments on this? I think the main things remaining are flushing D-cache, Ard's suggested simplification, and updating the README. |
|
It's on my TODO list. IIUC, I need to add a conditional check to see if MMU is disabled for current EL, and invalidate d-cache if it's off, or clean&invalidate if it's on, after all relocations are performed. |
Follow the https://docs.kernel.org/arch/arm64/booting.html protocol to allow building a bootable linux-like image.
This'd be ideal for a compile once, run "anywhere" use-case.
I've been able to create linux bootable images with this diff.
file(1)recognizes the image asLinux kernel ARM64 boot executable Image, little-endian, 4K pagesand QEMU can boot it with direct kernel boot. Also with Xen under QEMU by loading the image as the dom0 with theguest-loaderdevice.Finally, I added the UEFI
MZmagic as the first instruction but I haven't had time to look into writing a proper stub.TODOs: