Rust in QEMU update, April 2025

Rust in QEMU update, April 2025

[Paolo Bonzini]

It's been roughly three months since my previous update on the Rust in
QEMU project.  Support for Rust remains experimental, with most of the
past three months spent cleaning up the bindings and making more
functionality available from safe Rust.

As before, this mostly covers what I have looked at, which is making
it possible to write devices in safe Rust.  Topics such as QAPI and
async (block devices) are missing for this reason.

Overall, I'd say the progress is good: most of the missing features
mentioned in the previous update have been fixed or at least have a
plan for the next few months.

Table of contents
'''''''''''''''''

* Status in QEMU 10.0
* Build system
* Feature parity for devices
* Remaining unsafe code
* Rust version requirements
* A coding style for devices
* Next steps


Status in QEMU 10.0
'''''''''''''''''''

QEMU when built with ``--enable-rust`` compiles on all supported
build platforms.  It passes CI and ``make check-unit`` runs tests for
rust/qemu-api.  ``make check-qtests`` covers the Rust pl011 and HPET
device models, including migration of the former.  pl011 is entirely
implemented using safe code (minus migration and qdev properties).
HPET uses unsafe in some small and fairly well confined cases (see
below).

Since the previous update, some mistakes in the early bindings code
have become apparent; in particular, orphan rules made it too hard
to implement classes outside the qemu_api crate, and in general to
split the qemu_api crate in multiple parts---for example, parts that
are of interest to tools and parts that are only used by system
emulators.  Another important change is the separation between
bindgen-generated types and the structs that are actually used by
Rust code.  This allows traits such as Send, Sync or Zeroable to be
specified independently for C and Rust structs.

Thanks to Kevin Wolf's work on the block layer a new module appeared
to convert between C success/-errno conventions and ``io::Result``.
This module is also used in character device bindings.


Build system
''''''''''''

Developers can use ninja to easily access clippy, rustfmt and rustdoc.
Meson 1.8 supports clippy and rustdoc natively (including doctests),
but due to some regressions in 1.8.0 this will have to wait for the
next stable release.  This update to Meson will also make it possible
to use --enable-modules and --enable-rust together.

Rust is still not enabled and its presence is not checked for by
default.  The main reason is that Rust staticlibs also link statically
to the Rust standard library, thus bloating the resulting executable
(and making distros hate us as well).  A pending Meson pull request[1]
will fix this, as long as system/main.c is rewritten or wrapped in Rust.

.. [1] https://github.com/mesonbuild/meson/pull/14224


Feature parity for devices
''''''''''''''''''''''''''

Support for HPET live migration is ready to be merged.

As before, some recent pl011 commits are missing in the Rust version.

Logging and tracing were proposed as a project for Google Summer of
Code.


Remaining unsafe code
'''''''''''''''''''''

qdev bindings cover basic classes and interfaces, including
GPIO pins, timers, clocks and MemoryRegionOps.  VMState
still needs unsafe callbacks for pre_save/post_load, with
the final version waiting for a bump of the minimum supported
Rust version to 1.83.0.

Apart from VMState, the remaining instances of `unsafe` blocks in the
pl011 and HPET code can all be removed without bumping the language
version.

HPET does some very simple memory accesses; a good safe solution
for this may be the ``vm-memory`` crate.  While I have not looked into
using it, ``vm-memory`` and ``vm-virtio`` were written with QEMU's
use cases in mind.

The ``instance_init`` method is using unsafe code.  There are multiple
solutions to this: the one I planned for was to use a crate such as
`pin_init <https://docs.rs/pin_init/>`__ or
`pinned_init <https://docs.rs/pinned_init/>`__, but
I have also worked for self-education on a simpler version based on
``std::mem::MaybeUninit`` field projections.  This one removes ``unsafe``
only from the implementation and not from the ``instance_init`` method
itself, but it is less invasive and could be a possibility in the
short term.

The amount of functionality available from safe Rust is enough that
including new devices should be possible, even if they need some unsafe
code for parts of QEMU that do not have bindings yet.  Most devices
added to QEMU are simple and do not do any complex DMA; while such
simple devices have very little benefit from *rewriting* them in Rust,
there will be a substantial benefit to writing *new* devices in Rust as
soon as tracing and logging are supported.  Even though unsafe code in
migration and ``instance_init`` would count as technical debt for every
Rust device that is added to QEMU, I don't expect a flood of Rust devices
in the next few months such that this would be a problem.

There is still no interoperability between QEMU's C data structure and
Rust counterparts has no news either.  As before, we'll figure it out
as soon as we need a realize() implementation that can fail, or when
tackling QAPI.


Rust version requirements
'''''''''''''''''''''''''

Patches are on the list (and have mostly been reviewed) to bump the
minimum supported Rust version to 1.77.0.  However, there will probably
be at least one more bump to support references to statics in constants,
which are stable in 1.83.0 and are important for migration support in
safe Rust.

This will require dropping support for ``--enable-rust`` on Debian
bookworm with a distro-provided compiler.  If any devices are contributed
that are written in Rust and do not have a C counterpart, it may be
worth splitting "enable Rust" from "enable all devices written in Rust".
This way, the C versions of the pl011 and HPET devices remain available
on bookworm.


A coding style for devices
''''''''''''''''''''''''''

pl011 and HPET were developed independently and sometimes have different
idioms that could be unified.  Peter Maydell made several observations:

   Something I do notice is that there's some inconsistency in
   how we've structured things between the two devices, e.g.:

   * the pl011 main source file is device.rs, but the hpet one
     is hpet.rs

   * some places we use the actual names of bits in registers
     (eg Interrupt's OE, BE, etc consts), and some places we
     seem to have renamed them (e.g. pl011 Flags has clear_to_send
     not CTS, etc)

   * pl011 has defined named fields for its registers, but hpet does
     things like::

        self.config.get() & (1 << HPET_CFG_LEG_RT_SHIFT) != 0

   * pl011 has a split between PL011State and PL011Registers,
     but HPET does not. As I mentioned in an email thread a
     little while back, I feel like the State/Registers split
     is something we should either make a more clear deliberate
     formalised separation that's part of how we recommend
     device models should be designed

   [...]

   I think it would be good to figure out what we think is the
   right, best style, for writing this kind of thing, and be
   consistent. We have long standing problems in the C device
   models where there are multiple different styles for how
   we write them, and it would be good to try to aim for
   more uniformity on the Rust side.

One thing that I noticed is that in Rust QEMU code I tend to rely on
``const`` and ``static`` a lot, and several crates are not friendly
to this style, including the ``bilge`` crate that we use for named
fields and others such as ``bitflags``.  In both cases, this is
related to Rust not having const traits, e.g. for ``from()``/into()``
or operator overloading).  I already have a prototype of a bitflags-like
macro that is more const friendly, and we also need to make a decision
on whether to keep using ``bilge``, fork it, rewrite it or whatever.


