Manos Pitsidianakis <manos.pitsidiana...@linaro.org> writes:
> This commit adds a re-implementation of hw/char/pl011.c in Rust.
>
> How to build:
>
> 1. Configure a QEMU build with:
> --enable-system --target-list=aarch64-softmmu --enable-rust
> 2. Launching a VM with qemu-system-aarch64 should use the Rust version
> of the pl011 device
>
> Co-authored-by: Junjie Mao <junjie....@intel.com>
> Co-authored-by: Paolo Bonzini <pbonz...@redhat.com>
> Signed-off-by: Junjie Mao <junjie....@intel.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
> ---
[snip]
> diff --git a/rust/hw/char/pl011/src/device.rs
> b/rust/hw/char/pl011/src/device.rs
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..6bd121b83ae831e838b05d83b67c698474b00b4a
> --- /dev/null
> +++ b/rust/hw/char/pl011/src/device.rs
> @@ -0,0 +1,600 @@
> +// Copyright 2024, Linaro Limited
> +// Author(s): Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +use core::{
> + ffi::{c_int, c_uchar, c_uint, c_void, CStr},
> + ptr::{addr_of, addr_of_mut, NonNull},
> +};
> +
> +use qemu_api::{
> + bindings::{self, *},
> + definitions::ObjectImpl,
> +};
> +
> +use crate::{
> + memory_ops::PL011_OPS,
> + registers::{self, Interrupt},
> + RegisterOffset,
> +};
> +
> +static PL011_ID_ARM: [c_uchar; 8] = [0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0,
> 0x05, 0xb1];
> +
> +const DATA_BREAK: u32 = 1 << 10;
> +
> +/// QEMU sourced constant.
> +pub const PL011_FIFO_DEPTH: usize = 16_usize;
> +
> +#[repr(C)]
> +#[derive(Debug, qemu_api_macros::Object)]
> +/// PL011 Device Model in QEMU
> +pub struct PL011State {
> + pub parent_obj: SysBusDevice,
> + pub iomem: MemoryRegion,
> + #[doc(alias = "fr")]
> + pub flags: registers::Flags,
> + #[doc(alias = "lcr")]
> + pub line_control: registers::LineControl,
> + #[doc(alias = "rsr")]
> + pub receive_status_error_clear: registers::ReceiveStatusErrorClear,
> + #[doc(alias = "cr")]
> + pub control: registers::Control,
> + pub dmacr: u32,
> + pub int_enabled: u32,
> + pub int_level: u32,
> + pub read_fifo: [u32; PL011_FIFO_DEPTH],
> + pub ilpr: u32,
> + pub ibrd: u32,
> + pub fbrd: u32,
> + pub ifl: u32,
Some of the fields can be private to the implementation of PL011State.
> + pub read_pos: usize,
> + pub read_count: usize,
> + pub read_trigger: usize,
> + #[doc(alias = "chr")]
> + pub char_backend: CharBackend,
> + /// QEMU interrupts
> + ///
> + /// ```text
> + /// * sysbus MMIO region 0: device registers
> + /// * sysbus IRQ 0: `UARTINTR` (combined interrupt line)
> + /// * sysbus IRQ 1: `UARTRXINTR` (receive FIFO interrupt line)
> + /// * sysbus IRQ 2: `UARTTXINTR` (transmit FIFO interrupt line)
> + /// * sysbus IRQ 3: `UARTRTINTR` (receive timeout interrupt line)
> + /// * sysbus IRQ 4: `UARTMSINTR` (momem status interrupt line)
> + /// * sysbus IRQ 5: `UARTEINTR` (error interrupt line)
> + /// ```
> + #[doc(alias = "irq")]
> + pub interrupts: [qemu_irq; 6usize],
> + #[doc(alias = "clk")]
> + pub clock: NonNull<Clock>,
> + #[doc(alias = "migrate_clk")]
> + pub migrate_clock: bool,
> +}
> +
> +impl ObjectImpl for PL011State {
> + type Class = PL011Class;
> + const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! {
> Self };
> + const TYPE_NAME: &'static CStr = c"pl011";
Can use crate::definitions::TYPE_PL011 to avoid duplication.
> + const PARENT_TYPE_NAME: Option<&'static CStr> =
> Some(TYPE_SYS_BUS_DEVICE);
> + const ABSTRACT: bool = false;
> + const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> =
> Some(pl011_init);
> + const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)>
> = None;
> + const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)>
> = None;
> +}
> +
[snip]
> +
> +/// # Safety
> +///
> +/// We expect the FFI user of this function to pass a valid pointer, that has
> +/// the same size as [`PL011State`]. We also expect the device is
> +/// readable/writeable from one thread at any time.
> +#[no_mangle]
> +pub unsafe extern "C" fn pl011_can_receive(opaque: *mut c_void) -> c_int {
> + unsafe {
> + assert!(!opaque.is_null());
> + let state = NonNull::new_unchecked(opaque.cast::<PL011State>());
Those two lines can be combined as:
let state = NonNull::new(opaque.cast::<PL011State>()).unwrap();
Alternatively, use debug_assert! if the assertion is only meant to be
active in debug builds.
> + state.as_ref().can_receive().into()
> + }
> +}
> +
> +/// # Safety
> +///
> +/// We expect the FFI user of this function to pass a valid pointer, that has
> +/// the same size as [`PL011State`]. We also expect the device is
> +/// readable/writeable from one thread at any time.
> +///
> +/// The buffer and size arguments must also be valid.
> +#[no_mangle]
> +pub unsafe extern "C" fn pl011_receive(
> + opaque: *mut core::ffi::c_void,
> + buf: *const u8,
> + size: core::ffi::c_int,
> +) {
> + unsafe {
> + assert!(!opaque.is_null());
> + let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
> + if state.as_ref().loopback_enabled() {
> + return;
> + }
> + if size > 0 {
> + assert!(!buf.is_null());
> + state.as_mut().put_fifo(c_uint::from(buf.read_volatile()))
> + }
> + }
> +}
> +
> +/// # Safety
> +///
> +/// We expect the FFI user of this function to pass a valid pointer, that has
> +/// the same size as [`PL011State`]. We also expect the device is
> +/// readable/writeable from one thread at any time.
> +#[no_mangle]
> +pub unsafe extern "C" fn pl011_event(opaque: *mut core::ffi::c_void, event:
> QEMUChrEvent) {
> + unsafe {
> + assert!(!opaque.is_null());
> + let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
> + state.as_mut().event(event)
> + }
> +}
> +
> +/// # Safety
> +///
> +/// We expect the FFI user of this function to pass a valid pointer for
> `chr`.
> +#[no_mangle]
> +pub unsafe extern "C" fn pl011_create(
> + addr: u64,
> + irq: qemu_irq,
> + chr: *mut Chardev,
> +) -> *mut DeviceState {
> + unsafe {
> + let dev: *mut DeviceState = qdev_new(PL011State::TYPE_INFO.name);
> + assert!(!dev.is_null());
qdev_new() aborts on error. So the assert! above is unnecessary.
> + let sysbus: *mut SysBusDevice = dev as *mut SysBusDevice;
In C such casts are typically done using the object checker generated by
the DECLARE_INSTANCE_CHECKER macro. With qom_cast_debug the checker is
able to capture casts to wrong types.
As a future work, similar mechanism should be available in Rust for such
casts.
> +
> + qdev_prop_set_chr(dev, bindings::TYPE_CHARDEV.as_ptr(), chr);
> + sysbus_realize_and_unref(sysbus, addr_of!(error_fatal) as *mut *mut
> Error);
> + sysbus_mmio_map(sysbus, 0, addr);
> + sysbus_connect_irq(sysbus, 0, irq);
> + dev
> + }
> +}
> +
> +/// # Safety
> +///
> +/// We expect the FFI user of this function to pass a valid pointer, that has
> +/// the same size as [`PL011State`]. We also expect the device is
> +/// readable/writeable from one thread at any time.
> +#[no_mangle]
I don't think #[no_mangle] is needed for functions being exposed to C
via function pointers. The annotated function has external linkage,
while the common practice is to make such callbacks static.
> +pub unsafe extern "C" fn pl011_init(obj: *mut Object) {
> + unsafe {
> + assert!(!obj.is_null());
> + let mut state = NonNull::new_unchecked(obj.cast::<PL011State>());
> + state.as_mut().init();
> + }
> +}
[snip]
> diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..541109d4d8f87a70748820cecee24656802a6350
> --- /dev/null
> +++ b/rust/hw/char/pl011/src/lib.rs
> @@ -0,0 +1,586 @@
> +// Copyright 2024, Linaro Limited
> +// Author(s): Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +//
> +// PL011 QEMU Device Model
> +//
> +// This library implements a device model for the PrimeCell® UART (PL011)
> +// device in QEMU.
> +//
> +#![doc = include_str!("../README.md")]
> +//! # Library crate
> +//!
> +//! See [`PL011State`](crate::device::PL011State) for the device model type
> and
> +//! the [`registers`] module for register types.
> +
> +#![deny(
> + unsafe_op_in_unsafe_fn,
Shall we make this builtin lint denied for all crates? That can be done
by adding '-D unsafe_op_in_unsafe_fn' to rustc argument.
> + rustdoc::broken_intra_doc_links,
> + rustdoc::redundant_explicit_links,
> + clippy::correctness,
> + clippy::suspicious,
> + clippy::complexity,
> + clippy::perf,
> + clippy::cargo,
> + clippy::nursery,
> + clippy::style,
> + // restriction group
> + clippy::dbg_macro,
> + clippy::as_underscore,
> + clippy::assertions_on_result_states,
> + // pedantic group
> + clippy::doc_markdown,
> + clippy::borrow_as_ptr,
> + clippy::cast_lossless,
> + clippy::option_if_let_else,
> + clippy::missing_const_for_fn,
> + clippy::cognitive_complexity,
> + clippy::missing_safety_doc,
> + )]
> +
--
Best Regards
Junjie Mao
