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
