qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/10] rust: Add HPET timer device
@ 2025-02-10  3:00 Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to hpet.c Zhao Liu
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

Hi folks,

This is Rust HPET normal patch series, and you can find the previous RFC
at [1].

This series is based on Paolo's rust-next branch at the commit
43c0ef3bfb3b ("rust: vmstate: remove redundant link targets") along with
another 2 more seperate patches:

* https://lore.kernel.org/qemu-devel/20241205203721.347233-1-pbonzini@redhat.com/
* https://lore.kernel.org/qemu-devel/20250121140121.84550-1-zhao1.liu@intel.com/

Overall, HPET in Rust maintains the same logic as the original C
version, adhering to the Intel IA PC-HPET spec v1.0a [2]. While keeping
the logic unchanged, it attempts to keep up with the current development
progress of Rust for QEMU, leveraging the latest and ongoing Rust binding
updates as much as possible, such as BqlCell / BqlRefCell, qom & qdev
enhancements, irq binding, and more.

Additionally, it introduces new bindings, including gpio_{in|out},
memattrs, and timer.

Welcome your comments and feedback!


TODOs
=====

* Add more documentations for new bindings and HPET.
* Live migration support for HPET.
* address-spaces binding (for address_space_memory, address_space_stl_le).


Introduction
============

.
│ 
...
└── timer
    ├── hpet
    │   ├── Cargo.toml
    │   ├── meson.build
    │   └── src
    │       ├── fw_cfg.rs
    │       ├── hpet.rs
    │       ├── lib.rs
    ├── Kconfig
    └── meson.build


HPET emulation contains 2 parts:
 * HPET device emulation:
   - hpet.rs:
     It includes basic operations for the HPET timer and HPET state
     (which actually represents the HPET timer block).

     Here, similar to the C implementation, it directly defines the
     registers and bit shifts as const variables, without a complete
     register space structure.

     And, it implements various QEMU qdev/qom required traits for the
     HPETState.

 * Global HPET firmwrie configuration:
   - fw_cfg.rs
     In the C code, there is a variable hpet_fw_cfg (old name: hpet_cfg)
     used to configure the number of HPET timer blocks and the basic
     HPET firmware configuration. It is defined in .c file and is
     referenced as extern in the .h file.

     For the Rust HPET, fw_cfg.rs also implementes hpet_fw_cfg so that
     the .h file can still reference it.


Change Log
==========

Main changes since Patch v1:
 * Reimplement InterruptSource::slice_as_ptr() by dereferring `slice[0]`.
 * Ensure the callback parameter of input GPIO/timer is not none.
 * Use Timer/TimerListGroup in Rust code instead of QEMUTimer/
   QEMUTimerListGroup.
 * Add MS/US/NS wrappers.
 * Enable workspace's lint configurations in hpet's Cargo.toml.
 * Add rust_version in Cargo.toml.
 * Fix complaints from cargo (+nightly) doc/clippy/fmt (v1.84.0).
 * Do `addr & !4` in timer's register read and write.
 * Mask the timer N's register address with 0x1F (timer_and_addr()).
 * Drop HPETClass.
 * Fix a memory leak issue by not destroying the timer instance in
   HPETTimer::del_timer().


[1]: https://lore.kernel.org/qemu-devel/20250125125137.1223277-1-zhao1.liu@intel.com/
[2]: https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/software-developers-hpet-spec-1-0a.pdf

Thanks and Best Rgards,
Zhao
---
Zhao Liu (10):
  i386/fw_cfg: move hpet_cfg definition to hpet.c
  rust/qdev: add the macro to define bit property
  rust/irq: Add a helper to convert [InterruptSource] to pointer
  rust: add bindings for gpio_{in|out} initialization
  rust: add bindings for memattrs
  rust: add bindings for timer
  rust/timer/hpet: define hpet_fw_cfg
  rust/timer/hpet: add basic HPET timer and HPETState
  rust/timer/hpet: add qom and qdev APIs support
  i386: enable rust hpet for pc when rust is enabled

 configs/devices/i386-softmmu/default.mak |   1 +
 hw/i386/fw_cfg.c                         |   6 +-
 hw/i386/pc.c                             |   2 +-
 hw/timer/Kconfig                         |   6 +-
 hw/timer/hpet.c                          |  14 +-
 include/hw/timer/hpet.h                  |   2 +-
 meson.build                              |   7 +
 rust/Cargo.lock                          |   8 +
 rust/Cargo.toml                          |   1 +
 rust/hw/Kconfig                          |   1 +
 rust/hw/meson.build                      |   1 +
 rust/hw/timer/Kconfig                    |   2 +
 rust/hw/timer/hpet/Cargo.toml            |  18 +
 rust/hw/timer/hpet/meson.build           |  18 +
 rust/hw/timer/hpet/src/fw_cfg.rs         |  78 ++
 rust/hw/timer/hpet/src/hpet.rs           | 890 +++++++++++++++++++++++
 rust/hw/timer/hpet/src/lib.rs            |  18 +
 rust/hw/timer/meson.build                |   1 +
 rust/qemu-api/meson.build                |   1 +
 rust/qemu-api/src/irq.rs                 |   5 +
 rust/qemu-api/src/lib.rs                 |   1 +
 rust/qemu-api/src/memory.rs              |  16 +-
 rust/qemu-api/src/qdev.rs                |  59 +-
 rust/qemu-api/src/timer.rs               |  98 +++
 rust/qemu-api/src/zeroable.rs            |   7 +-
 rust/wrapper.h                           |   3 +
 tests/qtest/meson.build                  |   3 +-
 27 files changed, 1245 insertions(+), 22 deletions(-)
 create mode 100644 rust/hw/timer/Kconfig
 create mode 100644 rust/hw/timer/hpet/Cargo.toml
 create mode 100644 rust/hw/timer/hpet/meson.build
 create mode 100644 rust/hw/timer/hpet/src/fw_cfg.rs
 create mode 100644 rust/hw/timer/hpet/src/hpet.rs
 create mode 100644 rust/hw/timer/hpet/src/lib.rs
 create mode 100644 rust/hw/timer/meson.build
 create mode 100644 rust/qemu-api/src/timer.rs

-- 
2.34.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to hpet.c
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-13 11:25   ` Paolo Bonzini
  2025-02-10  3:00 ` [PATCH v2 02/10] rust/qdev: add the macro to define bit property Zhao Liu
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

HPET device needs to access and update hpet_cfg variable, but now it is
defined in hw/i386/fw_cfg.c and Rust code can't access it.

Move hpet_cfg definition to hpet.c (and rename it to hpet_fw_cfg). This
allows Rust HPET device implements its own global hpet_fw_cfg variable,
and will further reduce the use of unsafe C code access and calls in the
Rust HPET implementation.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/i386/fw_cfg.c        |  4 +---
 hw/timer/hpet.c         | 14 ++++++++------
 include/hw/timer/hpet.h |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index d2cb08715a21..546de63123e6 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -26,8 +26,6 @@
 #include CONFIG_DEVICES
 #include "target/i386/cpu.h"
 
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
-
 const char *fw_cfg_arch_key_name(uint16_t key)
 {
     static const struct {
@@ -153,7 +151,7 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
     PCMachineState *pcms =
         (PCMachineState *)object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE);
     if (pcms && pcms->hpet_enabled) {
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
+        fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_fw_cfg, sizeof(hpet_fw_cfg));
     }
 #endif
 
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 92b6d509edda..f2e2d83a3f67 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -40,6 +40,8 @@
 #include "qom/object.h"
 #include "trace.h"
 
+struct hpet_fw_config hpet_fw_cfg = {.count = UINT8_MAX};
+
 #define HPET_MSI_SUPPORT        0
 
 OBJECT_DECLARE_SIMPLE_TYPE(HPETState, HPET)
@@ -655,8 +657,8 @@ static void hpet_reset(DeviceState *d)
     s->hpet_counter = 0ULL;
     s->hpet_offset = 0ULL;
     s->config = 0ULL;
-    hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
-    hpet_cfg.hpet[s->hpet_id].address = sbd->mmio[0].addr;
+    hpet_fw_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
+    hpet_fw_cfg.hpet[s->hpet_id].address = sbd->mmio[0].addr;
 
     /* to document that the RTC lowers its output on reset as well */
     s->rtc_irq_level = 0;
@@ -698,17 +700,17 @@ static void hpet_realize(DeviceState *dev, Error **errp)
     if (!s->intcap) {
         warn_report("Hpet's intcap not initialized");
     }
-    if (hpet_cfg.count == UINT8_MAX) {
+    if (hpet_fw_cfg.count == UINT8_MAX) {
         /* first instance */
-        hpet_cfg.count = 0;
+        hpet_fw_cfg.count = 0;
     }
 
-    if (hpet_cfg.count == 8) {
+    if (hpet_fw_cfg.count == 8) {
         error_setg(errp, "Only 8 instances of HPET is allowed");
         return;
     }
 
-    s->hpet_id = hpet_cfg.count++;
+    s->hpet_id = hpet_fw_cfg.count++;
 
     for (i = 0; i < HPET_NUM_IRQ_ROUTES; i++) {
         sysbus_init_irq(sbd, &s->irqs[i]);
diff --git a/include/hw/timer/hpet.h b/include/hw/timer/hpet.h
index 71e8c62453d1..c2656f7f0bef 100644
--- a/include/hw/timer/hpet.h
+++ b/include/hw/timer/hpet.h
@@ -73,7 +73,7 @@ struct hpet_fw_config
     struct hpet_fw_entry hpet[8];
 } QEMU_PACKED;
 
-extern struct hpet_fw_config hpet_cfg;
+extern struct hpet_fw_config hpet_fw_cfg;
 
 #define TYPE_HPET "hpet"
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 02/10] rust/qdev: add the macro to define bit property
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to hpet.c Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 03/10] rust/irq: Add a helper to convert [InterruptSource] to pointer Zhao Liu
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

HPET device (Rust device) needs to define the bit type property.

