Re: [PATCH V4 vfio 0/9] Introduce a vfio driver over virtio devices

Yishai Hadas
2023-11-30T15:47:39Z

On 30/11/2023 12:07, Michael S. Tsirkin wrote:
> On Wed, Nov 29, 2023 at 04:37:37PM +0200, Yishai Hadas wrote:
>> This series introduce a vfio driver over virtio devices to support the
>> legacy interface functionality for VFs.
>>
>> Background, from the virtio spec [1].
>> --------------------------------------------------------------------
>> In some systems, there is a need to support a virtio legacy driver with
>> a device that does not directly support the legacy interface. In such
>> scenarios, a group owner device can provide the legacy interface
>> functionality for the group member devices. The driver of the owner
>> device can then access the legacy interface of a member device on behalf
>> of the legacy member device driver.
>>
>> For example, with the SR-IOV group type, group members (VFs) can not
>> present the legacy interface in an I/O BAR in BAR0 as expected by the
>> legacy pci driver. If the legacy driver is running inside a virtual
>> machine, the hypervisor executing the virtual machine can present a
>> virtual device with an I/O BAR in BAR0. The hypervisor intercepts the
>> legacy driver accesses to this I/O BAR and forwards them to the group
>> owner device (PF) using group administration commands.
>> --------------------------------------------------------------------
>>
>> The first 6 patches are in the virtio area and handle the below:
>> - Introduce the admin virtqueue infrastcture.
>> - Expose the layout of the commands that should be used for
>> supporting the legacy access.
>> - Expose APIs to enable upper layers as of vfio, net, etc
>> to execute admin commands.
>>
>> The above follows the virtio spec that was lastly accepted in that area
>> [1].
>>
>> The last 3 patches are in the vfio area and handle the below:
>> - Expose some APIs from vfio/pci to be used by the vfio/virtio driver.
>> - Introduce a vfio driver over virtio devices to support the legacy
>> interface functionality for VFs.
>>
>> The series was tested successfully over virtio-net VFs in the host,
>> while running in the guest both modern and legacy drivers.
>>
>> [1]
>> https://github.com/oasis-tcs/virtio-spec/commit/03c2d32e5093ca9f2a17797242fbef88efe94b8c
>
>
>
> I sent a minor question, but besides that, patches 1-6 look ok
> Acked-by: Michael S. Tsirkin <mst@redhat.com
Thanks Michael

I'll add your Acked-by for the above patches as part of V5 and will drop
the unused macro (i.e.VIRTIO_ADMIN_MAX_CMD_OPCODE) as you asked.

>
> I take no stance on patches 7-9 this is up to Alex.

Alex,
Are we fine with the rest of series from your side ?

Thanks,
Yishai

