-
Notifications
You must be signed in to change notification settings - Fork 74
riscv: Initial support with v6.5 bindings #92
Conversation
46edc05
to
139a008
Compare
Hi @endeneer, welcome to rust-vmm! Thank you for your contribution :) We've been getting questions about Risc-V support every now and again, so its nice to see something happening in that direction. I was just wondering, do you have an outline of all the changes that will be required across rust-vmm crates to support Risc-V? E.g. CI/testing, how to integrate Risc-V support into other fundamental crates such as https://github.com/rust-vmm/linux-loader or https://github.com/rust-vmm/vm-memory. Just so we have an overview over where these efforts will be going :) Also, it'd be nice to have all kernel bindings be generated from the same kernel version. Currently, x86_64 and aarch64 are generated from 6.2, can be use the same base for Risc-V? |
139a008
to
60bf0ef
Compare
Initially I think cross-compiling the bindings would be a good start: a cross compiling GH action example can be found here - https://github.com/rust-vmm/acpi_tables/blob/main/.github/workflows/quality.yaml#L14 vm-memory shouldn't need any changes, for linux-loader the PE loader from aarch64 can be reused for RISC-V
I agree; I would like to see the same version used consistently. |
fcb771d
to
9d8356f
Compare
Sorry for being ambiguous - you need to bump all the architectures to 6.5. Kernel headers from 6.2 are not really sufficient for virtualisation on RISC-V (no AIA/IMSIC.) |
It would be nice to have AIA/IMSIC/IOMMU RISC-V support, but IMHO v6.2 should be more than enough for |
It's really not useful since without IMSIC you can't do MSI which really limits the usefulness for PCI. It would be fine for something like Firecracker but Cloud Hypervisor relies on PCI for the virtio devices. If your look at the other Rust based VMM that supports RV64 (CrosVM) it's bindings are generated from a kernel branch including the AIA changes. By not using the latest version you're just creating more work in the future when we have to update to v6.5 to make the bindings useful. |
9d8356f
to
54d4019
Compare
54d4019
to
5e6fee4
Compare
src/riscv/mod.rs
Outdated
// #[cfg(feature = "fam-wrappers")] | ||
// pub mod fam_wrappers; |
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.
Why do we not do the FamStruct stuff for risc? :o If they're simply not needed, then probably this comment should also be removed
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.
Not seeing any FAM usage in RISC-V KVM API in Linux v6.5, so I have removed the comment altogether.
src/riscv/mod.rs
Outdated
// Keep this until https://github.com/rust-lang/rust-bindgen/issues/1651 is fixed. | ||
#[cfg_attr(test, allow(deref_nullptr))] |
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 think this is necessary anymore, the linked issue got resolved a while ago, and the bindgen version you used to generate the bindings does not generate unsound derefs anymore (I think with a recent compiler version they actually became a hard-error, so we must've fixed it for the other bindings at some point, 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.
Ah I see, I have removed it.
Dear @endeneer, Great that you have done this first step in adding support for RISC-V in rust-vmm. In the context of the Vitamin-V EU project we are also working on this. For the time being we are updating (Sorry for abusing of your PR's message thread, but I did not know where else to contact you :) maybe Slack is a better place?) |
@arigo-vos Here is the summary of what I've done so far for RISC-V:
I have not started working on
My original aim is to enable Kata Containers on RISC-V but diverted due to
I just joined Slack ( |
Thanks for the prompt and detailed response. I agree about AIA support, however we'd like to test it as soon as possible on real hardware (at the moment, as far as I know, nothing supports the H extension). So I'm wondering whether the new hardware that will come will support both H and AIA or only H. Depending on this, we might also consider to add PLIC emulation. Anyway, let's continue the discussion on Slack to not clobber this thread. |
Generated based on Linux tag v6.5. Signed-off-by: Tan En De <[email protected]>
Hi @endeneer @arigo-vos ! I crossed this MR after starting an attempt to port kvm-ioctl and kvm-bindings to RISC-V myself. In my case I'm using a full emulated QEMU RISC-V host running Ubuntu 22. What's the status here? Anything I can do to help? I see the discussions about PLIC and AIA but I understand this isn't something that an initial port should worry about. Thanks! |
Hello @danielhb , We have been working on the |
Well my understanding is that without AIA we can't do MSI and from my perspective that also limits our ability to do virtio-pci as Cloud Hypervisor only does virtio-pci with MSI(-X). Anyway - the required functionality is now merged into Linus's tree so this should be in v6.9 and from there we can update this PR using that version. |
Generated based on Linux tag v6.5.
Summary of the PR
Related: #80
The first step of porting Rust virtualization stack to RISC-V is to enable
kvm-bindings
:-)The next step will be porting
kvm-ioctls
.Currently there is no CI for RISC-V, I will continue studying the
rust-vmm-ci
docs tomorrow.Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s
), and the commitmessage has max 60 characters for the summary and max 75 characters for each
description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafe
code is properly documented.