Add a variant of define_property macro to define bit type property.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/src/qdev.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 0041c66ed0cd..28d9be723d89 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -168,6 +168,18 @@ fn class_init(dc: &mut DeviceClass) {
 
 #[macro_export]
 macro_rules! define_property {
+    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, bit = $bitnr:expr, default = $defval:expr$(,)*) => {
+        $crate::bindings::Property {
+            // use associated function syntax for type checking
+            name: ::std::ffi::CStr::as_ptr($name),
+            info: $prop,
+            offset: $crate::offset_of!($state, $field) as isize,
+            bitnr: $bitnr,
+            set_default: true,
+            defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 },
+            ..$crate::zeroable::Zeroable::ZERO
+        }
+    };
     ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, default = $defval:expr$(,)*) => {
         $crate::bindings::Property {
             // use associated function syntax for type checking
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 03/10] rust/irq: Add a helper to convert [InterruptSource] to pointer
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to hpet.c Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 02/10] rust/qdev: add the macro to define bit property Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 04/10] rust: add bindings for gpio_{in|out} initialization Zhao Liu
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

This is useful to hanlde InterruptSource slice and pass it to C
bindings.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * New commit.

Changes since Patch v1:
 * Drop `pub(crate) use crate::bindings::IRQState`.
 * Derefer `slice[0]` directly.
---
 rust/qemu-api/src/irq.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs
index 835b027d5e5a..05f617b5684a 100644
--- a/rust/qemu-api/src/irq.rs
+++ b/rust/qemu-api/src/irq.rs
@@ -83,6 +83,12 @@ pub fn set(&self, level: T) {
     pub(crate) const fn as_ptr(&self) -> *mut *mut IRQState {
         self.cell.as_ptr()
     }
+
+    #[allow(dead_code)]
+    pub(crate) fn slice_as_ptr(slice: &[Self]) -> *mut *mut IRQState {
+        assert!(!slice.is_empty());
+        slice[0].as_ptr()
+    }
 }
 
 impl Default for InterruptSource {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 04/10] rust: add bindings for gpio_{in|out} initialization
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
                   ` (2 preceding siblings ...)
  2025-02-10  3:00 ` [PATCH v2 03/10] rust/irq: Add a helper to convert [InterruptSource] to pointer Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 05/10] rust: add bindings for memattrs Zhao Liu
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

Wrap qdev_init_gpio_{in|out} as methods in DeviceMethods. And for
qdev_init_gpio_in, based on FnCall, it can support idiomatic Rust
callback without the need for C style wrapper.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * Use FnCall to support gpio in callback.
 * Place gpio_{in|out} in DeviceMethods.
 * Accept &[InterruptSource] as the parameter of gpio_out.

Changes since Patch v1:
 * Ensure the handler of input GPIO is not none.
 * Fix doc issue. No need to specify InterruptSource path in doc.
---
 rust/qemu-api/src/irq.rs  |  1 -
 rust/qemu-api/src/qdev.rs | 47 +++++++++++++++++++++++++++++++++++----
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs
index 05f617b5684a..f87cd0c7d318 100644
--- a/rust/qemu-api/src/irq.rs
+++ b/rust/qemu-api/src/irq.rs
@@ -84,7 +84,6 @@ pub(crate) const fn as_ptr(&self) -> *mut *mut IRQState {
         self.cell.as_ptr()
     }
 
