Re: [PATCH 2/2] xen/virtio: Avoid use of the dom0 backend in dom0

[Roger Pau Monné]

On Tue, Jul 04, 2023 at 01:43:46PM +0200, Marek Marczykowski-Górecki wrote:
> Hi,
> 
> FWIW, I have ran into this issue some time ago too. I run Xen on top of
> KVM and then passthrough some of the virtio devices (network one
> specifically) into a (PV) guest. So, I hit both cases, the dom0 one and
> domU one. As a temporary workaround I needed to disable
> CONFIG_XEN_VIRTIO completely (just disabling
> CONFIG_XEN_VIRTIO_FORCE_GRANT was not enough to fix it).
> With that context in place, the actual response below.
> 
> On Tue, Jul 04, 2023 at 12:39:40PM +0200, Juergen Gross wrote:
> > On 04.07.23 09:48, Roger Pau Monné wrote:
> > > On Thu, Jun 29, 2023 at 03:44:04PM -0700, Stefano Stabellini wrote:
> > > > On Thu, 29 Jun 2023, Oleksandr Tyshchenko wrote:
> > > > > On 29.06.23 04:00, Stefano Stabellini wrote:
> > > > > > I think we need to add a second way? It could be anything that can help
> > > > > > us distinguish between a non-grants-capable virtio backend and a
> > > > > > grants-capable virtio backend, such as:
> > > > > > - a string on xenstore
> > > > > > - a xen param
> > > > > > - a special PCI configuration register value
> > > > > > - something in the ACPI tables
> > > > > > - the QEMU machine type
> > > > > 
> > > > > 
> > > > > Yes, I remember there was a discussion regarding that. The point is to
> > > > > choose a solution to be functional for both PV and HVM *and* to be able
> > > > > to support a hotplug. IIRC, the xenstore could be a possible candidate.
> > > > 
> > > > xenstore would be among the easiest to make work. The only downside is
> > > > the dependency on xenstore which otherwise virtio+grants doesn't have.
> > > 
> > > I would avoid introducing a dependency on xenstore, if nothing else we
> > > know it's a performance bottleneck.
> > > 
> > > We would also need to map the virtio device topology into xenstore, so
> > > that we can pass different options for each device.
> > 
> > This aspect (different options) is important. How do you want to pass virtio
> > device configuration parameters from dom0 to the virtio backend domain? You
> > probably need something like Xenstore (a virtio based alternative like virtiofs
> > would work, too) for that purpose.
> > 
> > Mapping the topology should be rather easy via the PCI-Id, e.g.:
> > 
> > /local/domain/42/device/virtio/0000:00:1c.0/backend
> 
> While I agree this would probably be the simplest to implement, I don't
> like introducing xenstore dependency into virtio frontend either.
> Toolstack -> backend communication is probably easier to solve, as it's
> much more flexible (could use qemu cmdline, QMP, other similar
> mechanisms for non-qemu backends etc).

I also think features should be exposed uniformly for devices, it's at
least weird to have certain features exposed in the PCI config space
while other features exposed in xenstore.

For virtio-mmio this might get a bit confusing, are we going to add
xenstore entries based on the position of the device config mmio
region?

I think on Arm PCI enumeration is not (usually?) done by the firmware,
at which point the SBDF expected by the tools/backend might be
different than the value assigned by the guest OS.

I think there are two slightly different issues, one is how to pass
information to virtio backends, I think doing this initially based on
xenstore is not that bad, because it's an internal detail of the
backend implementation. However passing information to virtio
frontends using xenstore is IMO a bad idea, there's already a way to
negotiate features between virtio frontends and backends, and Xen
should just expand and use that.


> > > > Vikram is working on virtio with grants support in QEMU as we speak.
> > > > Maybe we could find a way to add a flag in QEMU so that we can detect at
> > > > runtime if a given virtio device support grants or not.
> > > 
> > > Isn't there a way for the device to expose capabilities already?  For
> > > example how does a virtio-blk backend expose support for indirect
> > > descriptors?
> > 
> > Those capabilities are defined in the virtio spec [1]. Adding the backend
> > domid would be possible, but it probably wouldn't be that easy (requires
> > changing the virtio spec by either expanding an existing config area or by
> > adding a new one). I'm not sure handling in the specific frontends is
> > generic enough for being able to have a central place where the backend
> > domid could be retrieved, without requiring any change of the frontends.
> 
> IMHO the proper solution is to extend the spec. I don't have much
> experience with virtio code, but reading the spec it looks like new
> config area will be better for compatibility/uniform handling in a
> frontent-agnostic way. Since it will definitely take time, some
> transitional solution (maybe even xenstore...) might be warranted.

My fear is that such 'transitional' solutions end up being definitive
ones, as people move to other stuff once things work.  Might be better
to implement a transitional approach based on a future virtio
specification change rather than something out of band based on a
completely different communication channel.

Thanks, Roger.