Skip to content
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

Introduce RISC-V architecture support #190

Merged
merged 8 commits into from
Oct 22, 2024

Conversation

TimePrinciple
Copy link
Contributor

Summary of the PR

Introduce RISC-V architecture to loader, bringing #163 forward.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@TimePrinciple TimePrinciple changed the title Introduce riscv architecture Introduce RISC-V architecture support Sep 11, 2024
@TimePrinciple TimePrinciple marked this pull request as draft September 11, 2024 09:53
@TimePrinciple TimePrinciple force-pushed the introduce-riscv-architecture branch 4 times, most recently from 3d64381 to 8a465c5 Compare October 15, 2024 07:21
@TimePrinciple TimePrinciple marked this pull request as ready for review October 15, 2024 07:31
@TimePrinciple
Copy link
Contributor Author

@roypat @rbradford @ShadowCurse PTAL when convenient :)

rbradford
rbradford previously approved these changes Oct 16, 2024
Copy link
Collaborator

@roypat roypat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there is a lot of code duplicated between the ARM and RISC implementations that doesn't really need to be duplicated like this. Can you have a look at that, to see where we can reuse instead? :o

benches/riscv64/mod.rs Outdated Show resolved Hide resolved
src/configurator/riscv64/fdt.rs Outdated Show resolved Hide resolved
src/loader/riscv64/pe/mod.rs Outdated Show resolved Hide resolved
@TimePrinciple
Copy link
Contributor Author

I feel like there is a lot of code duplicated between the ARM and RISC implementations that doesn't really need to be duplicated like this. Can you have a look at that, to see where we can reuse instead? :o

Yes, I noticed that, I just copied these files incase there is something to be diverged in the future

I can try to merge the common part as possible, but what should we name that, like riscs ? :o

@rbradford
Copy link
Collaborator

rbradford commented Oct 16, 2024

I feel like there is a lot of code duplicated between the ARM and RISC implementations that doesn't really need to be duplicated like this. Can you have a look at that, to see where we can reuse instead? :o

Yes, I noticed that, I just copied these files incase there is something to be diverged in the future

I can try to merge the common part as possible, but what should we name that, like riscs ? :o

Maybe move the fdt code to a higher level and then pub use to expose it from the arch module and keep the same API?

@roypat roypat mentioned this pull request Oct 21, 2024
4 tasks
Add binary used to test loader on `riscv64` platform.

Signed-off-by: Ruoqing He <[email protected]>
TimePrinciple added a commit to TimePrinciple/linux-loader that referenced this pull request Oct 22, 2024
Fix wrongly put rust-vmm#197 changelog and add entry for rust-vmm#190.

Signed-off-by: Ruoqing He <[email protected]>
TimePrinciple added a commit to TimePrinciple/linux-loader that referenced this pull request Oct 22, 2024
Fix wrongly put rust-vmm#197 changelog and add entry for rust-vmm#190.

Signed-off-by: Ruoqing He <[email protected]>
TimePrinciple added a commit to TimePrinciple/linux-loader that referenced this pull request Oct 22, 2024
Fix wrongly put rust-vmm#197 changelog and add entry for rust-vmm#190.

Signed-off-by: Ruoqing He <[email protected]>
RISC-V uses the same image format ARM64 did, reuse the PE image loader
and add support for loading a `riscv64` PE image.

Signed-off-by: Ruoqing He <[email protected]>
Enable `fdt` module in `configurator` on riscv64 architecture.

Signed-off-by: Ruoqing He <[email protected]>
As clippy command in our CI mandates: `cargo clippy --workspace --bins
--examples --benches --all-features --all-targets -- -D warnings -D
clippy::undocumented_unsafe_blocks`, add benchmarck test to appease
clippy.

Signed-off-by: Ruoqing He <[email protected]>
Add entries to document newly introduced support for riscv64.

Signed-off-by: Ruoqing He <[email protected]>
As `rustfmt` suggested, `config` is deprecated, moving to `config.toml`.

Signed-off-by: Ruoqing He <[email protected]>
Add `.platform` file to enable x86_64, aarch64, riscv64 CI.

Signed-off-by: Ruoqing He <[email protected]>
Fix wrongly put rust-vmm#197 changelog and add entry for rust-vmm#190.

Signed-off-by: Ruoqing He <[email protected]>
Copy link
Collaborator

@rbradford rbradford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the refactoring this change is lovely and compact.

@rbradford rbradford merged commit 4bc7015 into rust-vmm:main Oct 22, 2024
2 checks passed
@TimePrinciple TimePrinciple deleted the introduce-riscv-architecture branch October 22, 2024 13:11
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.

4 participants