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

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jun 11, 2024 at 03:16:19PM +0200, Paolo Bonzini wrote:
> On Tue, Jun 11, 2024 at 10:22 AM Daniel P. Berrangé <berrange@redhat.com> 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.
> 
> I think that's incompatible with the layout that Cargo expects.
> rust/hw/arm/pl011/ could be another possibility.

It doesn't look like its a problem in this patch series. It is just
introducing a "rust/pl011/Cargo.toml", and I don't see anything
that has an fundamental assumption that it is below a 'rust/' top
level dir, as opposed to under our existing dir structure.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|