-    #[allow(dead_code)]
     pub(crate) fn slice_as_ptr(slice: &[Self]) -> *mut *mut IRQState {
         assert!(!slice.is_empty());
         slice[0].as_ptr()
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 28d9be723d89..17f12ad1361c 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -6,17 +6,18 @@
 
 use std::{
     ffi::{CStr, CString},
-    os::raw::c_void,
+    os::raw::{c_int, c_void},
     ptr::NonNull,
 };
 
 pub use bindings::{Clock, ClockEvent, DeviceClass, DeviceState, Property, ResetType};
 
 use crate::{
-    bindings::{self, Error, ResettableClass},
+    bindings::{self, qdev_init_gpio_in, qdev_init_gpio_out, Error, ResettableClass},
     callbacks::FnCall,
     cell::bql_locked,
     chardev::Chardev,
+    irq::InterruptSource,
     prelude::*,
     qom::{ClassInitImpl, ObjectClass, ObjectImpl, Owned},
     vmstate::VMStateDescription,
@@ -28,8 +29,8 @@ pub trait ResettablePhasesImpl {
     /// If not None, this is called when the object enters reset. It
     /// can reset local state of the object, but it must not do anything that
     /// has a side-effect on other objects, such as raising or lowering an
-    /// [`InterruptSource`](crate::irq::InterruptSource), or reading or
-    /// writing guest memory. It takes the reset's type as argument.
+    /// [`InterruptSource`], or reading or writing guest memory. It takes the
+    /// reset's type as argument.
     const ENTER: Option<fn(&Self, ResetType)> = None;
 
     /// If not None, this is called when the object for entry into reset, once
@@ -320,6 +321,44 @@ fn prop_set_chr(&self, propname: &str, chr: &Owned<Chardev>) {
             bindings::qdev_prop_set_chr(self.as_mut_ptr(), c_propname.as_ptr(), chr.as_mut_ptr());
         }
     }
+
+    fn init_gpio_in<F: for<'a> FnCall<(&'a Self::Target, u32, u32)>>(
+        &self,
+        num_lines: u32,
+        _cb: F,
+    ) {
+        let _: () = F::ASSERT_IS_SOME;
+
+        unsafe extern "C" fn rust_irq_handler<T, F: for<'a> FnCall<(&'a T, u32, u32)>>(
+            opaque: *mut c_void,
+            line: c_int,
+            level: c_int,
+        ) {
+            // SAFETY: the opaque was passed as a reference to `T`
+            F::call((unsafe { &*(opaque.cast::<T>()) }, line as u32, level as u32))
+        }
+
+        let gpio_in_cb: unsafe extern "C" fn(*mut c_void, c_int, c_int) =
+            rust_irq_handler::<Self::Target, F>;
+
+        unsafe {
+            qdev_init_gpio_in(
+                self.as_mut_ptr::<DeviceState>(),
+                Some(gpio_in_cb),
+                num_lines as c_int,
+            );
+        }
+    }
+
+    fn init_gpio_out(&self, pins: &[InterruptSource]) {
+        unsafe {
+            qdev_init_gpio_out(
+                self.as_mut_ptr::<DeviceState>(),
+                InterruptSource::slice_as_ptr(pins),
+                pins.len() as c_int,
+            );
+        }
+    }
 }
 
 impl<R: ObjectDeref> DeviceMethods for R where R::Target: IsA<DeviceState> {}
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 05/10] rust: add bindings for memattrs
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
                   ` (3 preceding siblings ...)
  2025-02-10  3:00 ` [PATCH v2 04/10] rust: add bindings for gpio_{in|out} initialization Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-10 10:03   ` Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 06/10] rust: add bindings for timer Zhao Liu
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

The MemTxAttrs structure contains bitfield members, and bindgen is
unable to generate an equivalent macro definition for
MEMTXATTRS_UNSPECIFIED.

Therefore, manually define a global constant variable
MEMTXATTRS_UNSPECIFIED to support calls from Rust code.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * With a boolean type unspecified field, no need to add once_cell.
 * Merge memattrs binding to memory.rs.
---
 rust/qemu-api/src/memory.rs   | 16 ++++++++++++++--
 rust/qemu-api/src/zeroable.rs |  1 +
 rust/wrapper.h                |  1 +
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/src/memory.rs b/rust/qemu-api/src/memory.rs
index 963d689c27d4..fff92508c68f 100644
--- a/rust/qemu-api/src/memory.rs
+++ b/rust/qemu-api/src/memory.rs
@@ -2,7 +2,7 @@
 // Author(s): Paolo Bonzini <pbonzini@redhat.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-//! Bindings for `MemoryRegion` and `MemoryRegionOps`
+//! Bindings for `MemoryRegion`, `MemoryRegionOps` and `MemTxAttrs`
 
 use std::{
     ffi::{CStr, CString},
@@ -11,7 +11,7 @@
     ptr::addr_of,
 };
 
-pub use bindings::hwaddr;
+pub use bindings::{hwaddr, MemTxAttrs};
 
 use crate::{
     bindings::{self, device_endian, memory_region_init_io},
@@ -189,3 +189,15 @@ unsafe impl ObjectType for MemoryRegion {
         unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_MEMORY_REGION) };
 }
 qom_isa!(MemoryRegion: Object);
+
+/// A special `MemTxAttrs` constant, used to indicate that no memary
+/// attributes are specified.
+///
+/// Bus masters which don't specify any attributes will get this,
+/// which has all attribute bits clear except the topmost one
+/// (so that we can distinguish "all attributes deliberately clear"
+/// from "didn't specify" if necessary).
+pub const MEMTXATTRS_UNSPECIFIED: MemTxAttrs = MemTxAttrs {
+    unspecified: true,
+    ..Zeroable::ZERO
+};
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index 304ad6698360..53dc90da31a1 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -103,3 +103,4 @@ fn default() -> Self {
 impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_1);
 impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_2);
 impl_zeroable!(crate::bindings::MemoryRegionOps);
+impl_zeroable!(crate::bindings::MemTxAttrs);
diff --git a/rust/wrapper.h b/rust/wrapper.h
index a9bc67af0d5f..54839ce0f510 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -62,3 +62,4 @@ typedef enum memory_order {
 #include "qapi/error.h"
 #include "migration/vmstate.h"
 #include "chardev/char-serial.h"
+#include "exec/memattrs.h"
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 06/10] rust: add bindings for timer
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
                   ` (4 preceding siblings ...)
  2025-02-10  3:00 ` [PATCH v2 05/10] rust: add bindings for memattrs Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 07/10] rust/timer/hpet: define hpet_fw_cfg Zhao Liu
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

Add timer bindings to help handle idiomatic Rust callbacks.

Additionally, wrap QEMUClockType in ClockType binding to avoid unsafe
calls in device code.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * Use FnCall to support timer callback.
 * Only add timer_init_full() binding. timer_new() is unnecessary since
   device (HPET) could create and allocate QEMUTimer.
 * Implement Drop for QEMUTimer.
 * Add ClockType binding.

Changes since Patch v1:
 * Use Timer/TimerListGroup in Rust code instead of
   QEMUTimer/QEMUTimerListGroup.
 * Rename timer_init_full()/timer_mod() to init_full()/modify().
 * Ensure the callback passed to Timer::init() is not none.
 * Add as_mut_ptr()/delete() method for Timer.
 * Make ClockType's `id` field private, as well as make Timer::init()
   accept ClockType instead of QEMUClockType.
 * Drop used qemu_clock_get_virtual_ns().
 * Add MS/US/NS wrappers.
---
 meson.build                |  7 +++
 rust/qemu-api/meson.build  |  1 +
 rust/qemu-api/src/lib.rs   |  1 +
 rust/qemu-api/src/timer.rs | 98 ++++++++++++++++++++++++++++++++++++++
 rust/wrapper.h             |  1 +
 5 files changed, 108 insertions(+)
 create mode 100644 rust/qemu-api/src/timer.rs

diff --git a/meson.build b/meson.build
index 32abefb7c48c..20192347d4d6 100644
--- a/meson.build
+++ b/meson.build
@@ -4086,6 +4086,13 @@ if have_rust
   foreach enum : c_bitfields
     bindgen_args += ['--bitfield-enum', enum]
   endforeach
+  c_nocopy = [
+    'QEMUTimer',
+  ]
+  # Used to customize Drop trait
+  foreach struct : c_nocopy
+    bindgen_args += ['--no-copy', struct]
+  endforeach
 
   # TODO: Remove this comment when the clang/libclang mismatch issue is solved.
   #
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 45e30324b295..2e9c1078b9b2 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -30,6 +30,7 @@ _qemu_api_rs = static_library(
       'src/qdev.rs',
       'src/qom.rs',
       'src/sysbus.rs',
+      'src/timer.rs',
       'src/vmstate.rs',
       'src/zeroable.rs',
     ],
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 1d7112445e24..047511d36487 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -25,6 +25,7 @@
 pub mod qdev;
 pub mod qom;
 pub mod sysbus;
+pub mod timer;
 pub mod vmstate;
 pub mod zeroable;
 
diff --git a/rust/qemu-api/src/timer.rs b/rust/qemu-api/src/timer.rs
new file mode 100644
index 000000000000..05a5061b6dda
--- /dev/null
+++ b/rust/qemu-api/src/timer.rs
@@ -0,0 +1,98 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::os::raw::{c_int, c_void};
+
+use crate::{
+    bindings::{self, qemu_clock_get_ns, timer_del, timer_init_full, timer_mod, QEMUClockType},
+    callbacks::FnCall,
+};
+
+pub type Timer = bindings::QEMUTimer;
+pub type TimerListGroup = bindings::QEMUTimerListGroup;
+
+impl Timer {
+    pub fn new() -> Self {
+        Default::default()
+    }
+
+    const fn as_mut_ptr(&self) -> *mut Self {
+        self as *const Timer as *mut _
+    }
+
+    pub fn init_full<'timer, 'opaque: 'timer, T, F>(
+        &'timer mut self,
+        timer_list_group: Option<&TimerListGroup>,
+        clk_type: ClockType,
+        scale: u32,
+        attributes: u32,
+        _cb: F,
+        opaque: &'opaque T,
+    ) where
+        F: for<'a> FnCall<(&'a T,)>,
+    {
+        let _: () = F::ASSERT_IS_SOME;
+
+        /// timer expiration callback
+        unsafe extern "C" fn rust_timer_handler<T, F: for<'a> FnCall<(&'a T,)>>(
+            opaque: *mut c_void,
+        ) {
+            // SAFETY: the opaque was passed as a reference to `T`.
+            F::call((unsafe { &*(opaque.cast::<T>()) },))
+        }
+
+        let timer_cb: unsafe extern "C" fn(*mut c_void) = rust_timer_handler::<T, F>;
+
+        // SAFETY: the opaque outlives the timer
+        unsafe {
+            timer_init_full(
+                self,
+                if let Some(g) = timer_list_group {
+                    g as *const TimerListGroup as *mut _
+                } else {
+                    ::core::ptr::null_mut()
+                },
+                clk_type.id,
+                scale as c_int,
+                attributes as c_int,
+                Some(timer_cb),
+                (opaque as *const T).cast::<c_void>() as *mut c_void,
+            )
+        }
+    }
+
+    pub fn modify(&self, expire_time: u64) {
+        unsafe { timer_mod(self.as_mut_ptr(), expire_time as i64) }
+    }
+
+    pub fn delete(&self) {
+        unsafe { timer_del(self.as_mut_ptr()) }
+    }
+}
+
+impl Drop for Timer {
+    fn drop(&mut self) {
+        self.delete()
+    }
+}
+
+pub struct ClockType {
+    id: QEMUClockType,
+}
+
+impl ClockType {
+    pub fn get_ns(&self) -> u64 {
+        // SAFETY: cannot be created outside this module, therefore id
+        // is valid
+        (unsafe { qemu_clock_get_ns(self.id) }) as u64
+    }
+}
+
+pub const CLOCK_VIRTUAL: ClockType = ClockType {
+    id: QEMUClockType::QEMU_CLOCK_VIRTUAL,
+};
+
+pub const MS: u32 = bindings::SCALE_MS;
+pub const US: u32 = bindings::SCALE_US;
+pub const NS: u32 = bindings::SCALE_NS;
diff --git a/rust/wrapper.h b/rust/wrapper.h
index 54839ce0f510..a35bfbd1760d 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -63,3 +63,4 @@ typedef enum memory_order {
 #include "migration/vmstate.h"
 #include "chardev/char-serial.h"
 #include "exec/memattrs.h"
+#include "qemu/timer.h"
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 07/10] rust/timer/hpet: define hpet_fw_cfg
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
                   ` (5 preceding siblings ...)
  2025-02-10  3:00 ` [PATCH v2 06/10] rust: add bindings for timer Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 08/10] rust/timer/hpet: add basic HPET timer and HPETState Zhao Liu
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

Define HPETFwEntry structure with the same memory layout as
hpet_fw_entry in C.

Further, define the global hpet_cfg variable in Rust which is the
same as the C version. This hpet_cfg variable in Rust will replace
the C version one and allows both Rust code and C code to access it.

The Rust version of hpet_cfg is self-contained, avoiding unsafe
access to C code.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * hpet_fw_cfg doesn't need a BqlRefCell.

Changes since Patch v1:
 * Add rust_version in Cargo.toml.
 * Fix clippy's complaints about mut references to static mut.
 * Enable workspace's lint configurations for hpet.
 * Make update_hpet_cfg()'s parameters not in Option<>.
---
 rust/Cargo.lock                  |  8 ++++
 rust/Cargo.toml                  |  1 +
 rust/hw/meson.build              |  1 +
 rust/hw/timer/hpet/Cargo.toml    | 18 +++++++
 rust/hw/timer/hpet/meson.build   | 18 +++++++
 rust/hw/timer/hpet/src/fw_cfg.rs | 80 ++++++++++++++++++++++++++++++++
 rust/hw/timer/hpet/src/lib.rs    | 13 ++++++
 rust/hw/timer/meson.build        |  1 +
 rust/qemu-api/src/zeroable.rs    |  6 ++-
 9 files changed, 144 insertions(+), 2 deletions(-)
 create mode 100644 rust/hw/timer/hpet/Cargo.toml
 create mode 100644 rust/hw/timer/hpet/meson.build
 create mode 100644 rust/hw/timer/hpet/src/fw_cfg.rs
 create mode 100644 rust/hw/timer/hpet/src/lib.rs
 create mode 100644 rust/hw/timer/meson.build

diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index c0c6069247a8..79e142723b83 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -37,6 +37,14 @@ version = "1.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b"
 
+[[package]]
+name = "hpet"
+version = "0.1.0"
+dependencies = [
+ "qemu_api",
+ "qemu_api_macros",
+]
+
 [[package]]
 name = "itertools"
 version = "0.11.0"
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 5b6b6ca43825..9265d30f46a2 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -4,6 +4,7 @@ members = [
     "qemu-api-macros",
     "qemu-api",
     "hw/char/pl011",
+    "hw/timer/hpet",
 ]
 
 [workspace.lints.rust]
diff --git a/rust/hw/meson.build b/rust/hw/meson.build
index 860196645e71..9749d4adfc96 100644
--- a/rust/hw/meson.build
+++ b/rust/hw/meson.build
@@ -1 +1,2 @@
 subdir('char')
+subdir('timer')
diff --git a/rust/hw/timer/hpet/Cargo.toml b/rust/hw/timer/hpet/Cargo.toml
new file mode 100644
index 000000000000..147f216e7257
--- /dev/null
+++ b/rust/hw/timer/hpet/Cargo.toml
@@ -0,0 +1,18 @@
+[package]
+name = "hpet"
+version = "0.1.0"
+edition = "2021"
+authors = ["Zhao Liu <zhao1.liu@intel.com>"]
+license = "GPL-2.0-or-later"
+description = "IA-PC High Precision Event Timer emulation in Rust"
+rust-version = "1.63.0"
+
+[lib]
+crate-type = ["staticlib"]
+
+[dependencies]
+qemu_api = { path = "../../../qemu-api" }
+qemu_api_macros = { path = "../../../qemu-api-macros" }
+
+[lints]
+workspace = true
diff --git a/rust/hw/timer/hpet/meson.build b/rust/hw/timer/hpet/meson.build
new file mode 100644
index 000000000000..c2d7c0532ca4
--- /dev/null
+++ b/rust/hw/timer/hpet/meson.build
@@ -0,0 +1,18 @@
+_libhpet_rs = static_library(
+  'hpet',
+  files('src/lib.rs'),
+  override_options: ['rust_std=2021', 'build.rust_std=2021'],
+  rust_abi: 'rust',
+  dependencies: [
+    qemu_api,
+    qemu_api_macros,
+  ],
+)
+
+rust_devices_ss.add(when: 'CONFIG_X_HPET_RUST', if_true: [declare_dependency(
+  link_whole: [_libhpet_rs],
+  # Putting proc macro crates in `dependencies` is necessary for Meson to find
+  # them when compiling the root per-target static rust lib.
+  dependencies: [qemu_api_macros],
+  variables: {'crate': 'hpet'},
+)])
diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs
new file mode 100644
index 000000000000..d4c24813063d
--- /dev/null
+++ b/rust/hw/timer/hpet/src/fw_cfg.rs
@@ -0,0 +1,80 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#![allow(dead_code)]
+
+use std::ptr::addr_of_mut;
+
+use qemu_api::{cell::bql_locked, impl_zeroable, zeroable::Zeroable};
+
+/// Each HPETState represents a Event Timer Block. The v1 spec supports
+/// up to 8 blocks. QEMU only uses 1 block (in PC machine).
+const HPET_MAX_NUM_EVENT_TIMER_BLOCK: usize = 8;
+
+#[repr(C, packed)]
+#[derive(Copy, Clone, Default)]
+pub struct HPETFwEntry {
+    pub event_timer_block_id: u32,
+    pub address: u64,
+    pub min_tick: u16,
+    pub page_prot: u8,
+}
+
+unsafe impl Zeroable for HPETFwEntry {
+    const ZERO: Self = Self {
+        event_timer_block_id: 0,
+        address: 0,
+        min_tick: 0,
+        page_prot: 0,
+    };
+}
+
+#[repr(C, packed)]
+#[derive(Copy, Clone, Default)]
+pub struct HPETFwConfig {
+    pub count: u8,
+    pub hpet: [HPETFwEntry; HPET_MAX_NUM_EVENT_TIMER_BLOCK],
+}
+
+impl_zeroable!(HPETFwConfig);
+
+#[allow(non_upper_case_globals)]
+#[no_mangle]
+pub static mut hpet_fw_cfg: HPETFwConfig = HPETFwConfig {
+    count: u8::MAX,
+    ..Zeroable::ZERO
+};
+
+impl HPETFwConfig {
+    pub(crate) fn assign_hpet_id() -> usize {
+        assert!(bql_locked());
+        // SAFETY: all accesses go through these methods, which guarantee
+        // that the accesses are protected by the BQL.
+        let mut fw_cfg = unsafe { *addr_of_mut!(hpet_fw_cfg) };
+
+        if fw_cfg.count == u8::MAX {
+            // first instance
+            fw_cfg.count = 0;
+        }
+
+        if fw_cfg.count == 8 {
+            // TODO: Add error binding: error_setg()
+            panic!("Only 8 instances of HPET is allowed");
+        }
+
+        let id: usize = fw_cfg.count.into();
+        fw_cfg.count += 1;
+        id
+    }
+
+    pub(crate) fn update_hpet_cfg(hpet_id: usize, timer_block_id: u32, address: u64) {
+        assert!(bql_locked());
+        // SAFETY: all accesses go through these methods, which guarantee
+        // that the accesses are protected by the BQL.
+        let mut fw_cfg = unsafe { *addr_of_mut!(hpet_fw_cfg) };
+
+        fw_cfg.hpet[hpet_id].event_timer_block_id = timer_block_id;
+        fw_cfg.hpet[hpet_id].address = address;
+    }
+}
diff --git a/rust/hw/timer/hpet/src/lib.rs b/rust/hw/timer/hpet/src/lib.rs
new file mode 100644
index 000000000000..c2b9c64d0bfe
--- /dev/null
+++ b/rust/hw/timer/hpet/src/lib.rs
@@ -0,0 +1,13 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// HPET QEMU Device Model
+//
+// This library implements a device model for the IA-PC HPET (High
+// Precision Event Timers) device in QEMU.
+//
+
+#![deny(unsafe_op_in_unsafe_fn)]
+
+pub mod fw_cfg;
diff --git a/rust/hw/timer/meson.build b/rust/hw/timer/meson.build
new file mode 100644
index 000000000000..22a84f15536b
--- /dev/null
+++ b/rust/hw/timer/meson.build
@@ -0,0 +1 @@
+subdir('hpet')
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index 53dc90da31a1..a2356cb2f24c 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -58,6 +58,7 @@ pub unsafe trait Zeroable: Default {
 /// ## Differences with `core::mem::zeroed`
 ///
 /// `const_zero` zeroes padding bits, while `core::mem::zeroed` doesn't
+#[macro_export]
 macro_rules! const_zero {
     // This macro to produce a type-generic zero constant is taken from the
     // const_zero crate (v0.1.1):
@@ -79,10 +80,11 @@ union TypeAsBytes {
 }
 
 /// A wrapper to implement the `Zeroable` trait through the `const_zero` macro.
+#[macro_export]
 macro_rules! impl_zeroable {
     ($type:ty) => {
-        unsafe impl Zeroable for $type {
-            const ZERO: Self = unsafe { const_zero!($type) };
+        unsafe impl $crate::zeroable::Zeroable for $type {
+            const ZERO: Self = unsafe { $crate::const_zero!($type) };
         }
     };
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 08/10] rust/timer/hpet: add basic HPET timer and HPETState
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
                   ` (6 preceding siblings ...)
  2025-02-10  3:00 ` [PATCH v2 07/10] rust/timer/hpet: define hpet_fw_cfg Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 09/10] rust/timer/hpet: add qom and qdev APIs support Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 10/10] i386: enable rust hpet for pc when rust is enabled Zhao Liu
  9 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

Add the HPETTimer and HPETState (HPET timer block), along with their
basic methods and register definitions.

This is in preparation for supporting the QAPI interfaces.

Note, wrap all items in HPETState that may be changed in the callback
called by C code into the BqlCell/BqlRefCell.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * Don't wrap HPETState.flags in a BqlCell.
 * Consolidate inmutable &self and QOM casting.
 * Prefer timer iterator over loop.
 * Use BqlRefCell<HPETTimer> directly to store timers in HPETState.

Changes since Patch v1:
 * Fix complaints from cargo (+nightly) doc/clippy/fmt (v1.84.0).
 * Fix a memory leak issue by not destroying the timer instance in
   HPETTimer::del_timer().
---
 rust/hw/timer/hpet/src/hpet.rs | 629 +++++++++++++++++++++++++++++++++
 rust/hw/timer/hpet/src/lib.rs  |   1 +
 rust/wrapper.h                 |   1 +
 3 files changed, 631 insertions(+)
 create mode 100644 rust/hw/timer/hpet/src/hpet.rs

diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
new file mode 100644
index 000000000000..7d10c4205348
--- /dev/null
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -0,0 +1,629 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#![allow(dead_code)]
+
+use std::ptr::{addr_of_mut, null_mut, NonNull};
+
+use qemu_api::{
+    bindings::{address_space_memory, address_space_stl_le},
+    cell::{BqlCell, BqlRefCell},
+    irq::InterruptSource,
+    memory::{MemoryRegion, MEMTXATTRS_UNSPECIFIED},
+    prelude::*,
+    qom::ParentField,
+    sysbus::SysBusDevice,
+    timer::{Timer, CLOCK_VIRTUAL, NS},
+};
+
+/// Register space for each timer block (`HPET_BASE` is defined in hpet.h).
+const HPET_REG_SPACE_LEN: u64 = 0x400; // 1024 bytes
+
+/// Minimum recommended hardware implementation.
+const HPET_MIN_TIMERS: usize = 3;
+/// Maximum timers in each timer block.
+const HPET_MAX_TIMERS: usize = 32;
+
+/// Flags that HPETState.flags supports.
+const HPET_FLAG_MSI_SUPPORT_SHIFT: usize = 0;
+
+const HPET_NUM_IRQ_ROUTES: usize = 32;
+const HPET_LEGACY_PIT_INT: u32 = 0; // HPET_LEGACY_RTC_INT isn't defined here.
+const RTC_ISA_IRQ: usize = 8;
+
+const HPET_CLK_PERIOD: u64 = 10; // 10 ns
+const FS_PER_NS: u64 = 1000000; // 1000000 femtoseconds == 1 ns
+
+/// General Capabilities and ID Register
+const HPET_CAP_REG: u64 = 0x000;
+/// Revision ID (bits 0:7). Revision 1 is implemented (refer to v1.0a spec).
+const HPET_CAP_REV_ID_VALUE: u64 = 0x1;
+const HPET_CAP_REV_ID_SHIFT: usize = 0;
+/// Number of Timers (bits 8:12)
+const HPET_CAP_NUM_TIM_SHIFT: usize = 8;
+/// Counter Size (bit 13)
+const HPET_CAP_COUNT_SIZE_CAP_SHIFT: usize = 13;
+/// Legacy Replacement Route Capable (bit 15)
+const HPET_CAP_LEG_RT_CAP_SHIFT: usize = 15;
+/// Vendor ID (bits 16:31)
+const HPET_CAP_VENDER_ID_VALUE: u64 = 0x8086;
+const HPET_CAP_VENDER_ID_SHIFT: usize = 16;
+/// Main Counter Tick Period (bits 32:63)
+const HPET_CAP_CNT_CLK_PERIOD_SHIFT: usize = 32;
+
+/// General Configuration Register
+const HPET_CFG_REG: u64 = 0x010;
+/// Overall Enable (bit 0)
+const HPET_CFG_ENABLE_SHIFT: usize = 0;
+/// Legacy Replacement Route (bit 1)
+const HPET_CFG_LEG_RT_SHIFT: usize = 1;
+/// Other bits are reserved.
+const HPET_CFG_WRITE_MASK: u64 = 0x003;
+
+/// General Interrupt Status Register
+const HPET_INT_STATUS_REG: u64 = 0x020;
+
+/// Main Counter Value Register
+const HPET_COUNTER_REG: u64 = 0x0f0;
+
+/// Timer N Configuration and Capability Register (masked by 0x18)
+const HPET_TN_CFG_REG: u64 = 0x000;
+/// bit 0, 7, and bits 16:31 are reserved.
+/// bit 4, 5, 15, and bits 32:64 are read-only.
+const HPET_TN_CFG_WRITE_MASK: u64 = 0x7f4e;
+/// Timer N Interrupt Type (bit 1)
+const HPET_TN_CFG_INT_TYPE_SHIFT: usize = 1;
+/// Timer N Interrupt Enable (bit 2)
+const HPET_TN_CFG_INT_ENABLE_SHIFT: usize = 2;
+/// Timer N Type (Periodic enabled or not, bit 3)
+const HPET_TN_CFG_PERIODIC_SHIFT: usize = 3;
+/// Timer N Periodic Interrupt Capable (support Periodic or not, bit 4)
+const HPET_TN_CFG_PERIODIC_CAP_SHIFT: usize = 4;
+/// Timer N Size (timer size is 64-bits or 32 bits, bit 5)
+const HPET_TN_CFG_SIZE_CAP_SHIFT: usize = 5;
+/// Timer N Value Set (bit 6)
+const HPET_TN_CFG_SETVAL_SHIFT: usize = 6;
+/// Timer N 32-bit Mode (bit 8)
+const HPET_TN_CFG_32BIT_SHIFT: usize = 8;
+/// Timer N Interrupt Rout (bits 9:13)
+const HPET_TN_CFG_INT_ROUTE_MASK: u64 = 0x3e00;
+const HPET_TN_CFG_INT_ROUTE_SHIFT: usize = 9;
+/// Timer N FSB Interrupt Enable (bit 14)
+const HPET_TN_CFG_FSB_ENABLE_SHIFT: usize = 14;
+/// Timer N FSB Interrupt Delivery (bit 15)
+const HPET_TN_CFG_FSB_CAP_SHIFT: usize = 15;
+/// Timer N Interrupt Routing Capability (bits 32:63)
+const HPET_TN_CFG_INT_ROUTE_CAP_SHIFT: usize = 32;
+
+/// Timer N Comparator Value Register (masked by 0x18)
+const HPET_TN_CMP_REG: u64 = 0x008;
+
+/// Timer N FSB Interrupt Route Register (masked by 0x18)
+const HPET_TN_FSB_ROUTE_REG: u64 = 0x010;
+
+const fn hpet_next_wrap(cur_tick: u64) -> u64 {
+    (cur_tick | 0xffffffff) + 1
+}
+
+const fn hpet_time_after(a: u64, b: u64) -> bool {
+    ((b - a) as i64) < 0
+}
+
+const fn ticks_to_ns(value: u64) -> u64 {
+    value * HPET_CLK_PERIOD
+}
+
+const fn ns_to_ticks(value: u64) -> u64 {
+    value / HPET_CLK_PERIOD
+}
+
+// Avoid touching the bits that cannot be written.
+const fn hpet_fixup_reg(new: u64, old: u64, mask: u64) -> u64 {
+    (new & mask) | (old & !mask)
+}
+
+const fn activating_bit(old: u64, new: u64, shift: usize) -> bool {
+    let mask: u64 = 1 << shift;
+    (old & mask == 0) && (new & mask != 0)
+}
+
+const fn deactivating_bit(old: u64, new: u64, shift: usize) -> bool {
+    let mask: u64 = 1 << shift;
+    (old & mask != 0) && (new & mask == 0)
+}
+
+fn timer_handler(timer_cell: &BqlRefCell<HPETTimer>) {
+    timer_cell.borrow_mut().callback()
+}
+
+/// HPET Timer Abstraction
+#[repr(C)]
+#[derive(Debug, Default)]
+#[cfg_attr(has_offset_of, derive(qemu_api_macros::offsets))]
+pub struct HPETTimer {
+    /// timer N index within the timer block (`HPETState`)
+    #[doc(alias = "tn")]
+    index: usize,
+    qemu_timer: Option<Box<Timer>>,
+    /// timer block abstraction containing this timer
+    state: Option<NonNull<HPETState>>,
+
+    // Memory-mapped, software visible timer registers
+    /// Timer N Configuration and Capability Register
+    config: u64,
+    /// Timer N Comparator Value Register
+    cmp: u64,
+    /// Timer N FSB Interrupt Route Register
+    fsb: u64,
+
+    // Hidden register state
+    /// comparator (extended to counter width)
+    cmp64: u64,
+    /// Last value written to comparator
+    period: u64,
+    /// timer pop will indicate wrap for one-shot 32-bit
+    /// mode. Next pop will be actual timer expiration.
+    wrap_flag: u8,
+    /// last value armed, to avoid timer storms
+    last: u64,
+}
+
+impl HPETTimer {
+    fn init(&mut self, index: usize, state_ptr: *mut HPETState) -> &mut Self {
+        *self = HPETTimer::default();
+        self.index = index;
+        self.state = NonNull::new(state_ptr);
+        self
+    }
+
+    fn init_timer_with_state(&mut self) {
+        self.qemu_timer = Some(Box::new({
+            let mut t = Timer::new();
+            t.init_full(
+                None,
+                CLOCK_VIRTUAL,
+                NS,
+                0,
+                timer_handler,
+                &self.get_state().timers[self.index],
+            );
+            t
+        }));
+    }
+
+    fn get_state(&self) -> &HPETState {
+        // SAFETY:
+        // the pointer is convertible to a reference
+        unsafe { self.state.unwrap().as_ref() }
+    }
+
+    fn is_int_active(&self) -> bool {
+        self.get_state().is_timer_int_active(self.index)
+    }
+
+    const fn is_fsb_route_enabled(&self) -> bool {
+        self.config & (1 << HPET_TN_CFG_FSB_ENABLE_SHIFT) != 0
+    }
+
+    const fn is_periodic(&self) -> bool {
+        self.config & (1 << HPET_TN_CFG_PERIODIC_SHIFT) != 0
+    }
+
+    const fn is_int_enabled(&self) -> bool {
+        self.config & (1 << HPET_TN_CFG_INT_ENABLE_SHIFT) != 0
+    }
+
+    const fn is_32bit_mod(&self) -> bool {
+        self.config & (1 << HPET_TN_CFG_32BIT_SHIFT) != 0
+    }
+
+    const fn is_valset_enabled(&self) -> bool {
+        self.config & (1 << HPET_TN_CFG_SETVAL_SHIFT) != 0
+    }
+
+    fn clear_valset(&mut self) {
+        self.config &= !(1 << HPET_TN_CFG_SETVAL_SHIFT);
+    }
+
+    /// True if timer interrupt is level triggered; otherwise, edge triggered.
+    const fn is_int_level_triggered(&self) -> bool {
+        self.config & (1 << HPET_TN_CFG_INT_TYPE_SHIFT) != 0
+    }
+
+    /// calculate next value of the general counter that matches the
+    /// target (either entirely, or the low 32-bit only depending on
+    /// the timer mode).
+    fn calculate_cmp64(&self, cur_tick: u64, target: u64) -> u64 {
+        if self.is_32bit_mod() {
+            let mut result: u64 = cur_tick.deposit(0, 32, target);
+            if result < cur_tick {
+                result += 0x100000000;
+            }
+            result
+        } else {
+            target
+        }
+    }
+
+    const fn get_individual_route(&self) -> usize {
+        ((self.config & HPET_TN_CFG_INT_ROUTE_MASK) >> HPET_TN_CFG_INT_ROUTE_SHIFT) as usize
+    }
+
+    fn get_int_route(&self) -> usize {
+        if self.index <= 1 && self.get_state().is_legacy_mode() {
+            // If LegacyReplacement Route bit is set, HPET specification requires
+            // timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
+            // timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC.
+            //
+            // If the LegacyReplacement Route bit is set, the individual routing
+            // bits for timers 0 and 1 (APIC or FSB) will have no impact.
+            //
+            // FIXME: Consider I/O APIC case.
+            if self.index == 0 {
+                0
+            } else {
+                RTC_ISA_IRQ
+            }
+        } else {
+            // (If the LegacyReplacement Route bit is set) Timer 2-n will be
+            // routed as per the routing in the timer n config registers.
+            // ...
+            // If the LegacyReplacement Route bit is not set, the individual
+            // routing bits for each of the timers are used.
+            self.get_individual_route()
+        }
+    }
+
+    fn set_irq(&mut self, set: bool) {
+        let route = self.get_int_route();
+
+        if set && self.is_int_enabled() && self.get_state().is_hpet_enabled() {
+            if self.is_fsb_route_enabled() {
+                // SAFETY:
+                // the parameters are valid.
+                unsafe {
+                    address_space_stl_le(
+                        addr_of_mut!(address_space_memory),
+                        self.fsb >> 32,  // Timer N FSB int addr
+                        self.fsb as u32, // Timer N FSB int value, truncate!
+                        MEMTXATTRS_UNSPECIFIED,
+                        null_mut(),
+                    );
+                }
+            } else if self.is_int_level_triggered() {
+                self.get_state().irqs[route].raise();
+            } else {
+                self.get_state().irqs[route].pulse();
+            }
+        } else if !self.is_fsb_route_enabled() {
+            self.get_state().irqs[route].lower();
+        }
+    }
+
+    fn update_irq(&mut self, set: bool) {
+        // If Timer N Interrupt Enable bit is 0, "the timer will
+        // still operate and generate appropriate status bits, but
+        // will not cause an interrupt"
+        self.get_state()
+            .update_int_status(self.index as u32, set && self.is_int_level_triggered());
+        self.set_irq(set);
+    }
+
+    fn arm_timer(&mut self, tick: u64) {
+        let mut ns = self.get_state().get_ns(tick);
+
+        // Clamp period to reasonable min value (1 us)
+        if self.is_periodic() && ns - self.last < 1000 {
+            ns = self.last + 1000;
+        }
+
+        self.last = ns;
+        self.qemu_timer.as_ref().unwrap().modify(self.last);
+    }
+
+    fn set_timer(&mut self) {
+        let cur_tick: u64 = self.get_state().get_ticks();
+
+        self.wrap_flag = 0;
+        self.cmp64 = self.calculate_cmp64(cur_tick, self.cmp);
+        if self.is_32bit_mod() {
+            // HPET spec says in one-shot 32-bit mode, generate an interrupt when
+            // counter wraps in addition to an interrupt with comparator match.
+            if !self.is_periodic() && self.cmp64 > hpet_next_wrap(cur_tick) {
+                self.wrap_flag = 1;
+                self.arm_timer(hpet_next_wrap(cur_tick));
+                return;
+            }
+        }
+        self.arm_timer(self.cmp64);
+    }
+
+    fn del_timer(&mut self) {
+        // Just remove the timer from the timer_list without destroying
+        // this timer instance.
+        self.qemu_timer.as_ref().unwrap().delete();
+
+        if self.is_int_active() {
+            // For level-triggered interrupt, this leaves interrupt status
+            // register set but lowers irq.
+            self.update_irq(true);
+        }
+    }
+
+    /// Configuration and Capability Register
+    fn set_tn_cfg_reg(&mut self, shift: u32, len: u32, val: u64) {
+        // TODO: Add trace point - trace_hpet_ram_write_tn_cfg(addr & 4)
+        let old_val: u64 = self.config;
+        let mut new_val: u64 = old_val.deposit(shift, len, val);
+        new_val = hpet_fixup_reg(new_val, old_val, HPET_TN_CFG_WRITE_MASK);
+
+        // Switch level-type interrupt to edge-type.
+        if deactivating_bit(old_val, new_val, HPET_TN_CFG_INT_TYPE_SHIFT) {
+            // Do this before changing timer.config; otherwise, if
+            // HPET_TN_FSB is set, update_irq will not lower the qemu_irq.
+            self.update_irq(false);
+        }
+
+        self.config = new_val;
+
+        if activating_bit(old_val, new_val, HPET_TN_CFG_INT_ENABLE_SHIFT) && self.is_int_active() {
+            self.update_irq(true);
+        }
+
+        if self.is_32bit_mod() {
+            self.cmp = u64::from(self.cmp as u32); // truncate!
+            self.period = u64::from(self.period as u32); // truncate!
+        }
+
+        if self.get_state().is_hpet_enabled() {
+            self.set_timer();
+        }
+    }
+
+    /// Comparator Value Register
+    fn set_tn_cmp_reg(&mut self, shift: u32, len: u32, val: u64) {
+        let mut length = len;
+        let mut value = val;
+
+        // TODO: Add trace point - trace_hpet_ram_write_tn_cmp(addr & 4)
+        if self.is_32bit_mod() {
+            // High 32-bits are zero, leave them untouched.
+            if shift != 0 {
+                // TODO: Add trace point - trace_hpet_ram_write_invalid_tn_cmp()
+                return;
+            }
+            length = 64;
+            value = u64::from(value as u32); // truncate!
+        }
+
+        if !self.is_periodic() || self.is_valset_enabled() {
+            self.cmp = self.cmp.deposit(shift, length, value);
+        }
+
+        if self.is_periodic() {
+            self.period = self.period.deposit(shift, length, value);
+        }
+
+        self.clear_valset();
+        if self.get_state().is_hpet_enabled() {
+            self.set_timer();
+        }
+    }
+
+    /// FSB Interrupt Route Register
+    fn set_tn_fsb_route_reg(&mut self, shift: u32, len: u32, val: u64) {
+        self.fsb = self.fsb.deposit(shift, len, val);
+    }
+
+    fn reset(&mut self) {
+        self.del_timer();
+        self.cmp = u64::MAX; // Comparator Match Registers reset to all 1's.
+        self.config = (1 << HPET_TN_CFG_PERIODIC_CAP_SHIFT) | (1 << HPET_TN_CFG_SIZE_CAP_SHIFT);
+        if self.get_state().has_msi_flag() {
+            self.config |= 1 << HPET_TN_CFG_FSB_CAP_SHIFT;
+        }
+        // advertise availability of ioapic int
+        self.config |=
+            (u64::from(self.get_state().int_route_cap)) << HPET_TN_CFG_INT_ROUTE_CAP_SHIFT;
+        self.period = 0;
+        self.wrap_flag = 0;
+    }
+
+    /// timer expiration callback
+    fn callback(&mut self) {
+        let period: u64 = self.period;
+        let cur_tick: u64 = self.get_state().get_ticks();
+
+        if self.is_periodic() && period != 0 {
+            while hpet_time_after(cur_tick, self.cmp64) {
+                self.cmp64 += period;
+            }
+            if self.is_32bit_mod() {
+                self.cmp = u64::from(self.cmp64 as u32); // truncate!
+            } else {
+                self.cmp = self.cmp64;
+            }
+            self.arm_timer(self.cmp64);
+        } else if self.wrap_flag != 0 {
+            self.wrap_flag = 0;
+            self.arm_timer(self.cmp64);
+        }
+        self.update_irq(true);
+    }
+}
+
+/// HPET Event Timer Block Abstraction
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+pub struct HPETState {
+    parent_obj: ParentField<SysBusDevice>,
+    iomem: MemoryRegion,
+
+    // HPET block Registers: Memory-mapped, software visible registers
+    /// General Capabilities and ID Register
+    capability: BqlCell<u64>,
+    ///  General Configuration Register
+    config: BqlCell<u64>,
+    /// General Interrupt Status Register
+    #[doc(alias = "isr")]
+    int_status: BqlCell<u64>,
+    /// Main Counter Value Register
+    #[doc(alias = "hpet_counter")]
+    counter: BqlCell<u64>,
+
+    // Internal state
+    /// Capabilities that QEMU HPET supports.
+    /// bit 0: MSI (or FSB) support.
+    flags: u32,
+
+    /// Offset of main counter relative to qemu clock.
+    hpet_offset: BqlCell<u64>,
+    hpet_offset_saved: bool,
+
+    irqs: [InterruptSource; HPET_NUM_IRQ_ROUTES],
+    rtc_irq_level: BqlCell<u32>,
+    pit_enabled: InterruptSource,
+
+    /// Interrupt Routing Capability.
+    /// This field indicates to which interrupts in the I/O (x) APIC
+    /// the timers' interrupt can be routed, and is encoded in the
+    /// bits 32:64 of timer N's config register:
+    #[doc(alias = "intcap")]
+    int_route_cap: u32,
+
+    /// HPET timer array managed by this timer block.
+    #[doc(alias = "timer")]
+    timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS],
+    num_timers: BqlCell<usize>,
+
+    /// Instance id (HPET timer block ID).
+    hpet_id: BqlCell<usize>,
+}
+
+impl HPETState {
+    const fn has_msi_flag(&self) -> bool {
+        self.flags & (1 << HPET_FLAG_MSI_SUPPORT_SHIFT) != 0
+    }
+
+    fn is_legacy_mode(&self) -> bool {
+        self.config.get() & (1 << HPET_CFG_LEG_RT_SHIFT) != 0
+    }
+
+    fn is_hpet_enabled(&self) -> bool {
+        self.config.get() & (1 << HPET_CFG_ENABLE_SHIFT) != 0
+    }
+
+    fn is_timer_int_active(&self, index: usize) -> bool {
+        self.int_status.get() & (1 << index) != 0
+    }
+
+    fn get_ticks(&self) -> u64 {
+        ns_to_ticks(CLOCK_VIRTUAL.get_ns() + self.hpet_offset.get())
+    }
+
+    fn get_ns(&self, tick: u64) -> u64 {
+        ticks_to_ns(tick) - self.hpet_offset.get()
+    }
+
+    fn handle_legacy_irq(&self, irq: u32, level: u32) {
+        if irq == HPET_LEGACY_PIT_INT {
+            if !self.is_legacy_mode() {
+                self.irqs[0].set(level != 0);
+            }
+        } else {
+            self.rtc_irq_level.set(level);
+            if !self.is_legacy_mode() {
+                self.irqs[RTC_ISA_IRQ].set(level != 0);
+            }
+        }
+    }
+
+    fn init_timer(&self) {
+        let raw_ptr: *mut HPETState = self as *const HPETState as *mut HPETState;
+
+        for (index, timer) in self.timers.iter().enumerate() {
+            timer
+                .borrow_mut()
+                .init(index, raw_ptr)
+                .init_timer_with_state();
+        }
+    }
+
+    fn update_int_status(&self, index: u32, level: bool) {
+        self.int_status
+            .set(self.int_status.get().deposit(index, 1, u64::from(level)));
+    }
+
+    /// General Configuration Register
+    fn set_cfg_reg(&self, shift: u32, len: u32, val: u64) {
+        let old_val = self.config.get();
+        let mut new_val = old_val.deposit(shift, len, val);
+
+        new_val = hpet_fixup_reg(new_val, old_val, HPET_CFG_WRITE_MASK);
+        self.config.set(new_val);
+
+        if activating_bit(old_val, new_val, HPET_CFG_ENABLE_SHIFT) {
+            // Enable main counter and interrupt generation.
+            self.hpet_offset
+                .set(ticks_to_ns(self.counter.get()) - CLOCK_VIRTUAL.get_ns());
+
+            for timer in self.timers.iter().take(self.num_timers.get()) {
+                let mut t = timer.borrow_mut();
+
+                if t.is_int_enabled() && t.is_int_active() {
+                    t.update_irq(true);
+                }
+                t.set_timer();
+            }
+        } else if deactivating_bit(old_val, new_val, HPET_CFG_ENABLE_SHIFT) {
+            // Halt main counter and disable interrupt generation.
+            self.counter.set(self.get_ticks());
+
+            for timer in self.timers.iter().take(self.num_timers.get()) {
+                timer.borrow_mut().del_timer();
+            }
+        }
+
+        // i8254 and RTC output pins are disabled when HPET is in legacy mode
+        if activating_bit(old_val, new_val, HPET_CFG_LEG_RT_SHIFT) {
+            self.pit_enabled.set(false);
+            self.irqs[0].lower();
+            self.irqs[RTC_ISA_IRQ].lower();
+        } else if deactivating_bit(old_val, new_val, HPET_CFG_LEG_RT_SHIFT) {
+            // TODO: Add irq binding: qemu_irq_lower(s->irqs[0])
+            self.irqs[0].lower();
+            self.pit_enabled.set(true);
+            self.irqs[RTC_ISA_IRQ].set(self.rtc_irq_level.get() != 0);
+        }
+    }
+
+    /// General Interrupt Status Register: Read/Write Clear
+    fn set_int_status_reg(&self, shift: u32, _len: u32, val: u64) {
+        let new_val = val << shift;
+        let cleared = new_val & self.int_status.get();
+
+        for (index, timer) in self.timers.iter().take(self.num_timers.get()).enumerate() {
+            if cleared & (1 << index) != 0 {
+                timer.borrow_mut().update_irq(false);
+            }
+        }
+    }
+
+    /// Main Counter Value Register
+    fn set_counter_reg(&self, shift: u32, len: u32, val: u64) {
+        if self.is_hpet_enabled() {
+            // TODO: Add trace point -
+            // trace_hpet_ram_write_counter_write_while_enabled()
+            //
+            // HPET spec says that writes to this register should only be
+            // done while the counter is halted. So this is an undefined
+            // behavior. There's no need to forbid it, but when HPET is
+            // enabled, the changed counter value will not affect the
+            // tick count (i.e., the previously calculated offset will
+            // not be changed as well).
+        }
+        self.counter
+            .set(self.counter.get().deposit(shift, len, val));
+    }
+}
diff --git a/rust/hw/timer/hpet/src/lib.rs b/rust/hw/timer/hpet/src/lib.rs
index c2b9c64d0bfe..027f7f83349a 100644
--- a/rust/hw/timer/hpet/src/lib.rs
+++ b/rust/hw/timer/hpet/src/lib.rs
@@ -11,3 +11,4 @@
 #![deny(unsafe_op_in_unsafe_fn)]
 
 pub mod fw_cfg;
+pub mod hpet;
diff --git a/rust/wrapper.h b/rust/wrapper.h
index a35bfbd1760d..d927ad6799da 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -64,3 +64,4 @@ typedef enum memory_order {
 #include "chardev/char-serial.h"
 #include "exec/memattrs.h"
 #include "qemu/timer.h"
+#include "exec/address-spaces.h"
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 09/10] rust/timer/hpet: add qom and qdev APIs support
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
                   ` (7 preceding siblings ...)
  2025-02-10  3:00 ` [PATCH v2 08/10] rust/timer/hpet: add basic HPET timer and HPETState Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  2025-02-10  3:00 ` [PATCH v2 10/10] i386: enable rust hpet for pc when rust is enabled Zhao Liu
  9 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