Next steps
''''''''''

With respect to missing functionality, tracepoints and logging remain
the highest-priority missing feature, perhaps together with DMA, and the
main blocker before implementing new devices in Rust can be encouraged.
Hopefully this hole will be closed over the summer.

On the experimental side, if anybody wants to play with the ``vm-memory``
crate for DMA that would be very interesting.  However, the next steps
I am suggesting are mostly along the lines of cleaning up what is there,
ensuring that we're ready for more widespread usage of Rust in QEMU.

If someone like menial work, splitting the ``qemu_api`` crate is now
possible and a good thing to do.

If someone has good taste, they might go over the code with Peter's
above remarks in mind, cleaning up things so that pl011 and HPET both
provide good examples of Rust code in QEMU.

I also believe it's time to look again at using procedural macros to
simplify declaring QOM/qdev classes.  For example::

     #[derive(qemu_api_macros::Object(class_name="pl011", class=PL011Class))]
     #[derive(qemu_api_macros::Device(vmsd=VMSTATE_HPET))
     pub struct PL011State {
         pub parent_obj: ParentField<SysBusDevice>,
         pub iomem: MemoryRegion,
         pub regs: BqlRefCell<PL011Registers>,
         pub interrupts: [InterruptSource; IRQMASK.len()],
         pub clock: Owned<Clock>,

         #[qemu_api_macros::property(name="chr")]
         pub char_backend: CharBackend,

         #[qemu_api_macros::property(name="migrate-clk", default=true)]
         pub migrate_clock: bool,
     }

Related to this I have found recently the `attrs crate
<https://docs.rs/attrs/>`__, which provides an easy way to parse the
contents of attributes in a procedural macro.

Re: Rust in QEMU update, April 2025

[Manos Pitsidianakis]

On Fri, May 2, 2025 at 3:13 PM Paolo Bonzini <bonzini@gnu.org> wrote:
>
> It's been roughly three months since my previous update on the Rust in
> QEMU project.  Support for Rust remains experimental, with most of the
> past three months spent cleaning up the bindings and making more
> functionality available from safe Rust.
>
> As before, this mostly covers what I have looked at, which is making
> it possible to write devices in safe Rust.  Topics such as QAPI and
> async (block devices) are missing for this reason.
>
> Overall, I'd say the progress is good: most of the missing features
> mentioned in the previous update have been fixed or at least have a
> plan for the next few months.
>
> Table of contents
> '''''''''''''''''
>
> * Status in QEMU 10.0
> * Build system
> * Feature parity for devices
> * Remaining unsafe code
> * Rust version requirements
> * A coding style for devices
> * Next steps
>
>
> Status in QEMU 10.0
> '''''''''''''''''''
>
> QEMU when built with ``--enable-rust`` compiles on all supported
> build platforms.  It passes CI and ``make check-unit`` runs tests for
> rust/qemu-api.  ``make check-qtests`` covers the Rust pl011 and HPET
> device models, including migration of the former.  pl011 is entirely
> implemented using safe code (minus migration and qdev properties).
> HPET uses unsafe in some small and fairly well confined cases (see
> below).
>
> Since the previous update, some mistakes in the early bindings code
> have become apparent; in particular, orphan rules made it too hard
> to implement classes outside the qemu_api crate, and in general to
> split the qemu_api crate in multiple parts---for example, parts that
> are of interest to tools and parts that are only used by system
> emulators.  Another important change is the separation between
> bindgen-generated types and the structs that are actually used by
> Rust code.  This allows traits such as Send, Sync or Zeroable to be
> specified independently for C and Rust structs.
>
> Thanks to Kevin Wolf's work on the block layer a new module appeared
> to convert between C success/-errno conventions and ``io::Result``.
> This module is also used in character device bindings.
>
>
> Build system
> ''''''''''''
>
> Developers can use ninja to easily access clippy, rustfmt and rustdoc.
> Meson 1.8 supports clippy and rustdoc natively (including doctests),
> but due to some regressions in 1.8.0 this will have to wait for the
> next stable release.  This update to Meson will also make it possible
> to use --enable-modules and --enable-rust together.
>
> Rust is still not enabled and its presence is not checked for by
> default.  The main reason is that Rust staticlibs also link statically
> to the Rust standard library, thus bloating the resulting executable
> (and making distros hate us as well).  A pending Meson pull request[1]
> will fix this, as long as system/main.c is rewritten or wrapped in Rust.
>
> .. [1] https://github.com/mesonbuild/meson/pull/14224
>
>
> Feature parity for devices
> ''''''''''''''''''''''''''
>
> Support for HPET live migration is ready to be merged.
>
> As before, some recent pl011 commits are missing in the Rust version.
>
> Logging and tracing were proposed as a project for Google Summer of
> Code.
>
>
> Remaining unsafe code
> '''''''''''''''''''''
>
> qdev bindings cover basic classes and interfaces, including
> GPIO pins, timers, clocks and MemoryRegionOps.  VMState
> still needs unsafe callbacks for pre_save/post_load, with
> the final version waiting for a bump of the minimum supported
> Rust version to 1.83.0.
>
> Apart from VMState, the remaining instances of `unsafe` blocks in the
> pl011 and HPET code can all be removed without bumping the language
> version.
>
> HPET does some very simple memory accesses; a good safe solution
> for this may be the ``vm-memory`` crate.  While I have not looked into
> using it, ``vm-memory`` and ``vm-virtio`` were written with QEMU's
> use cases in mind.
>
> The ``instance_init`` method is using unsafe code.  There are multiple
> solutions to this: the one I planned for was to use a crate such as
> `pin_init <https://docs.rs/pin_init/>`__ or
> `pinned_init <https://docs.rs/pinned_init/>`__, but
> I have also worked for self-education on a simpler version based on
> ``std::mem::MaybeUninit`` field projections.  This one removes ``unsafe``
> only from the implementation and not from the ``instance_init`` method
> itself, but it is less invasive and could be a possibility in the
> short term.
>
> The amount of functionality available from safe Rust is enough that
> including new devices should be possible, even if they need some unsafe
> code for parts of QEMU that do not have bindings yet.  Most devices
> added to QEMU are simple and do not do any complex DMA; while such
> simple devices have very little benefit from *rewriting* them in Rust,
> there will be a substantial benefit to writing *new* devices in Rust as
> soon as tracing and logging are supported.  Even though unsafe code in
> migration and ``instance_init`` would count as technical debt for every
> Rust device that is added to QEMU, I don't expect a flood of Rust devices
> in the next few months such that this would be a problem.
>
> There is still no interoperability between QEMU's C data structure and
> Rust counterparts has no news either.  As before, we'll figure it out
> as soon as we need a realize() implementation that can fail, or when
> tackling QAPI.
>
>
> Rust version requirements
> '''''''''''''''''''''''''
>
> Patches are on the list (and have mostly been reviewed) to bump the
> minimum supported Rust version to 1.77.0.  However, there will probably
> be at least one more bump to support references to statics in constants,
> which are stable in 1.83.0 and are important for migration support in
> safe Rust.
>
> This will require dropping support for ``--enable-rust`` on Debian
> bookworm with a distro-provided compiler.  If any devices are contributed
> that are written in Rust and do not have a C counterpart, it may be
> worth splitting "enable Rust" from "enable all devices written in Rust".
> This way, the C versions of the pl011 and HPET devices remain available
> on bookworm.
>
>
> A coding style for devices
> ''''''''''''''''''''''''''
>
> pl011 and HPET were developed independently and sometimes have different
> idioms that could be unified.  Peter Maydell made several observations:
>
>    Something I do notice is that there's some inconsistency in
>    how we've structured things between the two devices, e.g.:
>
>    * the pl011 main source file is device.rs, but the hpet one
>      is hpet.rs
>
>    * some places we use the actual names of bits in registers
>      (eg Interrupt's OE, BE, etc consts), and some places we
>      seem to have renamed them (e.g. pl011 Flags has clear_to_send
>      not CTS, etc)
>
>    * pl011 has defined named fields for its registers, but hpet does
>      things like::
>
>         self.config.get() & (1 << HPET_CFG_LEG_RT_SHIFT) != 0
>
>    * pl011 has a split between PL011State and PL011Registers,
>      but HPET does not. As I mentioned in an email thread a
>      little while back, I feel like the State/Registers split
>      is something we should either make a more clear deliberate
>      formalised separation that's part of how we recommend
>      device models should be designed
>
>    [...]
>
>    I think it would be good to figure out what we think is the
>    right, best style, for writing this kind of thing, and be
>    consistent. We have long standing problems in the C device
>    models where there are multiple different styles for how
>    we write them, and it would be good to try to aim for
>    more uniformity on the Rust side.

The pl011 stuff was deliberate decisions:

- device.rs vs pl011.rs: the device was written as a crate, so it's
essentially its own library, plus pl011/src/pl011.rs would be
redundant :)
  That said, it's not important, we can choose either convention. I
like the less redundancy and separation of concerns: if pl011 gets
converted into a module in a future refactor, it could keep its
functionality split into different submodules and `pl011.rs` or
`pl011/mod.rs` would be the device module.
  Rust's concept of files being either a leaf module or parent module
does not translate to C's "every file is a compilation unit" cleanly.
- Using typed registers instead of constants: yes coming from C I can
understand it can feel unfamiliar. I specifically wanted to make the
register fields typed to avoid making the implementation a "C port",
and I think it's worthwhile to use the type system as much as
possible.
  A straight C port would result into integer constants with integer
typed fields everywhere for registers/flags.
  Yes, From/Into aren't const, at least yet, but it's not really a
hotpath performance wise. I think non-dynamically dispatched trait
methods can be inlined by annotating the `fn from(..)` impl with a
`#[inline(always)]` hint but I haven't confirmed this, just
speculation.
  Again, no strong opinions here. I like the "everything is typed"
approach and I think it's worth it to explore it because it allows us
to "make invalid/illegal states unrepresentable" as one sage article
goes.

>
> One thing that I noticed is that in Rust QEMU code I tend to rely on
> ``const`` and ``static`` a lot, and several crates are not friendly
> to this style, including the ``bilge`` crate that we use for named
> fields and others such as ``bitflags``.  In both cases, this is
> related to Rust not having const traits, e.g. for ``from()``/into()``
> or operator overloading).  I already have a prototype of a bitflags-like
> macro that is more const friendly, and we also need to make a decision
> on whether to keep using ``bilge``, fork it, rewrite it or whatever.
>
>
> Next steps
> ''''''''''
>
> With respect to missing functionality, tracepoints and logging remain
> the highest-priority missing feature, perhaps together with DMA, and the
> main blocker before implementing new devices in Rust can be encouraged.
> Hopefully this hole will be closed over the summer.
>
> On the experimental side, if anybody wants to play with the ``vm-memory``
> crate for DMA that would be very interesting.  However, the next steps
> I am suggesting are mostly along the lines of cleaning up what is there,
> ensuring that we're ready for more widespread usage of Rust in QEMU.
>
> If someone like menial work, splitting the ``qemu_api`` crate is now
> possible and a good thing to do.
>
> If someone has good taste, they might go over the code with Peter's
> above remarks in mind, cleaning up things so that pl011 and HPET both
> provide good examples of Rust code in QEMU.
>
> I also believe it's time to look again at using procedural macros to
> simplify declaring QOM/qdev classes.  For example::
>
>      #[derive(qemu_api_macros::Object(class_name="pl011", class=PL011Class))]
>      #[derive(qemu_api_macros::Device(vmsd=VMSTATE_HPET))
>      pub struct PL011State {
>          pub parent_obj: ParentField<SysBusDevice>,
>          pub iomem: MemoryRegion,
>          pub regs: BqlRefCell<PL011Registers>,
>          pub interrupts: [InterruptSource; IRQMASK.len()],
>          pub clock: Owned<Clock>,
>
>          #[qemu_api_macros::property(name="chr")]
>          pub char_backend: CharBackend,
>
>          #[qemu_api_macros::property(name="migrate-clk", default=true)]
>          pub migrate_clock: bool,
>      }
>
> Related to this I have found recently the `attrs crate
> <https://docs.rs/attrs/>`__, which provides an easy way to parse the
> contents of attributes in a procedural macro.

