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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 14 Dec 2023 11:37:10 +0200
Yishai Hadas <yishaih@xxxxxxxxxx> wrote:

> On 14/12/2023 11:19, Michael S. Tsirkin wrote:
> > On Thu, Dec 14, 2023 at 11:03:49AM +0200, Yishai Hadas wrote:  
> >> On 14/12/2023 8:38, Michael S. Tsirkin wrote:  
> >>> On Thu, Dec 07, 2023 at 12:28:20PM +0200, Yishai Hadas wrote:  
> >>>> 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.
> >>>> --------------------------------------------------------------------
> >>>>
> >>>> Specifically, this driver adds support for a virtio-net VF to be exposed
> >>>> as a transitional device to a guest driver and allows the legacy IO BAR
> >>>> functionality on top.
> >>>>
> >>>> This allows a VM which uses a legacy virtio-net driver in the guest to
> >>>> work transparently over a VF which its driver in the host is that new
> >>>> driver.
> >>>>
> >>>> The driver can be extended easily to support some other types of virtio
> >>>> devices (e.g virtio-blk), by adding in a few places the specific type
> >>>> properties as was done for virtio-net.
> >>>>
> >>>> For now, only the virtio-net use case was tested and as such we introduce
> >>>> the support only for such a device.
> >>>>
> >>>> Practically,
> >>>> Upon probing a VF for a virtio-net device, in case its PF supports
> >>>> legacy access over the virtio admin commands and the VF doesn't have BAR
> >>>> 0, we set some specific 'vfio_device_ops' to be able to simulate in SW a
> >>>> transitional device with I/O BAR in BAR 0.
> >>>>
> >>>> The existence of the simulated I/O bar is reported later on by
> >>>> overwriting the VFIO_DEVICE_GET_REGION_INFO command and the device
> >>>> exposes itself as a transitional device by overwriting some properties
> >>>> upon reading its config space.
> >>>>
> >>>> Once we report the existence of I/O BAR as BAR 0 a legacy driver in the
> >>>> guest may use it via read/write calls according to the virtio
> >>>> specification.
> >>>>
> >>>> Any read/write towards the control parts of the BAR will be captured by
> >>>> the new driver and will be translated into admin commands towards the
> >>>> device.
> >>>>
> >>>> Any data path read/write access (i.e. virtio driver notifications) will
> >>>> be forwarded to the physical BAR which its properties were supplied by
> >>>> the admin command VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO upon the
> >>>> probing/init flow.
> >>>>
> >>>> With that code in place a legacy driver in the guest has the look and
> >>>> feel as if having a transitional device with legacy support for both its
> >>>> control and data path flows.
> >>>>
> >>>> [1]
> >>>> https://github.com/oasis-tcs/virtio-spec/commit/03c2d32e5093ca9f2a17797242fbef88efe94b8c
> >>>>
> >>>> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> >>>> Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxx>
> >>>> ---
> >>>>    MAINTAINERS                      |   7 +
> >>>>    drivers/vfio/pci/Kconfig         |   2 +
> >>>>    drivers/vfio/pci/Makefile        |   2 +
> >>>>    drivers/vfio/pci/virtio/Kconfig  |  16 +
> >>>>    drivers/vfio/pci/virtio/Makefile |   4 +
> >>>>    drivers/vfio/pci/virtio/main.c   | 567 +++++++++++++++++++++++++++++++
> >>>>    6 files changed, 598 insertions(+)
> >>>>    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
> >>>>
> >>>> diff --git a/MAINTAINERS b/MAINTAINERS
> >>>> index 012df8ccf34e..b246b769092d 100644
> >>>> --- a/MAINTAINERS
> >>>> +++ b/MAINTAINERS
> >>>> @@ -22872,6 +22872,13 @@ L:	kvm@xxxxxxxxxxxxxxx
> >>>>    S:	Maintained
> >>>>    F:	drivers/vfio/pci/mlx5/
> >>>> +VFIO VIRTIO PCI DRIVER
> >>>> +M:	Yishai Hadas <yishaih@xxxxxxxxxx>
> >>>> +L:	kvm@xxxxxxxxxxxxxxx
> >>>> +L:	virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
> >>>> +S:	Maintained
> >>>> +F:	drivers/vfio/pci/virtio
> >>>> +
> >>>>    VFIO PCI DEVICE SPECIFIC DRIVERS
> >>>>    R:	Jason Gunthorpe <jgg@xxxxxxxxxx>
> >>>>    R:	Yishai Hadas <yishaih@xxxxxxxxxx>
> >>>> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> >>>> index 8125e5f37832..18c397df566d 100644
> >>>> --- a/drivers/vfio/pci/Kconfig
> >>>> +++ b/drivers/vfio/pci/Kconfig
> >>>> @@ -65,4 +65,6 @@ source "drivers/vfio/pci/hisilicon/Kconfig"
> >>>>    source "drivers/vfio/pci/pds/Kconfig"
> >>>> +source "drivers/vfio/pci/virtio/Kconfig"
> >>>> +
> >>>>    endmenu
> >>>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> >>>> index 45167be462d8..046139a4eca5 100644
> >>>> --- a/drivers/vfio/pci/Makefile
> >>>> +++ b/drivers/vfio/pci/Makefile
> >>>> @@ -13,3 +13,5 @@ obj-$(CONFIG_MLX5_VFIO_PCI)           += mlx5/
> >>>>    obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/
> >>>>    obj-$(CONFIG_PDS_VFIO_PCI) += pds/
> >>>> +
> >>>> +obj-$(CONFIG_VIRTIO_VFIO_PCI) += virtio/
> >>>> diff --git a/drivers/vfio/pci/virtio/Kconfig b/drivers/vfio/pci/virtio/Kconfig
> >>>> new file mode 100644
> >>>> index 000000000000..3a6707639220
> >>>> --- /dev/null
> >>>> +++ b/drivers/vfio/pci/virtio/Kconfig
> >>>> @@ -0,0 +1,16 @@
> >>>> +# SPDX-License-Identifier: GPL-2.0-only
> >>>> +config VIRTIO_VFIO_PCI
> >>>> +        tristate "VFIO support for VIRTIO NET PCI devices"
> >>>> +        depends on VIRTIO_PCI
> >>>> +        select VFIO_PCI_CORE
> >>>> +        help
> >>>> +          This provides support for exposing VIRTIO NET VF devices which support
> >>>> +          legacy IO access, using the VFIO framework that can work with a legacy
> >>>> +          virtio driver in the guest.
> >>>> +          Based on PCIe spec, VFs do not support I/O Space; thus, VF BARs shall
> >>>> +          not indicate I/O Space.
> >>>> +          As of that this driver emulated I/O BAR in software to let a VF be
> >>>> +          seen as a transitional device in the guest and let it work with
> >>>> +          a legacy driver.
> >>>> +
> >>>> +          If you don't know what to do here, say N.  
> >>>
> >>> BTW shouldn't this driver be limited to X86? Things like lack of memory
> >>> barriers will make legacy virtio racy on e.g. ARM will they not?
> >>> And endian-ness will be broken on PPC ...
> >>>  
> >>
> >> OK, if so, we can come with the below extra code.
> >> Makes sense ?
> >>
> >> I'll squash it as part of V8 to the relevant patch.
> >>
> >> diff --git a/drivers/virtio/virtio_pci_modern.c
> >> b/drivers/virtio/virtio_pci_modern.c
> >> index 37a0035f8381..b652e91b9df4 100644
> >> --- a/drivers/virtio/virtio_pci_modern.c
> >> +++ b/drivers/virtio/virtio_pci_modern.c
> >> @@ -794,6 +794,9 @@ bool virtio_pci_admin_has_legacy_io(struct pci_dev
> >> *pdev)
> >>          struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev);
> >>          struct virtio_pci_device *vp_dev;
> >>
> >> +#ifndef CONFIG_X86
> >> +       return false;
> >> +#endif
> >>          if (!virtio_dev)
> >>                  return false;
> >>
> >> Yishai  
> > 
> > Isn't there going to be a bunch more dead code that compiler won't be
> > able to elide?
> >   
> 
> On my setup the compiler didn't complain about dead-code (I simulated it 
> by using ifdef CONFIG_X86 return false).
> 
> However, if we suspect that some compiler might complain, we can come 
> with the below instead.
> 
> Do you prefer that ?
> 
> index 37a0035f8381..53e29824d404 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -782,6 +782,7 @@ static void vp_modern_destroy_avq(struct 
> virtio_device *vdev)
>           BIT_ULL(VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ) | \
>           BIT_ULL(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO))
> 
> +#ifdef CONFIG_X86
>   /*
>    * virtio_pci_admin_has_legacy_io - Checks whether the legacy IO
>    * commands are supported
> @@ -807,6 +808,12 @@ bool virtio_pci_admin_has_legacy_io(struct pci_dev 
> *pdev)
>                  return true;
>          return false;
>   }
> +#else
> +bool virtio_pci_admin_has_legacy_io(struct pci_dev *pdev)
> +{
> +       return false;
> +}
> +#endif
>   EXPORT_SYMBOL_GPL(virtio_pci_admin_has_legacy_io);

Doesn't this also raise the question of the purpose of virtio-vfio-pci
on non-x86?  Without any other features it offers nothing over vfio-pci
and we should likely adjust the Kconfig for x86 or COMPILE_TEST.
Thanks,

Alex





[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux