-
Notifications
You must be signed in to change notification settings - Fork 91
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
elfloader/risc-v: improve OpenSBI support #117
Conversation
20daf3d
to
b0d9f85
Compare
b0d9f85
to
608d24b
Compare
cmake-tool/helpers/rootserver.cmake
Outdated
# When using OpenSBI, the whole system image (usually consisting | ||
# of the ELF-Loader, a device tree, the kernel, and a userland) | ||
# is packaged as an OpenSBI payload. | ||
file(GLOB_RECURSE deps) |
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.
What's happening here? deps looks to be unused, and this cmake command isn't given any paths to glob and so deps should end up empty too.
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've moved the statement back where it was. If this does nothing, we can also remove it completely 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.
Thanks.It looks like it's missing the other parts of it's implementation, actually specifying the path to the opensbi sources and then adding the sources to the depends line of add_custom_command. If you wanted to add a commit to this PR adding ${BBL_PATH}
to the end of that statement, and also adding the deps arg to the ${CMAKE_BINARY_DIR}/bbl/bbl custom command then that'd still come under the PR's goal of "improve OpenSBI support". Alternatively you could ignore it, or remove it if you think it's better to assume that the opensbi implementation isn't going to be edited by anyone between builds.
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.
Can we do this in a separate PR? I will open a dedicated issue for this.
- improve comments and variable names - allow platforms to customize OpenSBI parameters - OPENSBI_PLAT_XLEN - OPENSBI_PLAT_ISA - OPENSBI_PLAT_ABI Signed-off-by: Axel Heider <[email protected]>
608d24b
to
b27c288
Compare
OPENSBI_PLAT_XLEN
OPENSBI_PLAT_ISA
OPENSBI_PLAT_ABI