I actually have some WIP patches for this I put a pause on and can
continue e.g. https://gitlab.com/epilys/rust-for-qemu/-/commit/c2c97caaaf03273fabc14aee5a4d1499668ddbe3

Re: Rust in QEMU update, April 2025

[Paolo Bonzini]

On 5/5/25 14:26, Manos Pitsidianakis wrote:
>>     Something I do notice is that there's some inconsistency in
>>     how we've structured things between the two devices, e.g.:
>>
>>     * the pl011 main source file is device.rs, but the hpet one
>>       is hpet.rs
>>
>>     * some places we use the actual names of bits in registers
>>       (eg Interrupt's OE, BE, etc consts), and some places we
>>       seem to have renamed them (e.g. pl011 Flags has clear_to_send
>>       not CTS, etc)
>>
>>     * pl011 has defined named fields for its registers, but hpet does
>>       things like::
>>
>>          self.config.get() & (1 << HPET_CFG_LEG_RT_SHIFT) != 0
>>
>>     * pl011 has a split between PL011State and PL011Registers,
>>       but HPET does not. As I mentioned in an email thread a
>>       little while back, I feel like the State/Registers split
>>       is something we should either make a more clear deliberate
>>       formalised separation that's part of how we recommend
>>       device models should be designed
>>
>>     [...]
>>
>>     I think it would be good to figure out what we think is the
>>     right, best style, for writing this kind of thing, and be
>>     consistent. We have long standing problems in the C device
>>     models where there are multiple different styles for how
>>     we write them, and it would be good to try to aim for
>>     more uniformity on the Rust side.
> 
> The pl011 stuff was deliberate decisions:
> 
> - device.rs vs pl011.rs: the device was written as a crate, so it's
> essentially its own library, plus pl011/src/pl011.rs would be
> redundant :)