Implement QOM & QAPI support for HPET device.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * Merge qdev.rs to hpet.rs.
 * Apply memory and Resettable bindings.
 * Consolidate inmutable &self and QOM casting.
 * Prefer timer iterator over loop.
 * Move init_mmio() and init_irq() to post_init().

Changes since Patch v1:
 * Fix complaints from cargo (+nightly) doc/clippy/fmt (v1.84.0).
 * Drop HPETClass.
 * Do `addr & !4` in timer's register read and write.
 * Mask the timer N's register address with 0x1F (timer_and_addr()).
 * Optimize variable naming for clarity.
---
 rust/hw/timer/hpet/src/fw_cfg.rs |   2 -
 rust/hw/timer/hpet/src/hpet.rs   | 275 ++++++++++++++++++++++++++++++-
 rust/hw/timer/hpet/src/lib.rs    |   4 +
 3 files changed, 272 insertions(+), 9 deletions(-)

diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs
index d4c24813063d..f548a3f4f8e1 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 std::ptr::addr_of_mut;
 
 use qemu_api::{cell::bql_locked, impl_zeroable, zeroable::Zeroable};
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 7d10c4205348..1fb4986dd688 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -2,21 +2,33 @@
 // Author(s): Zhao Liu <zhai1.liu@intel.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-#![allow(dead_code)]
