On Thu, Sep 9, 2021 at 8:53 PM Wu Zongyong <wuzongyong at
linux.alibaba.com> wrote:>
> On Thu, Sep 09, 2021 at 11:05:06AM +0800, Jason Wang wrote:
> > On Wed, Sep 8, 2021 at 8:22 PM Wu Zongyong <wuzongyong at
linux.alibaba.com> wrote:
> > >
> > > This series implements the vDPA driver for legacy virtio-pci
device.
> > > Currently we already have the vDPA driver for modern virtio-pci
device
> > > only, but there are some legacy virtio-pci devices conforming to
the
> > > virtio-pci specifications of 0.9.x or older versions. For
example,
> > > ENI(Elastic Network Interface) of Alibaba ECS baremetal instance
is a
> > > hardware virtio network device which follows the Virtio PCI Card
0.9.5
> > > Draft specification. Such legacy virtio-pci devices have some
> > > inconsistent behaviour with modern virtio-pci devices, so some
common
> > > codes are split out and modern device specific codes are moved to
a
> > > separated file.
> >
> > What worries me a little bit are:
> >
> > 1) vDPA requires IOMMU_PLATFORM to be supported by the device to work,
> > if I understand ENI correctly, it's a legacy device so it
can't
> > support ACCESS_PLATFORM. Or is it a legacy device that supports
> > ACCESS_PLATFORM implicitly.
> > 2) vDPA tries to present a 1.0 device, in this case the behavior could
> > be ruled by the spec. If we tries to present an 1.0 device on top of
> > legacy device we may suffer a lot of issues:
> >
> > - endian issue: 1.0 use le but legacy may use native endian
> > - queue_enable semantic which is missed in the legacy
> > - virtqueue size, as you mentioned below
> >
> > I guess what the device(ENI) supports are:
> >
> > 1) semantic of ACCESS_PLATFORM without a feature
> > 2) little endian
> > 3) but a legacy device
> >
> > So I think it might be better:
> >
> > 1) introduce the library for legacy as you did in this patch
> > 2) having a dedicated ENI vDPA driver
>
> Would you mind I place the ENI vDPA driver inside virtio_pci folder? Or
> should I create a new folder for it?
I think it's better to have a new folder.
Thanks
>
> >
> > 3) live migration support, though it was not supported by the spec
> > yet, but we are working on the support, and we know legacy device can
> > support this.
> >
> > Thanks
> >
> > >
> > > For legacy devices, it is not supported to negotiate the
virtqueue size
> > > by the specification. So a new callback get_vq_num_unchangeable
is
> > > introduced to indicate user not to try change the virtqueue size
of the
> > > legacy vdpa device. For example, QEMU should not allocate memory
for
> > > virtqueue according to the properties tx_queue_size and
rx_queue_size if
> > > we use legacy virtio-pci device as the vhost-vdpa backend.
Instead, QEMU
> > > should use the new callback get_vq_num_unchangeable first to
check if
> > > the vdpa device support to change virtqueue size. If not, QEMU
should
> > > call the callback get_vq_num_max to get the static virtqueue size
then
> > > allocate the same size memory for the virtqueue.
> > >
> > > This series have been tested with the ENI in Alibaba ECS
baremetal
> > > instance.
> > >
> > > These patches may under consideration, welcome for comments.
> > >
> > >
> > > Wu Zongyong (6):
> > > virtio-pci: introduce legacy device module
> > > vdpa: fix typo
> > > vp_vdpa: add vq irq offloading support
> > > vp_vdpa: split out reusable and device specific codes to
separate file
> > > vdpa: add get_vq_num_unchangeable callback in vdpa_config_ops
> > > vp_vdpa: introduce legacy virtio pci driver
> > >
> > > drivers/vdpa/Kconfig | 7 +
> > > drivers/vdpa/virtio_pci/Makefile | 3 +
> > > drivers/vdpa/virtio_pci/vp_vdpa.c | 8 +
> > > drivers/vdpa/virtio_pci/vp_vdpa_common.c | 220 ++++++++++++++
> > > drivers/vdpa/virtio_pci/vp_vdpa_common.h | 67 +++++
> > > drivers/vdpa/virtio_pci/vp_vdpa_legacy.c | 346
+++++++++++++++++++++++
> > > drivers/vdpa/virtio_pci/vp_vdpa_modern.c | 327
+++++++++++++++++++++
> > > drivers/vhost/vdpa.c | 19 ++
> > > drivers/virtio/Kconfig | 10 +
> > > drivers/virtio/Makefile | 1 +
> > > drivers/virtio/virtio_pci_common.c | 10 +-
> > > drivers/virtio/virtio_pci_common.h | 9 +-
> > > drivers/virtio/virtio_pci_legacy.c | 101 ++-----
> > > drivers/virtio/virtio_pci_legacy_dev.c | 220 ++++++++++++++
> > > drivers/virtio/virtio_vdpa.c | 5 +-
> > > include/linux/vdpa.h | 6 +-
> > > include/linux/virtio_pci_legacy.h | 44 +++
> > > include/uapi/linux/vhost.h | 2 +
> > > 18 files changed, 1320 insertions(+), 85 deletions(-)
> > > create mode 100644 drivers/vdpa/virtio_pci/vp_vdpa_common.c
> > > create mode 100644 drivers/vdpa/virtio_pci/vp_vdpa_common.h
> > > create mode 100644 drivers/vdpa/virtio_pci/vp_vdpa_legacy.c
> > > create mode 100644 drivers/vdpa/virtio_pci/vp_vdpa_modern.c
> > > create mode 100644 drivers/virtio/virtio_pci_legacy_dev.c
> > > create mode 100644 include/linux/virtio_pci_legacy.h
> > >
> > > --
> > > 2.31.1
> > >
>