Skip to content

Conversation

BillXiang
Copy link

Summary of the PR

  • Generate and add bindings for riscv64 sbi ecall interface from qemu-10.1.0 target\riscv\sbi_ecall_interface.h
    using bindgen-cli 0.72.1.
  • Handle console sbi call, which implement early console io while apply 'earlycon=sbi' into kernel parameters.

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.

@roypat
Copy link
Member

roypat commented Sep 9, 2025

@RuoqingHe could you have a look at this one? :o

@BillXiang BillXiang force-pushed the riscv64_sbi_bindings branch from 70ea5dc to 6e53a2a Compare September 9, 2025 12:14
Copy link
Member

@RuoqingHe RuoqingHe left a comment

Choose a reason for hiding this comment

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

6e53a2a should be squashed into other commits

@@ -5,12 +5,14 @@
#[allow(clippy::all)]
#[allow(clippy::undocumented_unsafe_blocks)]
pub mod bindings;
pub mod sbi_bindings;
Copy link
Member

Choose a reason for hiding this comment

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

Please align the bindgen version with the version we used to generate kvm-bindings

let ch = unsafe { &mut run.__bindgen_anon_1.riscv_sbi.ret[..1] };
Ok(VcpuExit::SbiExt0_1ConsoleGetchar(ch))
}
r => Ok(VcpuExit::Unsupported(r)),
Copy link
Member

Choose a reason for hiding this comment

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

Is this match arm correct, maybe _?

Comment on lines 1695 to 1714
#[cfg(target_arch = "riscv64")]
KVM_EXIT_RISCV_SBI => {
match unsafe {run.__bindgen_anon_1.riscv_sbi.extension_id} as u32 {
SBI_EXT_0_1_CONSOLE_PUTCHAR => {
let ch = unsafe { run.__bindgen_anon_1.riscv_sbi.args[0] };
Ok(VcpuExit::SbiExt0_1ConsolePutchar(ch))
}
SBI_EXT_0_1_CONSOLE_GETCHAR => {
let ch = unsafe { &mut run.__bindgen_anon_1.riscv_sbi.ret[..1] };
Ok(VcpuExit::SbiExt0_1ConsoleGetchar(ch))
}
r => Ok(VcpuExit::Unsupported(r)),
}
}
Copy link
Member

Choose a reason for hiding this comment

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

IMHO we should just return __bindgen_anon_1.riscv_sbi here, leave the vmm to decide what to do next

Copy link
Member

@RuoqingHe RuoqingHe left a comment

Choose a reason for hiding this comment

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

IMO we should only introduce KVM_EXIT_RISCV_SBI here and return __bindgen_anon_1.riscv_sbi for vmm to further decide

And those sbi bindings are not from kernel, this would not be the right place for them to live in, perhaps add them to the VMM you are working on 🙂

BillXiang added 4 commits September 9, 2025 20:47
Generate bindings from qemu-10.1.0  target\riscv\sbi_ecall_interface.h
using bindgen-cli 0.72.1

Signed-off-by: BillXiang <[email protected]>
Handle console sbi call, which implement early console io
while apply 'earlycon=sbi' into kernel parameters.

Signed-off-by: BillXiang <[email protected]>
running 1 test
test ioctls::vcpu::tests::test_run_code ... ok

Signed-off-by: BillXiang <[email protected]>
Signed-off-by: BillXiang <[email protected]>
@BillXiang BillXiang force-pushed the riscv64_sbi_bindings branch from 6e53a2a to cfc3c1c Compare September 9, 2025 12:59
@BillXiang
Copy link
Author

Maybe it's better to add those sbi bindings to the VMM.

// SAFETY: Safe because the exit_reason (which comes from the kernel) told us
// which union field to use and the type of extension_id is 'enum sbi_ext_id'.
match unsafe { run.__bindgen_anon_1.riscv_sbi.extension_id } as u32 {
SBI_EXT_0_1_CONSOLE_PUTCHAR => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these should be handled at the KVM crate level - they should be done by the VMM.

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