-
-use std::ptr::{addr_of_mut, null_mut, NonNull};
+use std::{
+    ffi::CStr,
+    ptr::{addr_of_mut, null_mut, NonNull},
+    slice::from_ref,
+};
 
 use qemu_api::{
-    bindings::{address_space_memory, address_space_stl_le},
+    bindings::{
+        address_space_memory, address_space_stl_le, qdev_prop_bit, qdev_prop_bool,
+        qdev_prop_uint32, qdev_prop_uint8,
+    },
+    c_str,
     cell::{BqlCell, BqlRefCell},
     irq::InterruptSource,
-    memory::{MemoryRegion, MEMTXATTRS_UNSPECIFIED},
+    memory::{
+        hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder, MEMTXATTRS_UNSPECIFIED,
+    },
     prelude::*,
-    qom::ParentField,
+    qdev::{DeviceImpl, DeviceMethods, DeviceState, Property, ResetType, ResettablePhasesImpl},
+    qom::{ObjectImpl, ObjectType, ParentField},
+    qom_isa,
     sysbus::SysBusDevice,
     timer::{Timer, CLOCK_VIRTUAL, NS},
 };
 
+use crate::fw_cfg::HPETFwConfig;
+
 /// Register space for each timer block (`HPET_BASE` is defined in hpet.h).
 const HPET_REG_SPACE_LEN: u64 = 0x400; // 1024 bytes
 