Right, I think Peter's comment was more about moving hpet.rs to 
device.rs, and merging PL011's device_class.rs into its device.rs.

>    That said, it's not important, we can choose either convention. I
> like the less redundancy and separation of concerns: if pl011 gets
> converted into a module in a future refactor, it could keep its
> functionality split into different submodules and `pl011.rs` or
> `pl011/mod.rs` would be the device module.

I think it's okay to decide that Rust devices will have mini 
directories: it's just the style of the language and Cargo more or less 
relies on having lib.rs.

In a vacuum I would prefer to have hw/char/pl011.rs for what is now 
rust/hw/char/pl011/lib.rs, and place the other files in hw/char/pl011; 
IIRC rustc accepts that style.  However we still rely on Cargo for some 
things(*), and as long as we do there's not much we can do about it.

     (*) notably "cargo fmt".  Everything else is more or less handled
         by Meson starting with 1.8.0.

> - Using typed registers instead of constants: yes coming from C I can
> understand it can feel unfamiliar. I specifically wanted to make the
> register fields typed to avoid making the implementation a "C port",
> and I think it's worthwhile to use the type system as much as
> possible.

Peter's comments (especially the second and third) were about two kinds 
of inconsistencies:

1) HPET not using bilge.  This was because Zhao looked at HPET from the 
opposite direction compared to what you did on pl011, namely avoiding 
unsafe and modeling the BQL properly, and sacrificed a bit the usage of 
idiomatic code like what bilge provides.

I think that you made the right choice for the first device and he made 
the right choice for the second device.  But now someone should look at 
HPET and do the work that you did to adopt bilge.

2) The choice between bilge on one side, and bitflags or integers on the 
other.  For pl011 you kept interrupt bits as integers for example, and 
this is related to the topic of (non-)availability of const in traits...

>    A straight C port would result into integer constants with integer
> typed fields everywhere for registers/flags.
>    Yes, From/Into aren't const, at least yet, but it's not really a
> hotpath performance wise. I think non-dynamically dispatched trait
> methods can be inlined by annotating the `fn from(..)` impl with a
> `#[inline(always)]` hint but I haven't confirmed this, just
> speculation.

It's not about hot paths, it's more that 1) you cannot use From/Into in 
a "static"'s initializer 2) bilge relies a lot on non-const methods in 
its internal implementation, which makes it quite messy to use it in 
some places.  See for example this thing for which I take all the blame:

     impl Data {
         // bilge is not very const-friendly, unfortunately
         pub const BREAK: Self = Self { value: 1 << 10 };
     }

and the same would be true of interrupt constants and the IRQMASK array.

The separate bilge and bitflags worlds are what bothers me the most in 
the experimental devices.  I can see why they would be very confusing 
for someone who's not had much experience with Rust, and therefore 
doesn't know *why* they are separate.

>    Again, no strong opinions here. I like the "everything is typed"
> approach and I think it's worth it to explore it because it allows us
> to "make invalid/illegal states unrepresentable" as one sage article
> goes.

I agree, and it's why I think you made the right choice using it for 
pl011.  With all the unsafe code that you had to use, strong-typing at 
least showed *something* that Rust could provide compared to C(*).  And 
I like the strong typing too, even if I'm not sure I like bilge's 
*implementation* that much anymore.

I'm not really up for rewriting it, but then I've also done more stupid 
rewrites in the past. :)

    (*) when we were discussing safety vs. unsafety last summer, I may
        have sounded dismissive of this kind of benefit.  My point at the
        time was that unsafe code was so much more complex than C, that
        the benefit of strong-typing wasn't enough to *offset* the
        complexity of unsafe code.  But it is absolutely present.

>> Related to this I have found recently the `attrs crate
>> <https://docs.rs/attrs/>`__, which provides an easy way to parse the
>> contents of attributes in a procedural macro.
> 
> I actually have some WIP patches for this I put a pause on and can
> continue e.g. https://gitlab.com/epilys/rust-for-qemu/-/commit/c2c97caaaf03273fabc14aee5a4d1499668ddbe3

The repository is private, but I look forward to seeing it!  If you want 
to post an RFC without even any code, just to show what the device code 
looks like, that would be helpful as it will catch stuff like lack of 
type safety.

BTW, if you need it to model reflection better I think it is acceptable 
to assume const_refs_to_static is present.

Paolo

Re: Rust in QEMU update, April 2025

[Peter Maydell]

On Mon, 5 May 2025 at 14:45, Paolo Bonzini <bonzini@gnu.org> wrote:
>
> On 5/5/25 14:26, Manos Pitsidianakis wrote:
> >>     Something I do notice is that there's some inconsistency in
> >>     how we've structured things between the two devices, e.g.:
> >>
> >>     * the pl011 main source file is device.rs, but the hpet one
> >>       is hpet.rs


> >>     I think it would be good to figure out what we think is the
> >>     right, best style, for writing this kind of thing, and be
> >>     consistent. We have long standing problems in the C device
> >>     models where there are multiple different styles for how
> >>     we write them, and it would be good to try to aim for
> >>     more uniformity on the Rust side.
> >
> > The pl011 stuff was deliberate decisions:
> >
> > - device.rs vs pl011.rs: the device was written as a crate, so it's
> > essentially its own library, plus pl011/src/pl011.rs would be
> > redundant :)
>
> Right, I think Peter's comment was more about moving hpet.rs to
> device.rs, and merging PL011's device_class.rs into its device.rs.

Well, I don't actually have an opinion about which is better: I don't
know enough Rust to have a sense of what's more idiomatic or otherwise
preferable. My point is the more general one, that we should decide
(in all of these cases) which approach is going to work better for us
and apply that consistently, now that we have the benefit of having
written a couple of device models so we can see what each path looks like.

These initial devices are going to be the models that other people
(perhaps less familiar with Rust) are going to use as patterns when
they write other device models. Converging on a consistent structure
and way of writing devices now will help those future device authors
(including me!), I think.

-- PMM

Re: Rust in QEMU update, April 2025

[Manos Pitsidianakis]

