Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Tue, 11 Jun 2024 12:45, Zhao Liu <zhao1.liu@intel.com> wrote:
> On Tue, Jun 11, 2024 at 09:22:44AM +0100, Daniel P. Berrangé wrote:
> > On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
> > > Hello everyone,
> > > 
> > > This is an early draft of my work on implementing a very simple device, 
> > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c 
> > > and is used in hw/arm/virt.c).
> >
> > looking at the diffstat:
> >
> > >  .gitignore                     |   2 +
> > >  .gitlab-ci.d/buildtest.yml     |  64 ++--
> > >  configure                      |  12 +
> > >  hw/arm/virt.c                  |   2 +-
> > >  meson.build                    |  99 ++++++
> > >  meson_options.txt              |   4 +
> > >  rust/meson.build               |  93 ++++++
> > >  rust/pl011/.cargo/config.toml  |   2 +
> > >  rust/pl011/.gitignore          |   2 +
> > >  rust/pl011/Cargo.lock          | 120 +++++++
> > >  rust/pl011/Cargo.toml          |  26 ++
> > >  rust/pl011/README.md           |  42 +++
> > >  rust/pl011/build.rs            |  44 +++
> > >  rust/pl011/meson.build         |   7 +
> > >  rust/pl011/rustfmt.toml        |  10 +
> > >  rust/pl011/src/definitions.rs  |  95 ++++++
> > >  rust/pl011/src/device.rs       | 531 ++++++++++++++++++++++++++++++
> > >  rust/pl011/src/device_class.rs |  95 ++++++
> > >  rust/pl011/src/generated.rs    |   5 +
> > >  rust/pl011/src/lib.rs          | 575 +++++++++++++++++++++++++++++++++
> > >  rust/pl011/src/memory_ops.rs   |  38 +++
> >
> > My thought is that if we're going to start implementing devices
> > or other parts of QEMU, in Rust, then I do not want to see it
> > placed in a completely separate directory sub-tree.
> >
> > In this example, I would expect to have hw/arm/pl011.rs, or hw/arm/pl011/*.rs
> > so that the device is part of the normal Arm hardware directory structure 
> > and maintainer assignments.
>
> It has its advantages. Otherwise, as the number of Rust implementations
> grows, the same mirror directory as QEMU will have to be rebuilt again
> in the Rust directory.

Offtopic for this RFC but:

It'd also mean that each crate would have its own subdir in the tree. In 
the future free-standing .rs files like in the kernel would be nice to 
have.

For those who are not familiar with Cargo, a cargo library is a single 
compilation unit. You cannot have interspersed .rs files and compile 
with cargo as one does generally. You'd have to generate the rustc 
commands for building.

> Further, putting C implementations in the same directory, there is again
> the question of why it needs to be duplicated :-) . This topic is
> probably also beyond the scope of this RFC, but it's nice to have a Rust
> example to start with.

pl011 was suggested by Peter as a very simple device to model. The 
duplication is not meant to replace the C version for now or at the 
foreseeable future. It's more of a reference implementation.

> Currently, pl011 exclusively occupies a cargo as a package. In the
> future, will other Rust implementations utilize the workspace mechanism
> to act as a second package in the same cargo? Or will new cargo be created
> again?

What do you mean by "new cargo"? I didn't catch that :(

A workspace would make sense if we have "general" crate libraries that 
hardware crates depend on.

> Under a unified Rust directory, using a workspace to manage multiple
> packages looks as if it would be easier to maintain. Decentralized to an
> existing directory, they're all separate cargos, and external dependencies
> tend to become fragmented?

Hmm potentially yes, but that's a "what if" scenario. Let's worry about 
that bridge when we cross it!