Re: [PATCH 00/14] rust: example of bindings code for Rust in QEMU

[Paolo Bonzini]

On Thu, Jul 4, 2024 at 9:26 PM Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
> > Patches 9-10 deal with how to define new subclasses in Rust.  They are
> > a lot less polished and less ready.  There is probably a lot of polish
> > that could be applied to make the code look nicer, but I guess there is
> > always time to make the code look nicer until the details are sorted out.
> >
> > The things that I considered here are:
> >
> > - splitting configuration from runtime state
> > - automatic generation of instance_init and instance_finalize
> > - generation of C vtables from safe code that is written in Rust
> > - generation of qdev property tables
>
> Shouldn't we focus more on how we want to write a QOM device in Rust
> instead, by making abstraction of existing C implementation first?
> Once we have satisfying idea, we could evaluate how it maps to existing
> implementation, and which compromise should be made for efficiency.
> It seems that the current approach is to stick to existing model, and
> derive Rust bindings from this.

I think I tried to do a little bit of that. For example, the split of
configuration from runtime state is done to isolate the interior
mutability, and the automatic generation of instance_init and
instance_finalize relies on Rust traits like Default and Drop; the
realize implementation returns a qemu::Result<()>; and so on.

But there are many steps towards converting the PL011 device to Rust:

- declarative definitions of bitfields and registers (done)
- using RefCell for mutable state
- using wrappers for class and property definition
- defining and using wrappers for Chardev/CharBackend
- defining and using wrappers for MemoryRegion callbacks
- defining and using wrappers for SysBusDevice functionality
- defining and using wrappers for VMState functionality

At each step we need to design "how we want to do that in Rust" and
all the things you mention above. This prototype covers the second and
third steps, and it's already big enough! :)

I expect that code like this one would be merged together with the
corresponding changes to PL011: the glue code is added to the qemu/
crate and used in the hw/core/pl011/ directory. This way, both authors
and reviewers will have a clue of what becomes more readable/idiomatic
in the resulting code.

> I agree this glue can be a source of technical debt, but on the other
> side, it should be easy to refactor it, if we decided first what the
> "clean and idiomatic" Rust API should look like.

That's true, especially if we want to add more macro magic on top. We
can decide later where to draw the line between complexity of glue
code and cleanliness of Rust code, and also move the line as we see
fit.

On the other hand, if we believe that _this_ code is already too much,
that's also a data point. Again: I don't think it is, but I want us to
make an informed decision.

> A compromise where you have to manually translate some structs, or copy
> memory to traverse languages borders at some point, could be worth the
> safety and expressiveness.

Yep, Error is an example of this. It's not the common case, so it's
totally fine to convert to and from C Error* (which also includes
copying the inner error string, from a String to malloc-ed char*) to
keep the APIs nicer.

> > We should have an idea of what this glue code looks like, in order to make
> > an informed choice.  If we think we're not comfortable with reviewing it,
> > well, we should be ready to say so and stick with C until we are.
>
> While it is important that this glue code is maintainable and looks
> great, I don't think it should be the main reason to accept usage of a
> new language.

Not the main reason, but an important factor in judging if we are
*able* to accept usage of a new language. Maybe it's just a formal
step, but it feels safer to have _some_ code to show and to read, even
if it's just a prototype that barely compiles.

> We could have a specific trait with functions to handle various kind of
> events. And the glue code could be responsible to translate this to
> callbacks for the C side (and calling Rust code accordingly, eventually
> serializing this on a single thread to avoid any race issues).

That's a possibility, yes. Something like (totally random):

impl CharBackend {
    pub fn start<T: CharBackendCallbacks>(&mut self, obj: &T) {
        qemu_chr_fe_set_handlers(self.0,
                             Some(obj::can_receive),
Some(obj::receive, obj::event),
                             Some(obj::be_change), obj as *const _ as
*const c_void,
                             ptr::null(), true);
    }
    pub fn stop(&mut self) {
        qemu_chr_fe_set_handlers(self.0, None, None,
                             None, ptr::null(), ptr::null(), true);
    }
}

but I left for later because I focused just on the lowest levels code
where you could have "one" design. For example, for memory regions
some devices are going to have more than one, so there could be
reasons to do callbacks in different ways. By the way, all of this
passing of Rust references as opaque pointers needs, at least in
theory, to be annotated for pinning. Do we have to make sure that
pinning is handled correctly, or can we just assume that QOM objects
are pinned? I am not sure I am ready to answer the question...

Paolo