diff mbox series

[RFC,12/13] rust/timer/hpet: add qdev APIs support

Message ID 20241205060714.256270-13-zhao1.liu@intel.com (mailing list archive)
State New
Headers show
Series rust: Reinvent the wheel for HPET timer in Rust | expand

Commit Message

Zhao Liu Dec. 5, 2024, 6:07 a.m. UTC
Implement QAPI support for HPET device in qdev.rs.

Additionally, wrap the handling of HPET internal details as traits to be
specifically implemented in hpet.rs.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/timer/hpet/src/fw_cfg.rs |   2 -
 rust/hw/timer/hpet/src/hpet.rs   | 232 ++++++++++++++++++++++++++++++-
 rust/hw/timer/hpet/src/lib.rs    |   5 +
 rust/hw/timer/hpet/src/qdev.rs   | 133 ++++++++++++++++++
 4 files changed, 365 insertions(+), 7 deletions(-)
 create mode 100644 rust/hw/timer/hpet/src/qdev.rs

Comments

Paolo Bonzini Dec. 5, 2024, 6:58 p.m. UTC | #1
On 12/5/24 07:07, Zhao Liu wrote:
> Implement QAPI support for HPET device in qdev.rs.
> 
> Additionally, wrap the handling of HPET internal details as traits to be
> specifically implemented in hpet.rs.

Why not just put everything in a simple file to avoid the traits?

> +pub(crate) trait RamOps {
> +    fn read(&mut self, addr: hwaddr, _size: c_uint) -> u64;
> +    fn write(&mut self, addr: hwaddr, value: u64, size: u64);
> +}

Please try as much as possible to make these "&self", same for the GPIO 
handler.  That's the point of BqlCell/BqlRefCell. :)

Paolo
Zhao Liu Dec. 7, 2024, 4:05 p.m. UTC | #2
On Thu, Dec 05, 2024 at 07:58:07PM +0100, Paolo Bonzini wrote:
> Date: Thu, 5 Dec 2024 19:58:07 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [RFC 12/13] rust/timer/hpet: add qdev APIs support
> 
> On 12/5/24 07:07, Zhao Liu wrote:
> > Implement QAPI support for HPET device in qdev.rs.
> > 
> > Additionally, wrap the handling of HPET internal details as traits to be
> > specifically implemented in hpet.rs.
> 
> Why not just put everything in a simple file to avoid the traits?

Ah, I considered that qdev has many unsafe C-style wrappers, to avoid
confusion with HPET state/timer methods... However, looking back now,
qdev and qom are already quite clean. I will merge this into hpet.rs in
the next version.

> > +pub(crate) trait RamOps {
> > +    fn read(&mut self, addr: hwaddr, _size: c_uint) -> u64;
> > +    fn write(&mut self, addr: hwaddr, value: u64, size: u64);
> > +}
> 
> Please try as much as possible to make these "&self", same for the GPIO
> handler.  That's the point of BqlCell/BqlRefCell. :)

Sure! Will do as you suggested.

Regards,
Zhao
diff mbox series

Patch

diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs
index a057c2778be4..6515a428cebb 100644
--- a/rust/hw/timer/hpet/src/fw_cfg.rs
+++ b/rust/hw/timer/hpet/src/fw_cfg.rs
@@ -2,8 +2,6 @@ 
 // Author(s): Zhao Liu <zhai1.liu@intel.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-#![allow(dead_code)]
-
 use qemu_api::{cell::BqlCell, zeroable::Zeroable};
 
 // Each HPETState represents a Event Timer Block. The v1 spec supports
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 9550d8fe438a..9480633a77dd 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -2,10 +2,8 @@ 
 // Author(s): Zhao Liu <zhai1.liu@intel.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-#![allow(dead_code)]
