[RFC 00/13] rust: Reinvent the wheel for HPET timer in Rust

[Zhao Liu]

Hi,

After making empty promises for many months, I have finally written the
Rust version of HPET :-) I'm also very grateful for the help from Paolo,
Manos, and Junjie!

Overall, HPET in Rust maintains the same logic as the original C
version, adhering to the IA-HPET spec v1.0a [1]. 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}, bitops, memattrs, and timer. Finally,
based on Paolo's suggestion, the vmstate part is temporarily on hold.

Welcome your comments and feedback!


(Next, I will introduce the structure of the code, the current gaps, and
share my verbose personal experience of writing a QEMU device in Rust.)


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

.
│ 
...
└── timer
    ├── hpet
    │   ├── Cargo.toml
    │   ├── meson.build
    │   └── src
    │       ├── fw_cfg.rs
    │       ├── hpet.rs
    │       ├── lib.rs
    │       └── qdev.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.

     My goal is to reduce unsafe code in this file as much as possible,
     especifically, try to eliminate the unsafe code brought by FFI.

   - qdev.rs:
     Here, it implements various QEMU qdev/qom required traits for the
     HPET state and try to exclude the detailed HPET state operations to
     the hpet.rs file above.

 * 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.

     Specifically, I wrapped it in BqlCell, which ensures the safety of
     Rust device access. Additionally, because BqlCell does not change
     the memory layout, it does not disrupt access from C code.


Current Gaps
============

* Proper bindings for MemoryRegionOps, which needs to wrap the ram
  read/write callbacks.
  - I think it shouldn't be complicated because qom/qdev already
    provides good examples.

* vmstate support.
  - vmstate code for HPET is actually ready, but it will be pending (and
    maybe re-writing) until the vmstate infra gets cleaned up.

* Error handling.
  - Now HPET just use panic and println to replace error_setg and
    warn_report.

* Trace support.
  - No trace for now.


Experience and Considerations in Rust Device
============================================

BqlCell/BqlRefCell
------------------

Paolo provided a very useful Cell wrapper to operate the device under
the protection of BQL. So I tried to wrap as much as possible fields of
HPETState into BqlCell/BqlRefCell, and it works well :-). 

Anything that needs to be modified within a callback should be protected
by BqlCell/BqlRefCell.

Based on this, I am also considering how the opaque parameter in certain
callbacks should interact with BQL cells. In the timer binding (patch 
7), I think the opaque parameter accepted by the timer callback should
be placed in a BQL cell. However, considering generality, I did not make
further changes and only passed BqlRefCell<HPETTimer> as the opaque
parameter in the HPET code.

Furthermore, is it possible in the future to wrap the entire state
within a BQL cell? This could save the effort of wrapping many state
members individually when state becomes very huge and complex.


QDEV Property
-------------

To support bit type property, I added another macro variant (in patch 8)
to allow bitnr parameter. However, I think this lacks scalability.

In qdev-properties.h, it is clear that the PropertyInfo of a property is
bound to its type. Previously, Junjie and I attempted to do the same in
Rust by binding PropertyInfo to the type, thereby avoiding the need to
specify too many parameters in the macro definitions:

https://lore.kernel.org/qemu-devel/20241017143245.1248589-1-zhao1.liu@intel.com/

However, unfortunately, it was missed. I am not sure if this is the
right direction, but perhaps I can pick it up again?


MEMTXATTRS_UNSPECIFIED
----------------------

MEMTXATTRS_UNSPECIFIED is another global variable. Since it is
immutable, BQL cell is not needed.

But MemTxAttrs is a structure with bitfields, and the bindings generated
by bindgen can only be modified through methods. Therefore, it is
necessary to introduce lazy to initialize MEMTXATTRS_UNSPECIFIED in a
const context (patch 6).


Cycle Reference
---------------

A common pattern in QEMU devices is that a sub-member of the state
contains a pointer to the state itself.

For HPETState, it maintains a HPETTimer array, and each HPETTimer also
has the pointer to parent HPETState. So there's the cycle reference
issue.

The common way to address this is to use RefCell<Weak<>> [2], but in
practice, I found it's difficult to be applied in device code. The
device instance is not created in Rust side, and there's only init()
method to initialize created device instance. This way, it's hard to be
compatible with the pattern of creating weak references (at least I
failed).

Then, I chose NonNull to address this issue, as recommended by the
author of NonNull and the standard collections [3].

Though NonNull is used for "covariant" and even its document mentions:

