On 10/24/24 16:02, Manos Pitsidianakis wrote:
Hello everyone, the pathological corrosion of QEMU code continues.
This series expands the device model harness work performed in the
initial Rust work from the previous month. In particular:
Wow, there's a lot of stuff here!
The very good news is that it's basically all the code that is needed to
get CI running, after which we can start with the fun stuff. At the
same time, "the fun stuff" is also the one that risks introducing
technical debt, so we need to switch to quality-over-quantity mode and
have a serious design discussion about it. I'll do that later as a
reply to the patches.
Code and functionality duplication is not fun, and pl011 was mostly
done as a proof of concept for a Rust device because of its small
complexity. As of this moment we have not decided on a policy for how
to handle these things and it is not in **scope for this patch series
anyway**.
That's fine.
Looking at the currently posted series, it seems that we have three main
themes:
1) small scale cleanups: duplicated and useless code, improved testing.
These are in
https://lore.kernel.org/qemu-devel/20241021163538.136941-1-pbonz...@redhat.com/T/
and they have been reviewed already. But these two:
Revert "rust: add PL011 device model"
rust: add PL011 device model
... should definitely be moved on top to clean up the authorship in "git
blame" and other similar tools. Sorry about that.
2) allow using older rustc/bindgen, extend CI to cover it. This is
https://lore.kernel.org/qemu-devel/20241022100956.196657-1-pbonz...@redhat.com/T/
which still needs review. These five:
rust: add support for migration in device models
rust/pl011: move CLK_NAME static to function scope
rust/pl011: add TYPE_PL011_LUMINARY device
rust/pl011: remove commented out C code
rust/pl011: Use correct masks for IBRD and FBRD
(minus the usage of #[derive()] should be included in that series, so
that qtests pass. It's not a huge amount of work and I can extract it,
of course with proper attribution/authorship.
The rest are future work:
rust/qemu-api-macros: introduce Device proc macro
This is useful as a starting point but it has the limit of being very
device-specific. This is of course okay with properties and vmstate,
but in my opinion the implementation of class_init should be as generic
as possible, and not too specialized for methods in Object or Device.
As I said above, we first need to agree on the design.
rust/pl011: move pub callback decl to local scope
This depends a lot on how we go implementing bindings to chardev. For
example if the callbacks turn out to be a trait, it would have to be
undone. Possibly the C callback wrappers would move to
rust/qemu-api/chardev. For now I'd leave it aside.
rust/qemu-api: add log module
rust/pl011: log guest/unimp errors
This also needs design discussion. Do we want the API to be the same as
C, i.e. keep the qemu_* prefix? Do we want wrapper macros that include
the format!() call?
You also have "pub enum LogMask" to work around the fact that log masks
are preprocessor macros. Is that okay, or do we want to modify C code
to make the bindings nicer? I for example would prefer the latter and
then declaring LogMask as a bitfield in bindgen.
Thanks,
Paolo
rust/wrapper.h | 1 +
rust/hw/char/pl011/src/device.rs | 419 +++++++++++++++++---------
rust/hw/char/pl011/src/device_class.rs | 70 -----
rust/hw/char/pl011/src/lib.rs | 2 +-
rust/qemu-api-macros/src/device.rs | 370 +++++++++++++++++++++++
rust/qemu-api-macros/src/lib.rs | 46 +--
rust/qemu-api-macros/src/object.rs | 107 +++++++
rust/qemu-api-macros/src/symbols.rs | 57 ++++
rust/qemu-api-macros/src/utilities.rs | 152 ++++++++++
rust/qemu-api-macros/src/vmstate.rs | 113 +++++++
rust/qemu-api/meson.build | 5 +-
rust/qemu-api/src/definitions.rs | 97 ------
rust/qemu-api/src/device_class.rs | 128 --------
rust/qemu-api/src/lib.rs | 10 +-
rust/qemu-api/src/log.rs | 140 +++++++++
rust/qemu-api/src/objects.rs | 90 ++++++
rust/qemu-api/src/tests.rs | 49 ---
rust/qemu-api/src/vmstate.rs | 403 +++++++++++++++++++++++++
subprojects/packagefiles/syn-2-rs/meson.build | 1 +
19 files changed, 1726 insertions(+), 534 deletions(-)
---
base-commit: 55522f72149fbf95ee3b057f1419da0cad46d0dd
change-id: 20241024-rust-round-2-69fa10c9a0c9
--
γαῖα πυρί μιχθήτω
