-
Notifications
You must be signed in to change notification settings - Fork 235
Add DYN (Position-Independent Executable file) support #1171
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: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Ariel Xiong <[email protected]>
| .PHONY: sail | ||
| sail: build/sail.dtb $(elf) $(SAIL_SIM) | ||
| $(SAIL_SIM) --no-trace -p -l $(LIMIT_INSTRUCTIONS) --device-tree-blob build/sail.dtb $(elf) | ||
| $(SAIL_SIM) --no-trace -p -l $(LIMIT_INSTRUCTIONS) --device-tree-blob build/sail.dtb $(elf) -d 0x80000000 |
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 would be nice to make this the default behaviour when passed a PIE executable
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.
Yeah if you don't specify it it should use a sane default like the start of DRAM
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 would be nice to make this the default behaviour when passed a PIE executable
No, you need to specify the offset or the loader will reject to load DYN. for openSBI it can be 0x80000000 but different value may be work too.
sail didn't provide the API for detecting whether it is a DYN or EXEC.
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.
Ok so Sail's load_elf_at_offset is really load_elf_with_dyn(/pie)_offset; that is it will ignore the offset if the provided binary is not ET_DYN. This means you should just use load_elf_at_offset unconditionally with a sensible offset, and if it's an ET_EXEC it will ignore the offset and load at the object's fixed address.
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.
Slightly unfortunate naming. We should definitely add a comment to clarify that behaviour.
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 other option is that we just use our own ELF loading code. That's what I ended up doing (using ELFIO).
| uint64_t begin_sig, end_sig; | ||
| load_elf(f, &is32bit, &entry); | ||
| if (load_offset) { | ||
| // dynamic load |
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.
Uh ET_EXEC should ignore it (or give an error, though that's annoying if loading multiple ELF files, e.g. an ET_DYN OpenSBI and an ET_EXEC kernel)
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.
do you mean we load an fw_jump.elf then with a kernel?
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.
fw_jump vs fw_dynamic is an orthogonal concern (referring to whether OpenSBI hard-codes the location at which the payload is loaded) to whether the kernel being loaded requests a fixed address (that may or may not match OpenSBI's expectation).
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.
currently the sail gives no api on detect whether it is DYN or EXEC.
| fprintf(stderr, "Unable to locate tohost symbol; disabling HTIF.\n"); | ||
| rv_enable_htif = false; | ||
| } else { | ||
| rv_htif_tohost += load_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.
These need a way to know whether load_elf_at_offset used load_offset or not; at first glance I don't see a way to tell. I suspect Sail needs a lookup_sym_with_(dyn/pie_)offset that is like load_elf_at_offset in conditionally including the PIE (dynamic) offset.
Signed-off-by: Ariel Xiong <[email protected]>
|
I prefer and suggest we keep the |
which char do you like? |
|
In the other PR to update the CLI we've removed short options so when you rebase on that I'd just remove it. Also I don't think having a single flag actually works because we support loading multiple elfs. We might need some syntax like |
|
Convert to draft since it needs #1177 merging. |
|
Do we still need this pr? @Timmmm |
|
Yes, if we want to support loading executables at arbitrary offsets. I don't know how much demand there is for that though. What was the original motivation for this? |
|
I think the original motivation was loading the default OpenSBI payload. |
|
If I'm understanding this correctly (not a sure thing), the dynamic part lets the Sail model load ELFs at addresses that are in .json files, as opposed to some fixed address. The .S files should themselves be position independent (because they need to be run. |
That's correct. |
add a
-dparameter for DYN files. Then give a offset, mostly0x80000000like openSBI.