On Mon, May 5, 2025 at 4:45 PM Paolo Bonzini <bonzini@gnu.org> wrote:
>
> On 5/5/25 14:26, Manos Pitsidianakis wrote:
> >>     Something I do notice is that there's some inconsistency in
> >>     how we've structured things between the two devices, e.g.:
> >>
> >>     * the pl011 main source file is device.rs, but the hpet one
> >>       is hpet.rs
> >>
> >>     * some places we use the actual names of bits in registers
> >>       (eg Interrupt's OE, BE, etc consts), and some places we
> >>       seem to have renamed them (e.g. pl011 Flags has clear_to_send
> >>       not CTS, etc)
> >>
> >>     * pl011 has defined named fields for its registers, but hpet does
> >>       things like::
> >>
> >>          self.config.get() & (1 << HPET_CFG_LEG_RT_SHIFT) != 0
> >>
> >>     * pl011 has a split between PL011State and PL011Registers,
> >>       but HPET does not. As I mentioned in an email thread a
> >>       little while back, I feel like the State/Registers split
> >>       is something we should either make a more clear deliberate
> >>       formalised separation that's part of how we recommend
> >>       device models should be designed
> >>
> >>     [...]
> >>
> >>     I think it would be good to figure out what we think is the
> >>     right, best style, for writing this kind of thing, and be
> >>     consistent. We have long standing problems in the C device
> >>     models where there are multiple different styles for how
> >>     we write them, and it would be good to try to aim for
> >>     more uniformity on the Rust side.
> >
> > The pl011 stuff was deliberate decisions:
> >
> > - device.rs vs pl011.rs: the device was written as a crate, so it's
> > essentially its own library, plus pl011/src/pl011.rs would be
> > redundant :)
>
> Right, I think Peter's comment was more about moving hpet.rs to
> device.rs, and merging PL011's device_class.rs into its device.rs.
>
> >    That said, it's not important, we can choose either convention. I
> > like the less redundancy and separation of concerns: if pl011 gets
> > converted into a module in a future refactor, it could keep its
> > functionality split into different submodules and `pl011.rs` or
> > `pl011/mod.rs` would be the device module.
>
> I think it's okay to decide that Rust devices will have mini
> directories: it's just the style of the language and Cargo more or less
> relies on having lib.rs.
>
> In a vacuum I would prefer to have hw/char/pl011.rs for what is now
> rust/hw/char/pl011/lib.rs, and place the other files in hw/char/pl011;
> IIRC rustc accepts that style.  However we still rely on Cargo for some
> things(*), and as long as we do there's not much we can do about it.
>
>      (*) notably "cargo fmt".  Everything else is more or less handled
>          by Meson starting with 1.8.0.
>
> > - Using typed registers instead of constants: yes coming from C I can
> > understand it can feel unfamiliar. I specifically wanted to make the
> > register fields typed to avoid making the implementation a "C port",
> > and I think it's worthwhile to use the type system as much as
> > possible.
>
> Peter's comments (especially the second and third) were about two kinds
> of inconsistencies:
>
> 1) HPET not using bilge.  This was because Zhao looked at HPET from the
> opposite direction compared to what you did on pl011, namely avoiding
> unsafe and modeling the BQL properly, and sacrificed a bit the usage of
> idiomatic code like what bilge provides.
>
> I think that you made the right choice for the first device and he made
> the right choice for the second device.  But now someone should look at
> HPET and do the work that you did to adopt bilge.
>
> 2) The choice between bilge on one side, and bitflags or integers on the
> other.  For pl011 you kept interrupt bits as integers for example, and
> this is related to the topic of (non-)availability of const in traits...

Oh right.

>
> >    A straight C port would result into integer constants with integer
> > typed fields everywhere for registers/flags.
> >    Yes, From/Into aren't const, at least yet, but it's not really a
> > hotpath performance wise. I think non-dynamically dispatched trait
> > methods can be inlined by annotating the `fn from(..)` impl with a
> > `#[inline(always)]` hint but I haven't confirmed this, just
> > speculation.
>
> It's not about hot paths, it's more that 1) you cannot use From/Into in
> a "static"'s initializer 2) bilge relies a lot on non-const methods in
> its internal implementation, which makes it quite messy to use it in
> some places.  See for example this thing for which I take all the blame:

Yeah it's not nice that we can't use it in static/const initializers.

>
>      impl Data {
>          // bilge is not very const-friendly, unfortunately
>          pub const BREAK: Self = Self { value: 1 << 10 };
>      }
>
> and the same would be true of interrupt constants and the IRQMASK array.
>
> The separate bilge and bitflags worlds are what bothers me the most in
> the experimental devices.  I can see why they would be very confusing
> for someone who's not had much experience with Rust, and therefore
> doesn't know *why* they are separate.
>
> >    Again, no strong opinions here. I like the "everything is typed"
> > approach and I think it's worth it to explore it because it allows us
> > to "make invalid/illegal states unrepresentable" as one sage article
> > goes.
>
> I agree, and it's why I think you made the right choice using it for
> pl011.  With all the unsafe code that you had to use, strong-typing at
> least showed *something* that Rust could provide compared to C(*).  And
> I like the strong typing too, even if I'm not sure I like bilge's
> *implementation* that much anymore.

It has its pros and cons that's for sure... there are many crates that
let you define typed bitfields, I haven't looked into the current
state of the art lately. We should definitely move on to something
better if it exists now or in the future.

> I'm not really up for rewriting it, but then I've also done more stupid
> rewrites in the past. :)
>
>     (*) when we were discussing safety vs. unsafety last summer, I may
>         have sounded dismissive of this kind of benefit.  My point at the
>         time was that unsafe code was so much more complex than C, that
>         the benefit of strong-typing wasn't enough to *offset* the
>         complexity of unsafe code.  But it is absolutely present.
>
> >> Related to this I have found recently the `attrs crate
> >> <https://docs.rs/attrs/>`__, which provides an easy way to parse the
> >> contents of attributes in a procedural macro.
> >
> > I actually have some WIP patches for this I put a pause on and can
> > continue e.g. https://gitlab.com/epilys/rust-for-qemu/-/commit/c2c97caaaf03273fabc14aee5a4d1499668ddbe3
>
> The repository is private, but I look forward to seeing it!  If you want
> to post an RFC without even any code, just to show what the device code
> looks like, that would be helpful as it will catch stuff like lack of
> type safety.

Oops, I forgot I had archived it because I moved development to
https://gitlab.com/epilys/qemu . I made it public again. Bear in mind
this was a WIP, basically my git stash but committed.

>
> BTW, if you need it to model reflection better I think it is acceptable
> to assume const_refs_to_static is present.
>
> Paolo

Re: Rust in QEMU update, April 2025

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1733 bytes --]

Il mar 6 mag 2025, 04:41 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
ha scritto:

> > It's not about hot paths, it's more that 1) you cannot use From/Into in
> > a "static"'s initializer 2) bilge relies a lot on non-const methods in
> > its internal implementation, which makes it quite messy to use it in
> > some places.  See for example this thing for which I take all the blame:
>
> Yeah it's not nice that we can't use it in static/const initializers. [...]

It has its pros and cons that's for sure... there are many crates that
> let you define typed bitfields, I haven't looked into the current
> state of the art lately. We should definitely move on to something
> better if it exists now or in the future.
>

I tried using bitfield-struct. The definitions are a bit less polished than
bilge but usage is the same, it is const friendly and has fewer
dependencies. I don't think there's one that's clearly better, but we can
look at the tradeoffs and decide.

