Re: [RFC] vhost-vdpa-net: add vhost-vdpa-net host device support

[Stefan Hajnoczi]

On Tue, Dec 14, 2021 at 10:22:53AM +0800, Jason Wang wrote:
> On Mon, Dec 13, 2021 at 11:14 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Mon, Dec 13, 2021 at 10:47:00AM +0800, Jason Wang wrote:
> > > On Sun, Dec 12, 2021 at 5:30 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Sat, Dec 11, 2021 at 03:00:27AM +0000, Longpeng (Mike, Cloud 
> > > > Infrastructure Service Product Dept.) wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Stefan Hajnoczi [mailto:stefanha@redhat.com]
> > > > > > Sent: Thursday, December 9, 2021 5:17 PM
> > > > > > To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> > > > > > <longpeng2@huawei.com>
> > > > > > Cc: jasowang@redhat.com; mst@redhat.com; parav@nvidia.com;
> > > > > > xieyongji@bytedance.com; sgarzare@redhat.com; Yechuan 
> > > > > > <yechuan@huawei.com>;
> > > > > > Gonglei (Arei) <arei.gonglei@huawei.com>; qemu-devel@nongnu.org
> > > > > > Subject: Re: [RFC] vhost-vdpa-net: add vhost-vdpa-net host device 
> > > > > > support
> > > > > >
> > > > > > On Wed, Dec 08, 2021 at 01:20:10PM +0800, Longpeng(Mike) wrote:
> > > > > > > From: Longpeng <longpeng2@huawei.com>
> > > > > > >
> > > > > > > Hi guys,
> > > > > > >
> > > > > > > This patch introduces vhost-vdpa-net device, which is inspired
> > > > > > > by vhost-user-blk and the proposal of vhost-vdpa-blk device [1].
> > > > > > >
> > > > > > > I've tested this patch on Huawei's offload card:
> > > > > > > ./x86_64-softmmu/qemu-system-x86_64 \
> > > > > > >     -device vhost-vdpa-net-pci,vdpa-dev=/dev/vhost-vdpa-0
> > > > > > >
> > > > > > > For virtio hardware offloading, the most important requirement 
> > > > > > > for us
> > > > > > > is to support live migration between offloading cards from 
> > > > > > > different
> > > > > > > vendors, the combination of netdev and virtio-net seems too 
> > > > > > > heavy, we
> > > > > > > prefer a lightweight way.
> > > > > > >
> > > > > > > Maybe we could support both in the future ? Such as:
> > > > > > >
> > > > > > > * Lightweight
> > > > > > >  Net: vhost-vdpa-net
> > > > > > >  Storage: vhost-vdpa-blk
> > > > > > >
> > > > > > > * Heavy but more powerful
> > > > > > >  Net: netdev + virtio-net + vhost-vdpa
> > > > > > >  Storage: bdrv + virtio-blk + vhost-vdpa
> > > > > > >
> > > > > > > [1] 
> > > > > > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg797569.html
> > > > > >
> > > > > > Stefano presented a plan for vdpa-blk at KVM Forum 2021:
> > > > > > https://kvmforum2021.sched.com/event/ke3a/vdpa-blk-unified-hardware-and-sof
> > > > > > tware-offload-for-virtio-blk-stefano-garzarella-red-hat
> > > > > >
> > > > > > It's closer to today's virtio-net + vhost-net approach than the
> > > > > > vhost-vdpa-blk device you have mentioned. The idea is to treat vDPA 
> > > > > > as
> > > > > > an offload feature rather than a completely separate code path that
> > > > > > needs to be maintained and tested. That way QEMU's block layer 
> > > > > > features
> > > > > > and live migration work with vDPA devices and re-use the virtio-blk
> > > > > > code. The key functionality that has not been implemented yet is a 
> > > > > > "fast
> > > > > > path" mechanism that allows the QEMU virtio-blk device's virtqueue 
> > > > > > to be
> > > > > > offloaded to vDPA.
> > > > > >
> > > > > > The unified vdpa-blk architecture should deliver the same 
> > > > > > performance
> > > > > > as the vhost-vdpa-blk device you mentioned but with more features, 
> > > > > > so I
> > > > > > wonder what aspects of the vhost-vdpa-blk idea are important to you?
> > > > > >
> > > > > > QEMU already has vhost-user-blk, which takes a similar approach as 
> > > > > > the
> > > > > > vhost-vdpa-blk device you are proposing. I'm not against the
> > > > > > vhost-vdpa-blk approach in priciple, but would like to understand 
> > > > > > your
> > > > > > requirements and see if there is a way to collaborate on one 
> > > > > > vdpa-blk
> > > > > > implementation instead of dividing our efforts between two.
> > > > > >
> > > > >
> > > > > We prefer a simple way in the virtio hardware offloading case, it 
> > > > > could reduce
> > > > > our maintenance workload, we no need to maintain the virtio-net, 
> > > > > netdev,
> > > > > virtio-blk, bdrv and ... any more. If we need to support other vdpa 
> > > > > devices
> > > > > (such as virtio-crypto, virtio-fs) in the future, then we also need 
> > > > > to maintain
> > > > > the corresponding device emulation code?
> > > > >
> > > > > For the virtio hardware offloading case, we usually use the vfio-pci 
> > > > > framework,
> > > > > it saves a lot of our maintenance work in QEMU, we don't need to 
> > > > > touch the device
> > > > > types. Inspired by Jason, what we really prefer is 
> > > > > "vhost-vdpa-pci/mmio", use it to
> > > > > instead of the vfio-pci, it could provide the same performance as 
> > > > > vfio-pci, but it's
> > > > > *possible* to support live migrate between offloading cards from 
> > > > > different vendors.
> > > >
> > > > OK, so the features you are dropping would be migration between
> > > > a vdpa, vhost and virtio backends. I think given vhost-vdpa-blk is seems
> > > > fair enough... What do others think?
> > >
> > > I think it should be fine, and it would be even better to make it not
> > > specific to device type.
> >
> > That's an interesting idea. A generic vDPA VirtIODevice could exposed as
> >
> >   --device vhost-vdpa-pci,
> >            [vhostfd=FD,|
> >             vhostpath=/dev/vhost-vdpa-N]
> >
> > (and for virtio-mmio and virtio-ccw too).
> >
> > I don't think this is possible yet because the vhost_vdpa ioctls are
> > missing some introspection functionality. Here is what I found:
> > - Device ID: ok, use VHOST_VDPA_GET_DEVICE_ID
> > - Device feature bits: ok, use VHOST_GET_BACKEND_FEATURES
> > - Configuration space size: missing, need ioctl for ops->get_config_size()
> 
> Any specific reason that we need this considering we've already had
> VHOST_VDPA_GET_CONFIG and we do the size validation there?

QEMU's virtio_init() takes a size_t config_size argument. We need to
determine the size of the vhost_vdpa's configuration space in order to
create the VirtIODevice in QEMU.

Do you mean probing by checking for the VHOST_VDPA_GET_CONFIG -E2BIG
return value? It's hacky but I guess it's possible to do a binary search
that calls VHOST_VDPA_GET_CONFIG each iteration and reduces the size if
-E2BIG is returned or increases the size otherwise.

Or do you mean re-writing QEMU's hw/virtio/virtio.c to allow the
VirtIODevice to override the size and we pass accesses through to
vhost_vdpa. That way it might be possible to avoid fetching the
configuration space size at startup, but I'm not sure this will work
because QEMU might depend on knowing the exact size (e.g. live
migration).

> > - Max virtqueue size: ok, VHOST_VDPA_GET_VRING_NUM
> > - Number of virtqueues: probe using VHOST_GET_VRING_BASE?
> 
> I'm not sure whether or not we need this and it seems not necessary
> since it can be deduced from the config space and features.

It can only be deduced in a device-specific way (net, blk, etc). I can't
think of a way to detect the number of virtqueues for an arbitrary
VIRTIO device from the features bits and configuration space contents.

Stefan

signature.asc
Description: PGP signature