@@ -451,11 +463,41 @@ fn callback(&mut self) {
         }
         self.update_irq(true);
     }
+
+    const fn read(&self, addr: hwaddr, _size: u32) -> u64 {
+        let shift: u64 = (addr & 4) * 8;
+
+        match addr & !4 {
+            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: u32) {
+        let shift = ((addr & 4) * 8) as u32;
+        let len = std::cmp::min(size * 8, 64 - shift);
+
+        match addr & !4 {
+            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.
+            }
+        }
+    }
 }
 
 /// HPET Event Timer Block Abstraction
 #[repr(C)]
-#[derive(qemu_api_macros::offsets)]
+#[derive(qemu_api_macros::Object, qemu_api_macros::offsets)]
 pub struct HPETState {
     parent_obj: ParentField<SysBusDevice>,
     iomem: MemoryRegion,
@@ -626,4 +668,223 @@ fn set_counter_reg(&self, shift: u32, len: u32, val: u64) {
         self.counter
             .set(self.counter.get().deposit(shift, len, val));
     }
+
+    unsafe fn init(&mut self) {
+        static HPET_RAM_OPS: MemoryRegionOps<HPETState> =
+            MemoryRegionOpsBuilder::<HPETState>::new()
+                .read(&HPETState::read)
+                .write(&HPETState::write)
+                .native_endian()
+                .valid_sizes(4, 8)
+                .impl_sizes(4, 8)
+                .build();
+
+        // SAFETY:
+        // self and self.iomem are guaranteed to be valid at this point since callers
+        // must make sure the `self` reference is valid.
+        MemoryRegion::init_io(
+            unsafe { &mut *addr_of_mut!(self.iomem) },
+            addr_of_mut!(*self),
+            &HPET_RAM_OPS,
+            "hpet",
+            HPET_REG_SPACE_LEN,
+        );
+    }
+
+    fn post_init(&self) {
+        self.init_mmio(&self.iomem);
+        for irq in self.irqs.iter() {
+            self.init_irq(irq);
+        }
+    }
+
+    fn realize(&self) {
+        if self.int_route_cap == 0 {
+            // TODO: Add error binding: warn_report()
+            println!("Hpet's hpet-intcap property not initialized");
+        }
+
+        self.hpet_id.set(HPETFwConfig::assign_hpet_id());
+
+        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, HPETState::handle_legacy_irq);
+        self.init_gpio_out(from_ref(&self.pit_enabled));
+    }
+
+    fn reset_hold(&self, _type: ResetType) {
+        let sbd = self.upcast::<SysBusDevice>();
+
+        for timer in self.timers.iter().take(self.num_timers.get()) {
+            timer.borrow_mut().reset();
+        }
+
+        self.counter.set(0);
+        self.config.set(0);
+        self.pit_enabled.set(true);
+        self.hpet_offset.set(0);
+
+        HPETFwConfig::update_hpet_cfg(
+            self.hpet_id.get(),
+            self.capability.get() as u32,
+            sbd.mmio[0].addr,
+        );
+
+        // to document that the RTC lowers its output on reset as well
+        self.rtc_irq_level.set(0);
+    }
+
+    fn timer_and_addr(&self, addr: hwaddr) -> Option<(&BqlRefCell<HPETTimer>, hwaddr)> {
+        let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
+
+        // TODO: Add trace point - trace_hpet_ram_[read|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)
+            None
+        } else {
+            // Keep the complete address so that HPETTimer's read and write could
+            // detect the invalid access.
+            Some((&self.timers[timer_id], addr & 0x1F))
+        }
+    }
+
+    fn read(&self, addr: hwaddr, size: u32) -> u64 {
+        let shift: u64 = (addr & 4) * 8;
+
+        // address range of all TN regs
+        // TODO: Add trace point - trace_hpet_ram_read(addr)
+        if (0x100..=0x3ff).contains(&addr) {
+            match self.timer_and_addr(addr) {
+                None => 0, // Reserved,
+                Some((timer, tn_addr)) => timer.borrow_mut().read(tn_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(&self, addr: hwaddr, value: u64, size: u32) {
+        let shift = ((addr & 4) * 8) as u32;
+        let len = std::cmp::min(size * 8, 64 - shift);
+
+        // TODO: Add trace point - trace_hpet_ram_write(addr, value)
+        if (0x100..=0x3ff).contains(&addr) {
+            match self.timer_and_addr(addr) {
+                None => (), // Reserved.
+                Some((timer, tn_addr)) => timer.borrow_mut().write(tn_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.
+                }
+            }
+        }
+    }
+}
+
+qom_isa!(HPETState: SysBusDevice, DeviceState, Object);
+
+unsafe impl ObjectType for HPETState {
+    // No need for HPETClass. Just like OBJECT_DECLARE_SIMPLE_TYPE in C.
+    type Class = <SysBusDevice as ObjectType>::Class;
+    const TYPE_NAME: &'static CStr = crate::TYPE_HPET;
+}
+
+impl ObjectImpl for HPETState {
+    type ParentType = SysBusDevice;
+
+    const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
+    const INSTANCE_POST_INIT: Option<fn(&Self)> = Some(Self::post_init);
+}
+
+// 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
+    ),
+}
+
+impl DeviceImpl for HPETState {
+    fn properties() -> &'static [Property] {
+        &HPET_PROPERTIES
+    }
+
+    const REALIZE: Option<fn(&Self)> = Some(Self::realize);
+}
+
+impl ResettablePhasesImpl for HPETState {
+    const HOLD: Option<fn(&Self, ResetType)> = Some(Self::reset_hold);
 }