> This is often the correct thing to use when building data structures
> using raw pointers, but is ultimately more dangerous to use because of
> its additional properties. If you’re not sure if you should use
> NonNull<T>, just use *mut T!

But I feel that the usage of NonNull has been greatly expanded, for
example, to ensure non-null pointers in FFI case. Additionally, in cases
of cyclic references, using NonNull for encapsulation also appears safer
and more elegant.


Public and Private in QOM State
-------------------------------

I recently asked on the mailing list [4] about the reason for using
"<public>"/"<private>" comments in QOM structures. Peter, Junjie, and
Balaton provided some explanations and feedback (thank you all).

It seems to be more of a historical issue where QOM did not handle the
public and private access very well. However, Peter and Igor (in the
discussion about LoongArch hotplug) advised me that it is best to use
properties to access QOM members from the outside.

Therefore, I believe that in Rust devices, QOM members should also be
kept private or at most `pub(crate)`. This is also why I tried to avoid
using direct `pub` in HPET.


[1]: 
https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/software-developers-hpet-spec-1-0a.pdf
[2]: https://doc.rust-lang.org/book/ch15-06-reference-cycles.html
[3]: https://rust-unofficial.github.io/too-many-lists/sixth-variance.html
[4]: https://lore.kernel.org/qemu-devel/ZxPZ5oUDRcVroh7o@intel.com/
---
Paolo Bonzini (2):
  bql: check that the BQL is not dropped within marked sections
  rust: cell: add BQL-enforcing RefCell variant

Zhao Liu (11):
  rust/cell: add get_mut() method for BqlCell
  rust: add bindings for gpio_{in|out} initialization
  rust: add a bit operation binding for deposit64
  rust: add bindings for memattrs
  rust: add bindings for timer
  rust/qdev: add the macro to define bit property
  i386/fw_cfg: move hpet_cfg definition to hpet.c
  rust/timer/hpet: define hpet_fw_cfg
  rust/timer/hpet: add basic HPET timer & state
  rust/timer/hpet: add qdev APIs support
  i386: enable rust hpet for pc when rust is enabled

 hw/i386/Kconfig                               |   2 +
 hw/i386/fw_cfg.c                              |   4 +-
 hw/timer/Kconfig                              |   1 -
 hw/timer/hpet.c                               |  16 +-
 include/hw/timer/hpet.h                       |   2 +-
 include/qemu/main-loop.h                      |  15 +
 rust/Cargo.lock                               |  15 +
 rust/Cargo.toml                               |   1 +
 rust/hw/Kconfig                               |   1 +
 rust/hw/meson.build                           |   1 +
 rust/hw/timer/Kconfig                         |   2 +
 rust/hw/timer/hpet/Cargo.toml                 |  14 +
 rust/hw/timer/hpet/meson.build                |  18 +
 rust/hw/timer/hpet/src/fw_cfg.rs              |  86 ++
 rust/hw/timer/hpet/src/hpet.rs                | 860 ++++++++++++++++++
 rust/hw/timer/hpet/src/lib.rs                 |  21 +
 rust/hw/timer/hpet/src/qdev.rs                | 133 +++
 rust/hw/timer/meson.build                     |   1 +
 rust/qemu-api/Cargo.toml                      |   4 +-
 rust/qemu-api/meson.build                     |  14 +-
 rust/qemu-api/src/bitops.rs                   |  11 +
 rust/qemu-api/src/cell.rs                     | 571 +++++++++++-
 rust/qemu-api/src/lib.rs                      |   3 +
 rust/qemu-api/src/memattrs.rs                 |  21 +
 rust/qemu-api/src/qdev.rs                     |  67 +-
 rust/qemu-api/src/timer.rs                    | 123 +++
 rust/wrapper.h                                |   3 +
 scripts/archive-source.sh                     |   2 +-
 scripts/make-release                          |   2 +-
 stubs/iothread-lock.c                         |  15 +
 subprojects/.gitignore                        |   1 +
 subprojects/once_cell-1.20-rs.wrap            |   7 +
 .../once_cell-1.20-rs/meson.build             |  23 +
 system/cpus.c                                 |  15 +
 34 files changed, 2046 insertions(+), 29 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/hpet/src/qdev.rs
 create mode 100644 rust/hw/timer/meson.build
 create mode 100644 rust/qemu-api/src/bitops.rs
 create mode 100644 rust/qemu-api/src/memattrs.rs
 create mode 100644 rust/qemu-api/src/timer.rs
 create mode 100644 subprojects/once_cell-1.20-rs.wrap
 create mode 100644 subprojects/packagefiles/once_cell-1.20-rs/meson.build

-- 
2.34.1