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 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 >
On Thu, Sep 9, 2021 at 11:05 AM Jason Wang <jasowang at redhat.com> 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 > > 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 canI meant "can't" actually. Thanks> 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 > >
Michael S. Tsirkin
2021-Sep-09 09:21 UTC
[PATCH 0/6] vDPA driver for legacy virtio-pci device
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 belowSo this all kind of works when guest and host are strongly ordered and LE. Case in point x86. Question is how do we limit this to an x86 guest? Add a new ioctl declaring that this is the case? -- MST