To streamline expressions on bit flags maybe we could have a macro like
bits!(Interrupt: RX | TX) which expands to
Interrupt.RX.union(Interrupt.TX), where union() is a const function
(likewise for & or !). This should not be hard to write as a procedural
macro, it's just a recursive descent parser.

Oops, I forgot I had archived it because I moved development to
> https://gitlab.com/epilys/qemu . I made it public again. Bear in mind
> this was a WIP, basically my git stash but committed.
>

Understood. The nice thing to have would be to automatically derive the
PropertyInfo from the type of the field.

Paolo

> BTW, if you need it to model reflection better I think it is acceptable
> > to assume const_refs_to_static is present.
> >
> > Paolo
>
>

[-- Attachment #2: Type: text/html, Size: 3063 bytes --]

Re: Rust in QEMU update, April 2025

[Zhao Liu]

(Resend as the previous email may have failed.)

> Remaining unsafe code
> '''''''''''''''''''''
> 
> qdev bindings cover basic classes and interfaces, including
> GPIO pins, timers, clocks and MemoryRegionOps.  VMState
> still needs unsafe callbacks for pre_save/post_load, with
> the final version waiting for a bump of the minimum supported
> Rust version to 1.83.0.
> 
> Apart from VMState, the remaining instances of `unsafe` blocks in the
> pl011 and HPET code can all be removed without bumping the language
> version.
> 
> HPET does some very simple memory accesses; a good safe solution
> for this may be the ``vm-memory`` crate.  While I have not looked into
> using it, ``vm-memory`` and ``vm-virtio`` were written with QEMU's
> use cases in mind.

I'm working on this and trying to wrap simple memory access by
vm-memory.
 
> A coding style for devices
> ''''''''''''''''''''''''''
> 
> pl011 and HPET were developed independently and sometimes have different
> idioms that could be unified.  Peter Maydell made several observations:
> 
>   Something I do notice is that there's some inconsistency in
>   how we've structured things between the two devices, e.g.:

...
 
>   * pl011 has defined named fields for its registers, but hpet does
>     things like::
> 
>        self.config.get() & (1 << HPET_CFG_LEG_RT_SHIFT) != 0

On the one hand, this way is more friendly and easy to review for C
developers (comparing with the C version), and on the other hand,
there are similar writeups in other projects (crosvm/firecracker). So,
HPET also shows a clumsy and conservative approach :-).

>   * pl011 has a split between PL011State and PL011Registers,
>     but HPET does not. As I mentioned in an email thread a
>     little while back, I feel like the State/Registers split
>     is something we should either make a more clear deliberate
>     formalised separation that's part of how we recommend
>     device models should be designed

Sorry, I just noticed this point.

I tried to abstract a HPETRegisters but found out it didn't work well in
HPET case...So I didn't insist.

The following is the lesson I learned when I wrote HPET in Rust in the
first time (copied from my HPET v2 cover letter [*]):

Additional Experience
=====================

PL011 provides a pattern to group all registers in one BqlRefCell
instead of multiple BqlCells.

I also tried to leverage this design to HPET, but it didn't fit very
well with this issue:
 * HPETState abstracts many helpers to check register bit and tell
   caller about the state, e.g., is_legacy_mode(),
   is_timer_int_active(), etc.
   
   But this also means these helpers can't be used in BqlRefCell::
   borrow_mut() context again, otherwise, they would cause the runtime
   bql check panic.

 - Some cases are easy to fix, i.e, use borrow_mut to modify the
   registers after the helpers' call.

 - Some cases would by tricky, like memory write callback, since it has
   complex logic and it's hard to decouple register changes from the
   reset logic as clearly as PL011 does. 

   The fix for this case is either to avoid using register helpers
   again in the borrow_mut context of write(), or to use BqlCell
   instead of BqlRefCell to get finer-grained access control to avoid
   refactoring code logic.

   I chose the latter.


So I think this might be a practical lesson that the choice of BqlCell
and BqlRefCell is also indeed related to code logic: If the code logic
is too complex to decouple borrow() and borrow_mut() (and the compiler
doesn't check this, only the runtime's panic_already_borrowed() will
complains!) , then BqlCell should be chosen. Because fine-grained access
is easier to control and avoid errors. :-)

[*]: https://lore.kernel.org/qemu-devel/20250125125137.1223277-1-zhao1.liu@intel.com/

Thanks,
Zhao

Re: Rust in QEMU update, April 2025

[Paolo Bonzini]

On 5/20/25 18:23, Zhao Liu wrote:
>> HPET does some very simple memory accesses; a good safe solution
>> for this may be the ``vm-memory`` crate.  While I have not looked into
>> using it, ``vm-memory`` and ``vm-virtio`` were written with QEMU's
>> use cases in mind.
> I'm working on this and trying to wrap simple memory access by
> vm-memory.

Ok.  Note that while the GuestAddressSpace corresponds QEMU's 
AddressSpace (so far so good :)), QEMU's MemoryRegion is completely 
unrelated to vm-memory's GuestMemoryRegion.  That's because vm-memory 
only operates on an array of non-overlapping regions, like QEMU's 
FlatRange or MemoryRegionSection structs.


The GuestMemory (GuestAddressSpace::M) corresponds to QEMU's FlatView. 
Indeed the functions in the trait match with what you expect of a FlatView:

     fn num_regions(&self) -> usize;
     fn find_region(&self, addr: GuestAddress) -> Option<&Self::R>;
     fn iter(&self) -> impl Iterator<Item = &Self::R>;

If the GuestMemory is a FlatView, the GuestAddressSpace::T, implements 
Clone + Deref<Target = FlatView>.  It's not too hard to see that 
GuestAddressSpace's memory() method must call 
address_space_get_flatview() and the GuestAddressSpace::T's drop method 
must call flatview_unref().  Let's call this (Rust-specific) struct 
FlatViewRefGuard, or something like that.


Going back to the GuestMemoryRegion (<FlatView as GuestMemory>::R), it 
could be either a QEMU FlatRange or a MemoryRegionSection.  Neither are 
good options.  Without a MemoryRegionSection you can't support IOMMU 
regions; but flatview_do_translate() returns the MemoryRegionSection by 
value, and GuestMemory's

     fn find_region(&self, addr: GuestAddress) -> Option<&Self::R>;

wants a reference instead!

Anyhow, all three types (AddressSpace, FlatView, FlatRange) are better 
wrapped with Opaque.

Looking more at FlatRange, these are easy:

     // Required methods
     fn len(&self) -> GuestUsize;
     fn start_addr(&self) -> GuestAddress;

But this one is another problem:

     fn bitmap(&self) -> &Self::B;

because it returns the "Bitmap" by reference.  QEMU's bitmap is a global 
variable indexed by ram_addr_t.  It would be better if this was declared 
like this:

    fn bitmap(&'a self) ->
       <Self::B as WithBitmapSlice<'a>>::S

I have no idea if this can be changed in upstream vm-virtio.  For now 
maybe you can leave it as ().  That's buggy but it's ok for a proof of 
concept.

So... not sure what to do there.  It seems like vm-memory is very close 
to being usable by QEMU, but maybe not completely. :(

Paolo

Re: Rust in QEMU update, April 2025

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

Il mer 21 mag 2025, 10:21 Zhao Liu <zhao1.liu@intel.com> ha scritto:

> I also realize that once FlatRange/FlatView is associated with
> GuestMemoryRegion/
> GuestMemory, it changes the usual practice in QEMU, where most memory
> operations
> are built around MemoryRegion/AddressSpace.
>

That shouldn't be a problem. In QEMU and vm-memory DMA always starts from
Address space/GuestAddressSpace, not from MemoryRegion, so if QEMU
implements GuestAddressSpace in qemu_api::AddressSpace everything matches
well. The only difference is that Rust code will do something like

  AddressSpace::MEMORY::memory().read(...)

(which retrieves the FlatView) instead of

  address_space_read(&address_space_memory, ...)

But that's just how the API is defined. It seems good to me. The mismatch
between MemoryRegion and GuestMemoryRegion is confusing, but will be mostly
hidden behind the prelude because Guest* are traits not structs.

> So... not sure what to do there.  It seems like vm-memory is very close to
> > being usable by QEMU, but maybe not completely. :(
>
> Is it possible or necessary for vm-memory to support overlap? Because I
> feel that if it is possible, the problem might be simplified. (As a
> beginner, I have yet to understand exactly how difficult it is.)
>

I don't think that's necessary. Just like in QEMU C code we have
AddressSpace for DMA and MemoryRegion for hierarchy, in Rust code you have
qemu_api::{AddressSpace,MemoryRegion}. FlatView, FlatRange,
MemoryRegionSection are hidden in both cases, and users don't care much
about which type implements GuestMemoryRegion because all they see is
AddressSpace. Again, it's all hidden behind the prelude.

The real problem is how hard it is to remove the references from the
vm-memory API... Maybe not much.

Paolo


> Thanks,
> Zhao
>
>
>

[-- Attachment #2: Type: text/html, Size: 2982 bytes --]

Re: Rust in QEMU update, April 2025

[Zhao Liu]

On Tue, May 20, 2025 at 07:48:11PM +0200, Paolo Bonzini wrote:
> Date: Tue, 20 May 2025 19:48:11 +0200
> From: Paolo Bonzini <bonzini@gnu.org>
> Subject: Re: Rust in QEMU update, April 2025
> 
> On 5/20/25 18:23, Zhao Liu wrote:
> > > HPET does some very simple memory accesses; a good safe solution
> > > for this may be the ``vm-memory`` crate.  While I have not looked into
> > > using it, ``vm-memory`` and ``vm-virtio`` were written with QEMU's
> > > use cases in mind.
> > I'm working on this and trying to wrap simple memory access by
> > vm-memory.
> 
> Ok.  Note that while the GuestAddressSpace corresponds QEMU's AddressSpace
> (so far so good :)), QEMU's MemoryRegion is completely unrelated to
> vm-memory's GuestMemoryRegion.  That's because vm-memory only operates on an
> array of non-overlapping regions, like QEMU's FlatRange or
> MemoryRegionSection structs.
> 
> The GuestMemory (GuestAddressSpace::M) corresponds to QEMU's FlatView.
> Indeed the functions in the trait match with what you expect of a FlatView:
> 
>     fn num_regions(&self) -> usize;
>     fn find_region(&self, addr: GuestAddress) -> Option<&Self::R>;
>     fn iter(&self) -> impl Iterator<Item = &Self::R>;

This is indeed exactly what I meet now. GuestMemory and QEMU's design don't
macth well! I've learned from the lessons of failure that MemoryRegion is
different from GuestMemoryRegion. Thank you for pointing the direction and
helping me decide to switch to FlatView.

> If the GuestMemory is a FlatView, the GuestAddressSpace::T, implements Clone
> + Deref<Target = FlatView>.  It's not too hard to see that
> GuestAddressSpace's memory() method must call address_space_get_flatview()
> and the GuestAddressSpace::T's drop method must call flatview_unref().
> Let's call this (Rust-specific) struct FlatViewRefGuard, or something like
> that.

I also realize that once FlatRange/FlatView is associated with GuestMemoryRegion/
GuestMemory, it changes the usual practice in QEMU, where most memory operations
are built around MemoryRegion/AddressSpace.

But that's okay because what users aware is the vm-memory interface.

> Going back to the GuestMemoryRegion (<FlatView as GuestMemory>::R), it could
> be either a QEMU FlatRange or a MemoryRegionSection.  Neither are good
> options.  Without a MemoryRegionSection you can't support IOMMU regions; but
> flatview_do_translate() returns the MemoryRegionSection by value, and
> GuestMemory's
> 
>     fn find_region(&self, addr: GuestAddress) -> Option<&Self::R>;
> 
> wants a reference instead!

Yes, there will be many gaps.

> Anyhow, all three types (AddressSpace, FlatView, FlatRange) are better
> wrapped with Opaque.
> 
> Looking more at FlatRange, these are easy:
> 
>     // Required methods
>     fn len(&self) -> GuestUsize;
>     fn start_addr(&self) -> GuestAddress;
> 
> But this one is another problem:
> 
>     fn bitmap(&self) -> &Self::B;
> 
> because it returns the "Bitmap" by reference.  QEMU's bitmap is a global
> variable indexed by ram_addr_t.  It would be better if this was declared
> like this:
> 
>    fn bitmap(&'a self) ->
>       <Self::B as WithBitmapSlice<'a>>::S
> 
> I have no idea if this can be changed in upstream vm-virtio.  For now maybe
> you can leave it as ().  That's buggy but it's ok for a proof of concept.

Yes, the bitmap is also a problem for me, and I also haven't figured out
how to align it with rust-vmm. In fact, in the code (that currently doesn't
run), I indeed used `()`. :-(

> So... not sure what to do there.  It seems like vm-memory is very close to
> being usable by QEMU, but maybe not completely. :(

Is it possible or necessary for vm-memory to support overlap? Because I
feel that if it is possible, the problem might be simplified. (As a
beginner, I have yet to understand exactly how difficult it is.)

Thanks,
Zhao

Re: Rust in QEMU update, April 2025

[Manos Pitsidianakis]

On Wed, May 21, 2025 at 11:36 AM Paolo Bonzini <bonzini@gnu.org> wrote:
>
>
>
> Il mer 21 mag 2025, 10:21 Zhao Liu <zhao1.liu@intel.com> ha scritto:
>>
>> I also realize that once FlatRange/FlatView is associated with GuestMemoryRegion/
>> GuestMemory, it changes the usual practice in QEMU, where most memory operations
>> are built around MemoryRegion/AddressSpace.
>
>
> That shouldn't be a problem. In QEMU and vm-memory DMA always starts from Address space/GuestAddressSpace, not from MemoryRegion, so if QEMU implements GuestAddressSpace in qemu_api::AddressSpace everything matches well. The only difference is that Rust code will do something like
>
>   AddressSpace::MEMORY::memory().read(...)
>
> (which retrieves the FlatView) instead of
>
>   address_space_read(&address_space_memory, ...)
>
> But that's just how the API is defined. It seems good to me. The mismatch between MemoryRegion and GuestMemoryRegion is confusing, but will be mostly hidden behind the prelude because Guest* are traits not structs.
>
>> > So... not sure what to do there.  It seems like vm-memory is very close to
>> > being usable by QEMU, but maybe not completely. :(
>>
>> Is it possible or necessary for vm-memory to support overlap? Because I
>> feel that if it is possible, the problem might be simplified. (As a
>> beginner, I have yet to understand exactly how difficult it is.)
>
>
> I don't think that's necessary. Just like in QEMU C code we have AddressSpace for DMA and MemoryRegion for hierarchy, in Rust code you have qemu_api::{AddressSpace,MemoryRegion}. FlatView, FlatRange, MemoryRegionSection are hidden in both cases, and users don't care much about which type implements GuestMemoryRegion because all they see is AddressSpace. Again, it's all hidden behind the prelude.
>
> The real problem is how hard it is to remove the references from the vm-memory API... Maybe not much.
>
> Paolo
>
>>
>> Thanks,
>> Zhao
>>
>>

vm-memory is a very rigid API unfortunately. It's excellent for
rust-vmm purposes. I presume it's possible to figure out a clever
solution to satisfy both rust-vmm and QEMU use needs but I'm not sure
it's worth it. It's really hard to retrofit other projects into
vm-memory if they don't use rust-vmm crates API design and it might
make both rust-vmm code and QEMU code more complex. QEMU would depend
on rust-vmm architectural decisions and vice-versa. The thing I'm
fearing most is needing to refactor memory APIs in QEMU in the future
and turn the vm-memory dependency into technical debt.

Perhaps it's more sensible to not use external dependencies to wrap
over our APIs but we can surely design our Rust bindings inspired by
them. I think it's an inescapable consequence of QEMU's internals
being fluid over time and "private"/unstable.

Personal anecdote: I tried using vm-memory on a personal TCG-like
emulator I am writing for fun, and I found it a daunting task as new
rust-vmm concepts appeared into my codebase as "scope creep". And I
wasn't even adapting an existing API to vm-memory, but designing a new
one based on it. I gave it up after a few days.

-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

Re: Rust in QEMU update, April 2025

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1634 bytes --]

Il mer 21 mag 2025, 11:35 Manos Pitsidianakis <
manos.pitsidianakis@linaro.org> ha scritto:

> vm-memory is a very rigid API unfortunately. It's excellent for
> rust-vmm purposes. I presume it's possible to figure out a clever
> solution to satisfy both rust-vmm and QEMU use needs but I'm not sure
> it's worth it. It's really hard to retrofit other projects into
> vm-memory if they don't use rust-vmm crates API design and it might
> make both rust-vmm code and QEMU code more complex. QEMU would depend
> on rust-vmm architectural decisions and vice-versa. The thing I'm
> fearing most is needing to refactor memory APIs in QEMU in the future
> and turn the vm-memory dependency into technical debt.


I agree that changing the memory API for the sake of Rust code is a bad
idea.

That said, I have some hope. vm-memory's GuestAddressSpace was designed for
cloud-hypervisor but with QEMU in mind and I was really happy about it. Of
course it's easy to screw up and miss details elsewhere (like the
difference between FlatRange * and MemoryRegionSection, and consequently
the type of the GuestMemory iterator); I am not even sure what QEMU code
for IOMMUs looked like in 2018. But it might be salvageable if the rust-vmm
guys accept a couple of small API breaks.

Perhaps it's more sensible to not use external dependencies to wrap
> over our APIs but we can surely design our Rust bindings inspired by
> them.
>

It would be nice to have crates like linux-loader or vm-virtio available.
But even then, even if we can fork the basic vm-memory traits, we can get
by with it as long as those handy crates still compile. :)

Paolo

>

[-- Attachment #2: Type: text/html, Size: 2597 bytes --]

Re: Rust in QEMU update, April 2025

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

Il mer 21 mag 2025, 10:37 Paolo Bonzini <bonzini@gnu.org> ha scritto:

> > So... not sure what to do there.  It seems like vm-memory is very close
> to
>
>> > being usable by QEMU, but maybe not completely. :(
>>
>> Is it possible or necessary for vm-memory to support overlap? Because I
>> feel that if it is possible, the problem might be simplified. (As a
>> beginner, I have yet to understand exactly how difficult it is.)
>>
>
> I don't think that's necessary. Just like in QEMU C code we have
> AddressSpace for DMA and MemoryRegion for hierarchy, in Rust code you have
> qemu_api::{AddressSpace,MemoryRegion}. FlatView, FlatRange,
> MemoryRegionSection are hidden in both cases, and users don't care much
> about which type implements GuestMemoryRegion because all they see is
> AddressSpace. Again, it's all hidden behind the prelude.
>
> The real problem is how hard it is to remove the references from the
> vm-memory API... Maybe not much.
>

Brain dump ahead! AddressSpaceDispatch is already storing
MemoryRegionSections. Therefore it should be possible to make
GuestMemory::R equal to MemoryRegionSection, or rather its Opaque wrapper.

Then one needs to implement Bytes<MemoryRegionAddress> in
MemoryRegionSection like
https://github.com/rust-vmm/vm-memory/blob/3f2fd80b11/src/mmap/mod.rs#L171,
use address_space_lookup_region() to implement find_region(), and (except
for IOMMU) everything should work.

To implement IOMMUs later, it's probably possible to call
flatview_translate() from the try_access() method, which is intended to be
internal but comes in handy here. try_access() is a bit complicated, but
we'll just have to copy some code from the default implementation at
https://github.com/rust-vmm/vm-memory/blob/main/src/guest_memory.rs, in
order to replace the call to find_region(). Notice how
address_space_lookup_region() returns MemoryRegionSection*, which
translates to Option<&Self::R>; while flatview_translate() returns a
MemoryRegionSection by value, that can become a local variable in
try_access() and everything is fine. Hopefully. The compiler will certainly
prove me wrong.

Another thing that will be needed later is support for MemTxAttrs. Maybe
squeeze those into the address, i.e. implement Bytes<(GuestAddress,
MemTxAttrs)> and Bytes<(MemoryRegionAddress, MemTxAttrs)>?

The dirty bitmap does need some changes in vm-memory, but it's pretty small
compared to the mess that I foreshadowed in order to change find_region().

Paolo

[-- Attachment #2: Type: text/html, Size: 3737 bytes --]