On 14/12/2023 18:15, Alex Williamson wrote:
On Thu, 14 Dec 2023 18:03:30 +0200
Yishai Hadas <yishaih@xxxxxxxxxx> wrote:
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:
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.
I understand the intent, but I don't think that justifies building a
driver that serves no purpose in the interim. IF and when migration
support becomes a reality, it's trivial to update the depends line.
OK, so I'll add a KCONFIG dependency on X86 as you suggested as part of
V9 inside VFIO.
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.
Maybe this should be governed by a new Kconfig option which would be
selected by drivers like this. Thanks,
We can still keep the simple ifdef X86 inside VIRTIO for future
users/usage which is not only VFIO.
Michael,
Can that work for you ?
Yishai
Alex