diff --git a/rust/hw/timer/hpet/src/lib.rs b/rust/hw/timer/hpet/src/lib.rs
index 027f7f83349a..25251112a86d 100644
--- a/rust/hw/timer/hpet/src/lib.rs
+++ b/rust/hw/timer/hpet/src/lib.rs
@@ -10,5 +10,9 @@
 
 #![deny(unsafe_op_in_unsafe_fn)]
 
+use qemu_api::c_str;
+
 pub mod fw_cfg;
 pub mod hpet;
+
+pub const TYPE_HPET: &::std::ffi::CStr = c_str!("hpet");
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 10/10] i386: enable rust hpet for pc when rust is enabled
  2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
                   ` (8 preceding siblings ...)
  2025-02-10  3:00 ` [PATCH v2 09/10] rust/timer/hpet: add qom and qdev APIs support Zhao Liu
@ 2025-02-10  3:00 ` Zhao Liu
  9 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10  3:00 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust, Zhao Liu

Add HPET configuration in PC's Kconfig options, and select HPET device
(Rust version) if Rust is supported.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC:
 * Fix rust hpet not being optional.
 * Fix bios-tables-test missing rust hpet.
---
 configs/devices/i386-softmmu/default.mak | 1 +
 hw/i386/fw_cfg.c                         | 2 +-
 hw/i386/pc.c                             | 2 +-
 hw/timer/Kconfig                         | 6 +++++-
 rust/hw/Kconfig                          | 1 +
 rust/hw/timer/Kconfig                    | 2 ++
 tests/qtest/meson.build                  | 3 ++-
 7 files changed, 13 insertions(+), 4 deletions(-)
 create mode 100644 rust/hw/timer/Kconfig

diff --git a/configs/devices/i386-softmmu/default.mak b/configs/devices/i386-softmmu/default.mak
index 4faf2f0315e2..9ef343cace06 100644
--- a/configs/devices/i386-softmmu/default.mak
+++ b/configs/devices/i386-softmmu/default.mak
@@ -6,6 +6,7 @@
 #CONFIG_APPLESMC=n
 #CONFIG_FDC=n
 #CONFIG_HPET=n
+#CONFIG_X_HPET_RUST=n
 #CONFIG_HYPERV=n
 #CONFIG_ISA_DEBUG=n
 #CONFIG_ISA_IPMI_BT=n
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 546de63123e6..a7d69a49edbf 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -147,7 +147,7 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
 #endif
     fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1);
 
-#ifdef CONFIG_HPET
+#if defined(CONFIG_HPET) || defined(CONFIG_X_HPET_RUST)
     PCMachineState *pcms =
         (PCMachineState *)object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE);
     if (pcms && pcms->hpet_enabled) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b46975c8a4db..04554817e021 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1701,7 +1701,7 @@ static void pc_machine_initfn(Object *obj)
     pcms->sata_enabled = true;
     pcms->i8042_enabled = true;
     pcms->max_fw_size = 8 * MiB;
-#ifdef CONFIG_HPET
+#if defined(CONFIG_HPET) || defined(CONFIG_X_HPET_RUST)
     pcms->hpet_enabled = true;
 #endif
     pcms->fd_bootchk = true;
diff --git a/hw/timer/Kconfig b/hw/timer/Kconfig
index c96fd5d97ae8..c051597180f4 100644
--- a/hw/timer/Kconfig
+++ b/hw/timer/Kconfig
@@ -11,7 +11,11 @@ config A9_GTIMER
 
 config HPET
     bool
-    default y if PC
+    default y if PC && !HAVE_RUST
+
+config X_HPET_RUST
+    bool
+    default y if PC && HAVE_RUST
 
 config I8254
     bool
diff --git a/rust/hw/Kconfig b/rust/hw/Kconfig
index 4d934f30afe1..36f92ec02874 100644
--- a/rust/hw/Kconfig
+++ b/rust/hw/Kconfig
@@ -1,2 +1,3 @@
 # devices Kconfig
 source char/Kconfig
+source timer/Kconfig
diff --git a/rust/hw/timer/Kconfig b/rust/hw/timer/Kconfig
new file mode 100644
index 000000000000..afd980335037
--- /dev/null
+++ b/rust/hw/timer/Kconfig
@@ -0,0 +1,2 @@
+config X_HPET_RUST
+    bool
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 5e062c752a7e..34c59107cf4f 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -103,7 +103,8 @@ qtests_i386 = \
    config_all_devices.has_key('CONFIG_VIRTIO_PCI') and                                      \
    slirp.found() ? ['virtio-net-failover'] : []) +                                          \
   (unpack_edk2_blobs and                                                                    \
-   config_all_devices.has_key('CONFIG_HPET') and                                            \
+   (config_all_devices.has_key('CONFIG_HPET') or                                            \
+    config_all_devices.has_key('CONFIG_X_HPET_RUST')) and                                   \
    config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) +             \
   qtests_pci +                                                                              \
   qtests_cxl +                                                                              \
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 05/10] rust: add bindings for memattrs
  2025-02-10  3:00 ` [PATCH v2 05/10] rust: add bindings for memattrs Zhao Liu
