Return riscv_sbi to VMM for further process #352
Conversation
BillXiang
requested review from
acatangiu,
andreeaflorescu,
lauralt,
sameo,
roypat and
ShadowCurse
as code owners
BillXiang
force-pushed
the
riscv64_sbi
branch
from
e051fd3 to
24b9c8a
Compare
| .sum(); | ||
| assert_eq!(dirty_pages, 1); | ||
| break; | ||
| break; |
There was a problem hiding this comment.
Why indent here, make sure you ran cargo fmt before proceeding to next commit 🙂
kvm-ioctls/src/ioctls/vcpu.rs
Outdated
| #[cfg(target_arch = "riscv64")] | ||
| #[test] | ||
| fn test_run_code() { | ||
| use sbi_spec::legacy::*; |
There was a problem hiding this comment.
If you want to use external crate sbi_spec solely in test module, dev-dependencies are the section for them. Honestly I don't think we really need this 🙂
| //sbi_console_getchar | ||
| 0x01, 0x45, // li a0, 0 | ||
| 0x81, 0x45, // li a1, 0 | ||
| 0x01, 0x46, // li a2, 0 | ||
| 0x81, 0x46, // li a3, 0 | ||
| 0x01, 0x47, // li a4, 0 | ||
| 0x81, 0x47, // li a5, 0 | ||
| 0x01, 0x48, // li a6, 0 | ||
| 0x89, 0x48, // li a7, 2 | ||
| 0x73, 0x00, 0x00, 0x00, //ecall | ||
| //sbi_console_putchar | ||
| 0x81, 0x45, // li a1, 0 | ||
| 0x01, 0x46, // li a2, 0 | ||
| 0x81, 0x46, // li a3, 0 | ||
| 0x01, 0x47, // li a4, 0 | ||
| 0x81, 0x47, // li a5, 0 | ||
| 0x01, 0x48, // li a6, 0 | ||
| 0x85, 0x48, // li a7, 1 | ||
| 0x73, 0x00, 0x00, 0x00, //ecall |
There was a problem hiding this comment.
The test_run_code is failing in our CI
kvm-ioctls/src/ioctls/vcpu.rs
Outdated
| LEGACY_CONSOLE_GETCHAR => { | ||
| // SAFETY: Safe because the extension_id (which comes from the kernel) told us | ||
| // ow to use riscv_sbi | ||
| let ch = &mut riscv_sbi.ret[..1]; | ||
| ch[0] = 0x2a; | ||
| } | ||
| LEGACY_CONSOLE_PUTCHAR => { |
There was a problem hiding this comment.
If the two constants are all you need from sbi_spec, you can simply write them explicitly with a comment
kvm-ioctls/CHANGELOG.md
Outdated
| ## Upcoming Release | ||
|
|
||
| - Plumb through KVM_CAP_DIRTY_LOG_RING as DirtyLogRing cap. | ||
| - Return riscv_sbi to VMM for further process. |
There was a problem hiding this comment.
Please refer to other entries to document PR number and link as well
Return riscv_sbi to VMM for further process. Signed-off-by: BillXiang <[email protected]>
BillXiang
force-pushed
the
riscv64_sbi
branch
from
24b9c8a to
c361659
Compare
kvm-ioctls/src/ioctls/vcpu.rs
Outdated
| } | ||
| VcpuExit::RiscvSbi(riscv_sbi) => { | ||
| match riscv_sbi.extension_id as usize { | ||
| LEGACY_CONSOLE_GETCHAR => { |
There was a problem hiding this comment.
I would favor something like
// Constants taken from qemu/target/riscv/sbi_ecall_interface.h
1 /* SBI_EXT_0_1_CONSOLE_PUTCHAR */ => ...
here and drop sbi_spec
BillXiang
force-pushed
the
riscv64_sbi
branch
2 times, most recently
from
6068910 to
8a07d53
Compare
Introduce sbi-spec and add tests for LEGACY_CONSOLE_GETCHAR and LEGACY_CONSOLE_PUTCHAR. Signed-off-by: BillXiang <[email protected]>
Signed-off-by: BillXiang <[email protected]>
BillXiang
force-pushed
the
riscv64_sbi
branch
from
8a07d53 to
b88d32b
Compare
There was a problem hiding this comment.
Code-wise looking good to me, thanks @BillXiang for your patience
Could you work out the tests? Is it working locally?
|
Thanks for the review, @RuoqingHe. As noted earlier, the CI image fails because CONFIG_RISCV_SBI_V01 is disabled; I’ve enabled it in riscv64/build_kernel.sh via #140. |
Yes, I noticed that |
Summary of the PR
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s), and the commit message has max 60 characters for thesummary and max 75 characters for each description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafecode is properly documented.