qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH v2 3/5] rust: add PL011 device model


From: Manos Pitsidianakis
Subject: Re: [RFC PATCH v2 3/5] rust: add PL011 device model
Date: Mon, 17 Jun 2024 16:54:11 +0300
User-agent: meli 0.8.6

On Mon, 17 Jun 2024 14:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
Il lun 17 giu 2024, 10:59 Manos Pitsidianakis <
manos.pitsidianakis@linaro.org> ha scritto:

>qdev_define_type!(c"test-device", TestDevice);
>impl ObjectImpl for TestDevice {}
>impl DeviceImpl for TestDevice {}
>
>fn main() {
>    let d = TestDevice::new();
>    d.cold_reset();
>}
>
>Of course the code makes no sense but it's a start.

Let's not rush into making interfaces without the need for them arising
first. It's easy to wander off into bikeshedding territory; case in
point, there has been little discussion on the code of this RFC and much
more focus on hypotheticals.


I see your point but I think it's important to understand the road ahead of
us.

Knowing that we can build and maintain a usable (does not have to be
perfect) interface to QOM is important, and in fact it's already useful for
the pl011 device's chardev. It's also important to play with more advanced
usage of the language to ascertain what features of the language will be
useful; for example, my current implementation uses generic associated
types which are not available on Debian Bookworm—it should be easy to
remove them but if I am wrong that's also a data point, and an important
one.

Don't get me wrong: *for this first device* only, it makes a lot of sense
to have a very C-ish implementation. It lets us sort out the build system
integration, tackle idiomatic bindings one piece at a time, and is easier
to review than Marc-André's approach of building the whole QAPI bindings.
But at the same time, I don't consider a C-ish device better just because
it's written in Rust: as things stand your code has all the disadvantages
of C and all the disadvantages of Rust. What makes it (extremely) valuable
is that your code can lead us along the path towards reaping the advantages
of Rust.

I respectfully disagree and recommend taking another look at the code.

The device actually performs all logic in non-unsafe methods and is typed instead of operating on raw integers as fields/state. The C stuff is the FFI boundary calls which you cannot avoid; they are the same things you'd wrap under these bindings we're talking about.

QEMU calls the device's FFI callbacks with its pointer and arguments, the pointer gets dereferenced to the actual Rust type which qemu guarantees is valid, then the Rust struct's methods are called to handle each functionality. There is nothing actually unsafe here, assuming QEMU's invariants and code are correct.


I think we're actually in a great position. We have a PoC from Marc-André,
plus the experience of glib-rs (see below), that shows us that our goal of
idiomatic bindings is doable; and a complementary PoC from you that will
guide us and let us reach it in little steps. The first 90% of the work is
done, now we only have to do the second 90%... :) but then we can open the
floodgates for Rust code in QEMU.

For what it's worth, in my opinion looking at glib-rs for inspiration is
a bad idea, because that project has to support an immutable public
interface (glib) while we do not.


glib-rs includes support for GObject, which was itself an inspiration for
QOM (with differences, granted).

There are a lot of libraries that we can take inspiration from: in addition
to glib-rs, zbus has an interesting approach to
serialization/deserialization for example that could inform future work on
QAPI. It's not a coincidence that these libraries integrate with more
traditional C code, because we are doing the same. Rust-vmm crates will
also become an interesting topic sooner or later.

There's more to discuss about this topic which I am open to continuing
on IRC instead!


Absolutely, I will try to post code soonish and also review the build
system integration.

Thanks,

Paolo


-- Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

>
>One thing that would be very useful is to have an Error
>implementation. Looking at what Marc-André did for Error*
>(
https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/20210907121943.3498701-13-marcandre.lureau@redhat.com/
),
>his precise implementation relies on his code to go back and forth
>between Rust representation, borrowed C object with Rust bindings and
>owned C object with Rust bindings. But I think we can at least have
>something like this:
>
>// qemu::Error
>pub struct Error {
>    msg: String,
>    /// Appends the print string of the error to the msg if not None
>    cause: Option<Box<dyn std::error::Error>>,
>    location: Option<(String, u32)>
>}
>
>impl std::error::Error for Error { ... }
>
>impl Error {
>  ...
>  fn into_c_error(self) -> *const bindings::Error { ... }
>}
>
>// qemu::Result<T>
>type Result<T> = Result<T, Error>;
>
>which can be implemented without too much code. This way any "bool
>f(..., Error *)" function (example: realize :)) could be implemented
>as returning qemu::Result<()>.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]