Re: [PATCH V5 1/6] libxl: Add support for Virtio I2C device

[Juergen Gross <jgross@xxxxxxxx>]

On 08.09.22 06:24, Viresh Kumar wrote:
> On 07-09-22, 18:49, Julien Grall wrote:
> > Looking at this series, you will add ~250 lines (assuming your new patch)
> > for the i2c and then likely the same amount for GPIO.
> >
> > I am assuming that for every new virtio device (e.g. gps, sound,
> > display...), we would also need to 250 lines of code. I am worry that we
> > will end up to bloat libxl with duplicated code and or for device that are
> > barely used.
> I agree.
>
> > I think it would be better to find a generic way to add new virtio device
> > without adding code (very limited) in libxl. The advantage is someone will
> > be able to create a new virtio device with less effort.
> >
> > The approach I can think of is something along the lines:
> >
> > virtio = ["type=<compatible>,transport=<transport>,..."]
> It sounds good and yeah it will save a lot of trouble.
Yes, I'd like that, too.

> > where the compatible is the one that should be written in the DT and
> > transport is mmio or pci. the [...] refers to specific parameters that would
> > need to be passed to the backend (it is not clear how you provide them
> > today?).
> The backend doesn't need lot of parameters to be passed right now, the host
> specific ones (like which devices on a bus to share to the guests) are passed by
> its command line. The backends in our case are hypervisor agnostic and are run
> independently as daemons, they just need to know base/irq, which we get from xen
> libraries.
When using qemu as a backend this will be quite different.

For my prototype "virtio using grants" I needed to add the following parameters
when starting qemu just for one virtio disk:

-drive file=/home/vm/sle15sp1/image,if=none,id=drive-virtio-disk0,format=raw' 
-device 
virtio-blk-pci,scsi=off,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=0,disable-legacy=on
As in commit 66dd1c62b2a3c70 I'd suggest to add "backendtype=" in order to
support further special handling for e.g. qemu. I guess your backendtype would
probably be "standalone", but in case you need further parameters from xl/libxl
you could add another type suiting your needs.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature