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

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

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!