@ 2025-02-10 10:03   ` Zhao Liu
  0 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-10 10:03 UTC (permalink / raw)
  To: Paolo Bonzini, Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé
  Cc: qemu-devel, qemu-rust

> +/// A special `MemTxAttrs` constant, used to indicate that no memary

typo... s/memary/memory/

> +/// attributes are specified.
> +///
> +/// Bus masters which don't specify any attributes will get this,
> +/// which has all attribute bits clear except the topmost one
> +/// (so that we can distinguish "all attributes deliberately clear"
> +/// from "didn't specify" if necessary).
> +pub const MEMTXATTRS_UNSPECIFIED: MemTxAttrs = MemTxAttrs {
> +    unspecified: true,
> +    ..Zeroable::ZERO
> +};


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to hpet.c
  2025-02-10  3:00 ` [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to hpet.c Zhao Liu
@ 2025-02-13 11:25   ` Paolo Bonzini
  2025-02-16 11:47     ` Zhao Liu
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2025-02-13 11:25 UTC (permalink / raw)
  To: Zhao Liu
  Cc: Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé, qemu-devel, qemu-rust

On Mon, Feb 10, 2025 at 3:41 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> index d2cb08715a21..546de63123e6 100644
> --- a/hw/i386/fw_cfg.c
> +++ b/hw/i386/fw_cfg.c
> @@ -26,8 +26,6 @@
>  #include CONFIG_DEVICES
>  #include "target/i386/cpu.h"
>
> -struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};

This must be kept for the case where HPET is not enabled at all in the
build; removing the FW_CFG_HPET file changes the guest API and I'd
prefer to merge the Rust HPET implementation without having to figure
out the safety of that change.

No need to do anything, I'll just make it

#if !defined(CONFIG_HPET) && !defined(CONFIG_X_HPET_RUST)
const struct hpet_fw_config hpet_fw_cfg = {.count = UINT8_MAX};
#endif

Paolo



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to hpet.c
  2025-02-13 11:25   ` Paolo Bonzini
@ 2025-02-16 11:47     ` Zhao Liu
  0 siblings, 0 replies; 14+ messages in thread
From: Zhao Liu @ 2025-02-16 11:47 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Manos Pitsidianakis, Junjie Mao, Alex Bennée,
	Philippe Mathieu-Daudé, Richard Henderson, Peter Maydell,
	Daniel P . Berrangé, qemu-devel, qemu-rust

On Thu, Feb 13, 2025 at 12:25:55PM +0100, Paolo Bonzini wrote:
> Date: Thu, 13 Feb 2025 12:25:55 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to
>  hpet.c
> 
> On Mon, Feb 10, 2025 at 3:41 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> > diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> > index d2cb08715a21..546de63123e6 100644
> > --- a/hw/i386/fw_cfg.c
> > +++ b/hw/i386/fw_cfg.c
> > @@ -26,8 +26,6 @@
> >  #include CONFIG_DEVICES
> >  #include "target/i386/cpu.h"
> >
> > -struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
> 
> This must be kept for the case where HPET is not enabled at all in the
> build; removing the FW_CFG_HPET file changes the guest API and I'd
> prefer to merge the Rust HPET implementation without having to figure
> out the safety of that change.
> 
> No need to do anything, I'll just make it
> 
> #if !defined(CONFIG_HPET) && !defined(CONFIG_X_HPET_RUST)
> const struct hpet_fw_config hpet_fw_cfg = {.count = UINT8_MAX};
> #endif
> 

Thanks! This makes sense.

Zhao



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-02-16 11:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-10  3:00 [PATCH v2 00/10] rust: Add HPET timer device Zhao Liu
2025-02-10  3:00 ` [PATCH v2 01/10] i386/fw_cfg: move hpet_cfg definition to hpet.c Zhao Liu
2025-02-13 11:25   ` Paolo Bonzini
2025-02-16 11:47     ` Zhao Liu
2025-02-10  3:00 ` [PATCH v2 02/10] rust/qdev: add the macro to define bit property Zhao Liu
2025-02-10  3:00 ` [PATCH v2 03/10] rust/irq: Add a helper to convert [InterruptSource] to pointer Zhao Liu
2025-02-10  3:00 ` [PATCH v2 04/10] rust: add bindings for gpio_{in|out} initialization Zhao Liu
2025-02-10  3:00 ` [PATCH v2 05/10] rust: add bindings for memattrs Zhao Liu
2025-02-10 10:03   ` Zhao Liu
2025-02-10  3:00 ` [PATCH v2 06/10] rust: add bindings for timer Zhao Liu
2025-02-10  3:00 ` [PATCH v2 07/10] rust/timer/hpet: define hpet_fw_cfg Zhao Liu
2025-02-10  3:00 ` [PATCH v2 08/10] rust/timer/hpet: add basic HPET timer and HPETState Zhao Liu
2025-02-10  3:00 ` [PATCH v2 09/10] rust/timer/hpet: add qom and qdev APIs support Zhao Liu
2025-02-10  3:00 ` [PATCH v2 10/10] i386: enable rust hpet for pc when rust is enabled Zhao Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).