-
-use core::ptr::{null_mut, NonNull};
-use std::os::raw::c_int;
+use core::ptr::{addr_of_mut, null_mut, NonNull};
+use std::os::raw::{c_uint, c_void};
 
 use qemu_api::{
     bindings::*,
@@ -13,9 +11,14 @@ 
     cell::{BqlCell, BqlRefCell},
     irq::InterruptSource,
     memattrs::MEMTXATTRS_UNSPECIFIED,
+    qdev::DeviceGPIOImpl,
+    qom::ObjectType,
     timer::{qemu_clock_get_virtual_ns, QEMUTimerImpl},
+    zeroable::Zeroable,
 };
 
+use crate::{fw_cfg::*, qdev::*};
+
 // Register space for each timer block. (HPET_BASE isn't defined here.)
 const HPET_REG_SPACE_LEN: u64 = 0x400; // 1024 bytes
 
@@ -453,6 +456,38 @@  fn callback(&mut self) {
     }
 }
 
+impl RamOps for HPETTimer {
+    fn read(&mut self, addr: hwaddr, _size: c_uint) -> u64 {
+        let shift: u64 = (addr & 4) * 8;
+
+        match addr & 0x18 {
+            HPET_TN_CFG_REG => self.config >> shift, // including interrupt capabilities
+            HPET_TN_CMP_REG => self.cmp >> shift,    // comparator register
+            HPET_TN_FSB_ROUTE_REG => self.fsb >> shift,
+            _ => {
+                // TODO: Add trace point - trace_hpet_ram_read_invalid()
+                // Reserved.
+                0
+            }
+        }
+    }
+
+    fn write(&mut self, addr: hwaddr, value: u64, size: u64) {
+        let shift = ((addr & 4) * 8) as usize;
+        let len = std::cmp::min(size * 8, 64 - shift as u64) as usize;
+
+        match addr & 0x18 {
+            HPET_TN_CFG_REG => self.set_tn_cfg_reg(shift, len, value),
+            HPET_TN_CMP_REG => self.set_tn_cmp_reg(shift, len, value),
+            HPET_TN_FSB_ROUTE_REG => self.set_tn_fsb_route_reg(shift, len, value),
+            _ => {
+                // TODO: Add trace point - trace_hpet_ram_write_invalid()
+                // Reserved.
+            }
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct HPETTimerInstance(BqlRefCell<HPETTimer>);
 
@@ -466,7 +501,7 @@  fn timer_handler(timer: &mut HPETTimerInstance) {
 /// Note: Wrap all items that may be changed in the callback called by C
 /// into the BqlCell/BqlRefCell.
 #[repr(C)]
-#[derive(Debug, qemu_api_macros::offsets)]
+#[derive(Debug, qemu_api_macros::Object, qemu_api_macros::offsets)]
 pub struct HPETState {
     parent_obj: SysBusDevice,
     iomem: MemoryRegion,
@@ -636,3 +671,190 @@  impl QEMUTimerImpl for HPETState {
     const QEMU_TIMER_CB: Option<fn(opaque: &mut HPETTimerInstance)> =
         Some(HPETTimerInstance::timer_handler);
 }
+
+impl ObjOps for HPETState {
+    // TODO: Add binding to register idiomatic Rust callback.
+    const HPET_RAM_OPS: MemoryRegionOps = MemoryRegionOps {
+        read: Some(hpet_ram_read),
+        write: Some(hpet_ram_write),
+        read_with_attrs: None,
+        write_with_attrs: None,
+        valid: MemoryRegionOps__bindgen_ty_1 {
+            min_access_size: 4,
+            max_access_size: 8,
+            ..Zeroable::ZERO
+        },
+        impl_: MemoryRegionOps__bindgen_ty_2 {
+            min_access_size: 4,
+            max_access_size: 8,
+            ..Zeroable::ZERO
+        },
+        endianness: device_endian::DEVICE_NATIVE_ENDIAN,
+    };
+
+    unsafe fn init(&mut self) {
+        // SAFETY:
+        // self and self.iomem are guaranteed to be valid at this point since callers
+        // must make sure the `self` reference is valid.
+        unsafe {
+            memory_region_init_io(
+                addr_of_mut!(self.iomem),
+                addr_of_mut!(*self).cast::<Object>(),
+                &Self::HPET_RAM_OPS,
+                addr_of_mut!(*self).cast::<c_void>(),
+                Self::TYPE_NAME.as_ptr(),
+                HPET_REG_SPACE_LEN,
+            );
+            let sbd = addr_of_mut!(*self).cast::<SysBusDevice>();
+            sysbus_init_mmio(sbd, addr_of_mut!(self.iomem));
+        }
+    }
+}
+
+impl DeviceGPIOImpl for HPETState {
+    const GPIO_IRQ_HANDLER: Option<fn(&mut Self, lines_num: u32, level: u32)> =
+        Some(HPETState::handle_legacy_irq);
+}
+
+impl QDevOps for HPETState {
+    fn realize(&mut self) {
+        // SAFETY:
+        // caller of C Qdev guarantees that the "self" passed in is a
+        // valid HPETState reference, so it is able to cast as SysBusDevice.
+        let sbd = unsafe { &mut *(addr_of_mut!(*self).cast::<SysBusDevice>()) };
+
+        if self.int_route_cap == 0 {
+            // TODO: Add error binding: warn_report()
+            println!("Hpet's hpet-intcap property not initialized");
+        }
+
+        // SAFETY:
+        // hpet_fw_cfg is contained in a BqlCell and is protected by BQL,
+        // so it's safe to modify it.
+        self.hpet_id.set(unsafe { hpet_fw_cfg.assign_hpet_id() });
+
+        for irq in self.irqs.iter() {
+            sbd.init_irq(irq);
+        }
+
+        if self.num_timers.get() < HPET_MIN_TIMERS {
+            self.num_timers.set(HPET_MIN_TIMERS);
+        } else if self.num_timers.get() > HPET_MAX_TIMERS {
+            self.num_timers.set(HPET_MAX_TIMERS);
+        }
+
+        self.init_timer();
+        // 64-bit General Capabilities and ID Register; LegacyReplacementRoute.
+        self.capability.set(
+            HPET_CAP_REV_ID_VALUE << HPET_CAP_REV_ID_SHIFT |
+            1 << HPET_CAP_COUNT_SIZE_CAP_SHIFT |
+            1 << HPET_CAP_LEG_RT_CAP_SHIFT |
+            HPET_CAP_VENDER_ID_VALUE << HPET_CAP_VENDER_ID_SHIFT |
+            ((self.num_timers.get() - 1) as u64) << HPET_CAP_NUM_TIM_SHIFT | // indicate the last timer
+            (HPET_CLK_PERIOD * FS_PER_NS) << HPET_CAP_CNT_CLK_PERIOD_SHIFT, // 10 ns
+        );
+
+        self.init_gpio_in(2);
+        self.init_gpio_out(&self.pit_enabled, 1);
+    }
+
+    fn reset(&mut self) {
+        let sbd: *mut SysBusDevice = addr_of_mut!(*self).cast::<SysBusDevice>();
+
+        for i in 0..self.num_timers.get() {
+            self.get_timer(i).borrow_mut().reset();
+        }
+
+        self.pit_enabled.set(true);
+        self.counter.set(0);
+        self.hpet_offset.set(0);
+        self.config.set(0);
+        // SAFETY:
+        // hpet_fw_cfg is contained in a BqlCell and is protected by BQL,
+        // so it's safe to modify it.
+        unsafe {
+            hpet_fw_cfg.update_hpet_cfg(
+                self.hpet_id.get(),
+                Some(self.capability.get() as u32),
+                Some((*sbd).mmio[0].addr),
+            )
+        };
+
+        // to document that the RTC lowers its output on reset as well
+        self.rtc_irq_level.set(0);
+    }
+}
+
+impl RamOps for HPETState {
+    fn read(&mut self, addr: hwaddr, size: c_uint) -> u64 {
+        let shift: u64 = (addr & 4) * 8;
+
+        // address range of all TN regs
+        if (0x100..=0x3ff).contains(&addr) {
+            let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
+
+            // TODO: Add trace point - trace_hpet_ram_read(addr)
+            if timer_id > self.num_timers.get() {
+                // TODO: Add trace point - trace_hpet_timer_id_out_of_range(timer_id)
+                // Reserved.
+                return 0;
+            }
+
+            self.get_timer(timer_id).borrow_mut().read(addr, size)
+        } else {
+            match addr & !4 {
+                HPET_CAP_REG => self.capability.get() >> shift, /* including HPET_PERIOD 0x004 */
+                // (CNT_CLK_PERIOD field)
+                HPET_CFG_REG => self.config.get() >> shift,
+                HPET_COUNTER_REG => {
+                    let cur_tick: u64 = if self.is_hpet_enabled() {
+                        self.get_ticks()
+                    } else {
+                        self.counter.get()
+                    };
+
+                    // TODO: Add trace point - trace_hpet_ram_read_reading_counter(addr & 4,
+                    // cur_tick)
+                    cur_tick >> shift
+                }
+                HPET_INT_STATUS_REG => self.int_status.get() >> shift,
+                _ => {
+                    // TODO: Add trace point- trace_hpet_ram_read_invalid()
+                    // Reserved.
+                    0
+                }
+            }
+        }
+    }
+
+    fn write(&mut self, addr: hwaddr, value: u64, size: u64) {
+        let shift = ((addr & 4) * 8) as usize;
+        let len = std::cmp::min(size * 8, 64 - shift as u64) as usize;
+
+        // TODO: Add trace point - trace_hpet_ram_write(addr, value)
+        if (0x100..=0x3ff).contains(&addr) {
+            let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
+
+            // TODO: Add trace point - trace_hpet_ram_write_timer_id(timer_id)
+            if timer_id > self.num_timers.get() {
+                // TODO: Add trace point - trace_hpet_timer_id_out_of_range(timer_id)
+                return;
+            }
+
+            self.get_timer(timer_id)
+                .borrow_mut()
+                .write(addr, value, size)
+        } else {
+            match addr & !0x4 {
+                HPET_CAP_REG => {} // General Capabilities and ID Register: Read Only
+                HPET_CFG_REG => self.set_cfg_reg(shift, len, value),
+                HPET_INT_STATUS_REG => self.set_int_status_reg(shift, len, value),
+                HPET_COUNTER_REG => self.set_counter_reg(shift, len, value),
+                _ => {
+                    // TODO: Add trace point - trace_hpet_ram_write_invalid()
+                    // Reserved.
+                }
+            }
+        }
+    }
+}
diff --git a/rust/hw/timer/hpet/src/lib.rs b/rust/hw/timer/hpet/src/lib.rs
index 387913bbdfb9..1692dbf19a85 100644
--- a/rust/hw/timer/hpet/src/lib.rs
+++ b/rust/hw/timer/hpet/src/lib.rs
@@ -10,7 +10,12 @@ 
 
 #![deny(unsafe_op_in_unsafe_fn)]
 
+use qemu_api::c_str;
+
 extern crate qemu_api;
 
 pub mod fw_cfg;
 pub mod hpet;
+pub mod qdev;
+
+pub const TYPE_HPET: &::std::ffi::CStr = c_str!("hpet");
diff --git a/rust/hw/timer/hpet/src/qdev.rs b/rust/hw/timer/hpet/src/qdev.rs
new file mode 100644
index 000000000000..6ddfc9422d78
--- /dev/null
+++ b/rust/hw/timer/hpet/src/qdev.rs
@@ -0,0 +1,133 @@ 
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use core::ptr::NonNull;
+use std::{
+    ffi::CStr,
+    os::raw::{c_uint, c_void},
+};
+
+use qemu_api::{
+    bindings::*,
+    c_str,
+    qdev::DeviceImpl,
+    qom::{ClassInitImpl, ObjectImpl, ObjectType},
+    qom_isa,
+};
+
+use crate::hpet::*;
+
+qom_isa!(HPETState : SysBusDevice, DeviceState, Object);
+
+// TODO: add OBJECT_DECLARE_SIMPLE_TYPE.
+#[repr(C)]
+pub struct HPETClass {
+    parent_class: <SysBusDevice as ObjectType>::Class,
+}
+
+unsafe impl ObjectType for HPETState {
+    type Class = HPETClass;
+    const TYPE_NAME: &'static CStr = crate::TYPE_HPET;
+}
+
+impl ClassInitImpl<HPETClass> for HPETState {
+    fn class_init(klass: &mut HPETClass) {
+        <Self as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut klass.parent_class);
+    }
+}
+
+pub(crate) trait ObjOps {
+    const HPET_RAM_OPS: MemoryRegionOps;
+    unsafe fn init(&mut self);
+}
+
+impl ObjectImpl for HPETState {
+    type ParentType = SysBusDevice;
+
+    const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
+}
+
+pub(crate) trait RamOps {
+    fn read(&mut self, addr: hwaddr, _size: c_uint) -> u64;
+    fn write(&mut self, addr: hwaddr, value: u64, size: u64);
+}
+
+pub(crate) unsafe extern "C" fn hpet_ram_read(
+    opaque: *mut c_void,
+    addr: hwaddr,
+    size: c_uint,
+) -> u64 {
+    // SAFETY:
+    // the pointer is convertible to a reference
+    let state: &mut HPETState =
+        unsafe { NonNull::new(opaque.cast::<HPETState>()).unwrap().as_mut() };
+
+    state.read(addr, size)
+}
+
+pub(crate) unsafe extern "C" fn hpet_ram_write(
+    opaque: *mut c_void,
+    addr: hwaddr,
+    data: u64,
+    size: c_uint,
+) {
+    // SAFETY:
+    // the pointer is convertible to a reference
+    let state: &mut HPETState =
+        unsafe { NonNull::new(opaque.cast::<HPETState>()).unwrap().as_mut() };
+
+    state.write(addr, data, size as u64);
+}
+
+// TODO: Make these properties user-configurable!
+qemu_api::declare_properties! {
+    HPET_PROPERTIES,
+    qemu_api::define_property!(
+        c_str!("timers"),
+        HPETState,
+        num_timers,
+        unsafe { &qdev_prop_uint8 },
+        u8,
+        default = HPET_MIN_TIMERS
+    ),
+    qemu_api::define_property!(
+        c_str!("msi"),
+        HPETState,
+        flags,
+        unsafe { &qdev_prop_bit },
+        u32,
+        bit = HPET_FLAG_MSI_SUPPORT_SHIFT as u8,
+        default = false,
+    ),
+    qemu_api::define_property!(
+        c_str!("hpet-intcap"),
+        HPETState,
+        int_route_cap,
+        unsafe { &qdev_prop_uint32 },
+        u32,
+        default = 0
+    ),
+    qemu_api::define_property!(
+        c_str!("hpet-offset-saved"),
+        HPETState,
+        hpet_offset_saved,
+        unsafe { &qdev_prop_bool },
+        bool,
+        default = true
+    ),
+}
+
+pub(crate) trait QDevOps {
+    fn realize(&mut self);
+    fn reset(&mut self);
+}
+
+impl DeviceImpl for HPETState {
+    fn properties() -> &'static [Property] {
+        &HPET_PROPERTIES
+    }
+
+    const REALIZE: Option<fn(&mut Self)> = Some(Self::realize);
+    const RESET: Option<fn(&mut Self)> = Some(Self::reset);
+}