On 14/12/2023 17:05, Michael S. Tsirkin wrote:
On Thu, Dec 14, 2023 at 07:59:05AM -0700, Alex Williamson wrote:
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
Kconfig dependency is what I had in mind, yes. The X86 specific code in
virtio_pci_modern.c can be moved to a separate file then use makefile
tricks to skip it on other platforms.
The next feature for that driver will be the live migration support over
virtio, once the specification which is WIP those day will be accepted.
The migration functionality is not X86 dependent and doesn't have the
legacy virtio driver limitations that enforced us to run only on X86.
So, by that time we may need to enable in VFIO the loading of
virtio-vfio-pci driver and put back the ifdef X86 inside VIRTIO, only on
the legacy IO API, as I did already in V8.
So using a KCONFIG solution in VFIO is a short term one, which will be
reverted just later on.
In addition, the virtio_pci_admin_has_legacy_io() API can be used in the
future not only by VFIO, this was one of the reasons to put it inside
VIRTIO.
As of that, we may expect that VIRTIO will consider its limitation and
return false in systems that are not X86 and won't expose its
concerns/knowledge to other modules.
As of the above, I believe that better staying with the solution inside
VIRTIO as was suggested here and is part of V8 already.
Makes sense ?
Yishai