>
>
>> Changes from V3: https://www.spinics.net/lists/kvm/msg333008.html
>> Virtio:
>> - Rebase on top of 6.7 rc3.
>> Vfio:
>> - Fix a typo, drop 'acc' from 'virtiovf_acc_vfio_pci_tran_ops'.
>>
>> Changes from V2: https://lore.kernel.org/all/20231029155952.67686-8-yishaih@nvidia.com/T/
>> Virtio:
>> - Rebase on top of 6.7 rc1.
>> - Add a mutex to serialize admin commands execution and virtqueue
>> deletion, as was suggested by Michael.
>> - Remove the 'ref_count' usage which is not needed any more.
>> - Reduce the depth of the admin vq to match a single command at a given time.
>> - Add a supported check upon command execution and move to use a single
>> flow of virtqueue_exec_admin_cmd().
>> - Improve the description of the exported commands to better match the
>> specification and the expected usage as was asked by Michael.
>>
>> Vfio:
>> - Upon calling to virtio_pci_admin_legacy/common_device_io_read/write()
>> supply the 'offset' within the relevant configuration area, following
>> the virtio exported APIs.
>>
>> Changes from V1: https://lore.kernel.org/all/20231023104548.07b3aa19.alex.williamson@redhat.com/T/
>> Virtio:
>> - Drop its first patch, it was accepted upstream already.
>> - Add a new patch (#6) which initializes the supported admin commands
>> upon admin queue activation as was suggested by Michael.
>> - Split the legacy_io_read/write commands per common/device
>> configuration as was asked by Michael.
>> - Don't expose any more the list query/used APIs outside of virtio.
>> - Instead, expose an API to check whether the legacy io functionality is
>> supported as was suggested by Michael.
>> - Fix some Krobot's note by adding the missing include file.
>>
>> Vfio:
>> - Refer specifically to virtio-net as part of the driver/module description
>> as Alex asked.
>> - Change to check MSIX enablement based on the irq type of the given vfio
>> core device. In addition, drop its capable checking from the probe flow
>> as was asked by Alex.
>> - Adapt to use the new virtio exposed APIs and clean some code accordingly.
>> - Adapt to some cleaner style code in some places (if/else) as was suggested
>> by Alex.
>> - Fix the range_intersect_range() function and adapt its usage as was
>> pointed by Alex.
>> - Make struct virtiovf_pci_core_device better packed.
>> - Overwrite the subsystem vendor ID to be 0x1af4 as was discussed in
>> the ML.
>> - Add support for the 'bar sizing negotiation' as was asked by Alex.
>> - Drop the 'acc' from the 'ops' as Alex asked.
>>
>> Changes from V0: https://www.spinics.net/lists/linux-virtualization/msg63802.html
>>
>> Virtio:
>> - Fix the common config map size issue that was reported by Michael
>> Tsirkin.
>> - Do not use vp_dev->vqs[] array upon vp_del_vqs() as was asked by
>> Michael, instead skip the AQ specifically.
>> - Move admin vq implementation into virtio_pci_modern.c as was asked by
>> Michael.
>> - Rename structure virtio_avq to virtio_pci_admin_vq and some extra
>> corresponding renames.
>> - Remove exported symbols virtio_pci_vf_get_pf_dev(),
>> virtio_admin_cmd_exec() as now callers are local to the module.
>> - Handle inflight commands as part of the device reset flow.
>> - Introduce APIs per admin command in virtio-pci as was asked by Michael.
>>
>> Vfio:
>> - Change to use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL for
>> vfio_pci_core_setup_barmap() and vfio_pci_iowrite#xxx() as pointed by
>> Alex.
>> - Drop the intermediate patch which prepares the commands and calls the
>> generic virtio admin command API (i.e. virtio_admin_cmd_exec()).
>> - Instead, call directly to the new APIs per admin command that are
>> exported from Virtio - based on Michael's request.
>> - Enable only virtio-net as part of the pci_device_id table to enforce
>> upon binding only what is supported as suggested by Alex.
>> - Add support for byte-wise access (read/write) over the device config
>> region as was asked by Alex.
>> - Consider whether MSIX is practically enabled/disabled to choose the
>> right opcode upon issuing read/write admin command, as mentioned
>> by Michael.
>> - Move to use VIRTIO_PCI_CONFIG_OFF instead of adding some new defines
>> as was suggested by Michael.
>> - Set the '.close_device' op to vfio_pci_core_close_device() as was
>> pointed by Alex.
>> - Adapt to Vfio multi-line comment style in a few places.
>> - Add virtualization@lists.linux-foundation.org in the MAINTAINERS file
>> to be CCed for the new driver as was suggested by Jason.
>>
>> Yishai
>>
>> Feng Liu (4):
>> virtio: Define feature bit for administration virtqueue
>> virtio-pci: Introduce admin virtqueue
>> virtio-pci: Introduce admin command sending function
>> virtio-pci: Introduce admin commands
>>
>> Yishai Hadas (5):
>> virtio-pci: Initialize the supported admin commands
>> virtio-pci: Introduce APIs to execute legacy IO admin commands
>> vfio/pci: Expose vfio_pci_core_setup_barmap()
>> vfio/pci: Expose vfio_pci_iowrite/read##size()
>> vfio/virtio: Introduce a vfio driver over virtio devices
>>
>> MAINTAINERS | 7 +
>> drivers/vfio/pci/Kconfig | 2 +
>> drivers/vfio/pci/Makefile | 2 +
>> drivers/vfio/pci/vfio_pci_core.c | 25 ++
>> drivers/vfio/pci/vfio_pci_rdwr.c | 38 +-
>> drivers/vfio/pci/virtio/Kconfig | 16 +
>> drivers/vfio/pci/virtio/Makefile | 4 +
>> drivers/vfio/pci/virtio/main.c | 554 +++++++++++++++++++++++++
>> drivers/virtio/virtio.c | 37 +-
>> drivers/virtio/virtio_pci_common.c | 14 +
>> drivers/virtio/virtio_pci_common.h | 21 +-
>> drivers/virtio/virtio_pci_modern.c | 503 +++++++++++++++++++++-
>> drivers/virtio/virtio_pci_modern_dev.c | 24 +-
>> include/linux/vfio_pci_core.h | 20 +
>> include/linux/virtio.h | 8 +
>> include/linux/virtio_config.h | 4 +
>> include/linux/virtio_pci_admin.h | 21 +
>> include/linux/virtio_pci_modern.h | 2 +
>> include/uapi/linux/virtio_config.h | 8 +-
>> include/uapi/linux/virtio_pci.h | 71 ++++
>> 20 files changed, 1342 insertions(+), 39 deletions(-)
>> create mode 100644 drivers/vfio/pci/virtio/Kconfig
>> create mode 100644 drivers/vfio/pci/virtio/Makefile
>> create mode 100644 drivers/vfio/pci/virtio/main.c
>> create mode 100644 include/linux/virtio_pci_admin.h
>>
>> --
>> 2.27.0
>