Re: [Qemu-devel] [virtio-dev] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

On Tue, Dec 05, 2017 at 05:55:45PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 02:59:50PM +0000, Stefan Hajnoczi wrote:
> > On Tue, Dec 05, 2017 at 11:33:11AM +0800, Wei Wang wrote:
> > > Add the vhost-pci-net device emulation. The device uses bar 2 to expose
> > > the remote VM's memory to the guest. The first 4KB of the the bar area
> > > stores the metadata which describes the remote memory and vring info.
> > 
> > This device looks like the beginning of a new "vhost-pci" virtio device
> > type.  There are layering violations:
> > 
> > 1. This has nothing to do with virtio-net or networking, it's purely
> >    vhost-pci.  Why is it called vhost-pci-net instead of vhost-pci?
> > 
> > 2. VirtIODevice does not know about PCI.  It should work over virtio-ccw
> >    or virtio-mmio.  This patch talks about BARs inside a VirtIODevice so
> >    there is a problem here.
> 
> I think the point is how memory is exposed to another guest.  This
> device exposes it as a pci bar. I don't think e.g. ccw can do this,
> it's all hypercall-based.

Yes, that's why the BAR issue needs to be discussed.

In terms of the patches, the clean way to do it is for the
vhost-pci device to have a memory region that is not called "BAR".  The
virtio-pci transport can expose it as a BAR but the device doesn't need
to know about it.  Other transports that support memory mapping could
then work with this device too.

The VIRTIO specification needs to capture this transport requirement
somehow too so it's clear that the vhost device can only run over
transports that support memory mapping.

That said, it's not clear to me why the vhost-pci device is a VIRTIO
device.  It doesn't use virtqueues or the configuration space.  It only
uses the vhost-user chardev and the mapped memory.  Isn't it better to
make it a PCI device?

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]