Re: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine

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

On Wed, Jun 26, 2024 at 12:18:55AM +0800, Zhao Liu wrote:
> Hi Manos,
> 
> On Wed, Jun 19, 2024 at 11:14:02PM +0300, Manos Pitsidianakis wrote:
> > Date: Wed, 19 Jun 2024 23:14:02 +0300
> > From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > Subject: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with
> >  x-pl011-rust in arm virt machine
> > X-Mailer: git-send-email 2.44.0
> > 
> > Convenience patch for testing the rust device.
> > 
> > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > ---
> >  hw/arm/virt.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 3c93c0c0a6..f33b58ae0d 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -912,7 +912,11 @@ static void create_uart(const VirtMachineState *vms, int uart,
> >      int irq = vms->irqmap[uart];
> >      const char compat[] = "arm,pl011\0arm,primecell";
> >      const char clocknames[] = "uartclk\0apb_pclk";
> > +#ifdef CONFIG_WITH_RUST
> > +    DeviceState *dev = qdev_new("x-pl011-rust");
> > +#else
> >      DeviceState *dev = qdev_new(TYPE_PL011);
> > +#endif
> >      SysBusDevice *s = SYS_BUS_DEVICE(dev);
> >      MachineState *ms = MACHINE(vms);
> >
> 
> I realized that if we want to merge the rust pl011 device, then this
> patch or similar enablement support is necessary, otherwise, the rust
> code is only used for compilation and cannot actually be run...
> 
> This is also an open for the devices that are rewrite in Rust.
> 
> I think there should be an option for the user to choose whether to
> enable pl011 in C or pl011 in Rust. What do you think?
> 
> Perhaps the easiest way to enable rust pl011 is to add an option for
> virt machine... But that's certainly not a long-term approach. I think
> the ideal way would be to allow rust pl011 to be specified in the
> command line via -device, but this approach would mean allowing the
> user to create pl011 and would require changes to the current buildin
> pl011's creation logic.

While having both impls present is reasonable for the RFC, IMHO, if
we're to merge this, then the Rust impl of pl011 should fully replace
the C impl. Maintaining 2 different impls of the same device in
parallel is highly undesirable. This would of course impl that the
Rust impls needs to be a feature match for the existing impl. If we
do a full replacement, then the question of how the user chooses
between the 2 impls is then entirely avoided.

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 :|