Re: [PATCH v14 19/20] vfio: initialize the virqfd workqueue in VFIO generic code
Hello Alex, The module solution seems fine for me, I have no argument against it. I used your patch on my tests, they are running ok. Regards, Baptiste On Wed, Mar 18, 2015 at 12:04 AM, Alex Williamson wrote: > On Tue, 2015-03-17 at 16:29 -0600, Alex Williamson wrote: >> On Mon, 2015-03-02 at 17:59 +0100, Baptiste Reynal wrote: >> > From: Antonios Motakis >> > >> > Now we have finally completely decoupled virqfd from VFIO_PCI. We can >> > initialize it from the VFIO generic code, in order to safely use it from >> > multiple independent VFIO bus drivers. >> > >> > Signed-off-by: Antonios Motakis >> > Signed-off-by: Baptiste Reynal >> > --- >> > drivers/vfio/Makefile | 4 +++- >> > drivers/vfio/pci/Makefile | 3 +-- >> > drivers/vfio/pci/vfio_pci.c | 8 >> > drivers/vfio/vfio.c | 8 >> > 4 files changed, 12 insertions(+), 11 deletions(-) >> > >> > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile >> > index dadf0ca..d798b09 100644 >> > --- a/drivers/vfio/Makefile >> > +++ b/drivers/vfio/Makefile >> > @@ -1,4 +1,6 @@ >> > -obj-$(CONFIG_VFIO) += vfio.o >> > +vfio_core-y := vfio.o virqfd.o >> > + >> > +obj-$(CONFIG_VFIO) += vfio_core.o >> > obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o >> > obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o >> > obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o >> >> This inadvertently (I assume) renames the main vfio module to vfio_core. >> That potentially breaks numerous userspace scripts that might try to >> load the "vfio" module. I don't think that's acceptable. A brute force >> way to fix this would be to rename vfio.c to vfio_core.c and change the >> Makefile to: >> >> vfio-y := vfio_core.o virqfd.o >> obj-$(CONFIG_VFIO) += vfio.o >> >> Is there any other trickery available to us that could include virqfd.o >> in vfio.o w/o source file renaming? Thanks, > > Maybe a better option, we could let virqfd be it's own support module > for bus drivers that need it. Then we keep it out of vfio "core". > Something like this: > > commit f4d91ec4b72ce11e9dba861d6bf2dba93b72f0ba > Author: Alex Williamson > Date: Tue Mar 17 08:33:38 2015 -0600 > > vfio: Split virqfd into a separate module for vfio bus drivers > > An unintended consequence of splittng virqfd support to be shared > by bus drivers is renaming the core vfio module to vfio_core. This > is not very friendly to user scripts that may try to load the vfio > module. To resolve that and to make it clear that virqfd is a bus > driver service and not a dependency of vfio core, move this to a > separate module on which the bus drivers will depend. > > Signed-off-by: Alex Williamson > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > index d5322a4..7d092dd 100644 > --- a/drivers/vfio/Kconfig > +++ b/drivers/vfio/Kconfig > @@ -13,6 +13,11 @@ config VFIO_SPAPR_EEH > depends on EEH && VFIO_IOMMU_SPAPR_TCE > default n > > +config VFIO_VIRQFD > + tristate > + depends on VFIO && EVENTFD > + default n > + > menuconfig VFIO > tristate "VFIO Non-Privileged userspace driver framework" > depends on IOMMU_API > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > index d798b09..7b8a31f 100644 > --- a/drivers/vfio/Makefile > +++ b/drivers/vfio/Makefile > @@ -1,6 +1,7 @@ > -vfio_core-y := vfio.o virqfd.o > +vfio_virqfd-y := virqfd.o > > -obj-$(CONFIG_VFIO) += vfio_core.o > +obj-$(CONFIG_VFIO) += vfio.o > +obj-$(CONFIG_VFIO_VIRQFD) += vfio_virqfd.o > obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o > obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o > obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o > diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig > index c6bb5da..579d83b 100644 > --- a/drivers/vfio/pci/Kconfig > +++ b/drivers/vfio/pci/Kconfig > @@ -1,6 +1,7 @@ > config VFIO_PCI > tristate "VFIO support for PCI devices" > depends on VFIO && PCI && EVENTFD > + select VFIO_VIRQFD > help > Support for the PCI VFIO bus driver. This is required to make > use of PCI drivers using the VFIO framework. > diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig > index c0a3bff..4ec14af 100644 > --- a/drivers/vfio/platform/Kconfig > +++ b/drivers/vfio/platform/Kconfig > @@ -1,6 +1,7 @@ > config VFIO_PLATFORM > tristate "VFIO support for platform devices" > depends on VFIO && EVENTFD && ARM > + select VFIO_VIRQFD > help > Support for platform devices with VFIO. This is required to make > use of platform devices present on the system using the VFIO > @@ -11,6 +12,7 @@ config VFIO_PLATFORM > config VFIO_AMBA > tristate "VFIO support for AMBA devices" > depends on VFIO_PLATFORM && ARM_AMBA > + select VFIO_VIRQFD > help > Support for ARM AMBA devices with VFIO. This is required to make >
Re: [PATCH v14 19/20] vfio: initialize the virqfd workqueue in VFIO generic code
On Tue, 2015-03-17 at 16:29 -0600, Alex Williamson wrote: > On Mon, 2015-03-02 at 17:59 +0100, Baptiste Reynal wrote: > > From: Antonios Motakis > > > > Now we have finally completely decoupled virqfd from VFIO_PCI. We can > > initialize it from the VFIO generic code, in order to safely use it from > > multiple independent VFIO bus drivers. > > > > Signed-off-by: Antonios Motakis > > Signed-off-by: Baptiste Reynal > > --- > > drivers/vfio/Makefile | 4 +++- > > drivers/vfio/pci/Makefile | 3 +-- > > drivers/vfio/pci/vfio_pci.c | 8 > > drivers/vfio/vfio.c | 8 > > 4 files changed, 12 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > > index dadf0ca..d798b09 100644 > > --- a/drivers/vfio/Makefile > > +++ b/drivers/vfio/Makefile > > @@ -1,4 +1,6 @@ > > -obj-$(CONFIG_VFIO) += vfio.o > > +vfio_core-y := vfio.o virqfd.o > > + > > +obj-$(CONFIG_VFIO) += vfio_core.o > > obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o > > obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o > > obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o > > This inadvertently (I assume) renames the main vfio module to vfio_core. > That potentially breaks numerous userspace scripts that might try to > load the "vfio" module. I don't think that's acceptable. A brute force > way to fix this would be to rename vfio.c to vfio_core.c and change the > Makefile to: > > vfio-y := vfio_core.o virqfd.o > obj-$(CONFIG_VFIO) += vfio.o > > Is there any other trickery available to us that could include virqfd.o > in vfio.o w/o source file renaming? Thanks, Maybe a better option, we could let virqfd be it's own support module for bus drivers that need it. Then we keep it out of vfio "core". Something like this: commit f4d91ec4b72ce11e9dba861d6bf2dba93b72f0ba Author: Alex Williamson Date: Tue Mar 17 08:33:38 2015 -0600 vfio: Split virqfd into a separate module for vfio bus drivers An unintended consequence of splittng virqfd support to be shared by bus drivers is renaming the core vfio module to vfio_core. This is not very friendly to user scripts that may try to load the vfio module. To resolve that and to make it clear that virqfd is a bus driver service and not a dependency of vfio core, move this to a separate module on which the bus drivers will depend. Signed-off-by: Alex Williamson diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index d5322a4..7d092dd 100644 --- a/drivers/vfio/Kconfig +++ b/drivers/vfio/Kconfig @@ -13,6 +13,11 @@ config VFIO_SPAPR_EEH depends on EEH && VFIO_IOMMU_SPAPR_TCE default n +config VFIO_VIRQFD + tristate + depends on VFIO && EVENTFD + default n + menuconfig VFIO tristate "VFIO Non-Privileged userspace driver framework" depends on IOMMU_API diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile index d798b09..7b8a31f 100644 --- a/drivers/vfio/Makefile +++ b/drivers/vfio/Makefile @@ -1,6 +1,7 @@ -vfio_core-y := vfio.o virqfd.o +vfio_virqfd-y := virqfd.o -obj-$(CONFIG_VFIO) += vfio_core.o +obj-$(CONFIG_VFIO) += vfio.o +obj-$(CONFIG_VFIO_VIRQFD) += vfio_virqfd.o obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index c6bb5da..579d83b 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -1,6 +1,7 @@ config VFIO_PCI tristate "VFIO support for PCI devices" depends on VFIO && PCI && EVENTFD + select VFIO_VIRQFD help Support for the PCI VFIO bus driver. This is required to make use of PCI drivers using the VFIO framework. diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig index c0a3bff..4ec14af 100644 --- a/drivers/vfio/platform/Kconfig +++ b/drivers/vfio/platform/Kconfig @@ -1,6 +1,7 @@ config VFIO_PLATFORM tristate "VFIO support for platform devices" depends on VFIO && EVENTFD && ARM + select VFIO_VIRQFD help Support for platform devices with VFIO. This is required to make use of platform devices present on the system using the VFIO @@ -11,6 +12,7 @@ config VFIO_PLATFORM config VFIO_AMBA tristate "VFIO support for AMBA devices" depends on VFIO_PLATFORM && ARM_AMBA + select VFIO_VIRQFD help Support for ARM AMBA devices with VFIO. This is required to make use of ARM AMBA devices present on the system using the VFIO diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 86aac7e..0d33662 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -1552,11 +1552,6 @@ static int __init vfio_init(void) if (ret) goto err_cdev_add; - /* Start the virqfd cleanup handler used by some VFIO bus drivers */
Re: [PATCH v14 19/20] vfio: initialize the virqfd workqueue in VFIO generic code
On Mon, 2015-03-02 at 17:59 +0100, Baptiste Reynal wrote: > From: Antonios Motakis > > Now we have finally completely decoupled virqfd from VFIO_PCI. We can > initialize it from the VFIO generic code, in order to safely use it from > multiple independent VFIO bus drivers. > > Signed-off-by: Antonios Motakis > Signed-off-by: Baptiste Reynal > --- > drivers/vfio/Makefile | 4 +++- > drivers/vfio/pci/Makefile | 3 +-- > drivers/vfio/pci/vfio_pci.c | 8 > drivers/vfio/vfio.c | 8 > 4 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > index dadf0ca..d798b09 100644 > --- a/drivers/vfio/Makefile > +++ b/drivers/vfio/Makefile > @@ -1,4 +1,6 @@ > -obj-$(CONFIG_VFIO) += vfio.o > +vfio_core-y := vfio.o virqfd.o > + > +obj-$(CONFIG_VFIO) += vfio_core.o > obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o > obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o > obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o This inadvertently (I assume) renames the main vfio module to vfio_core. That potentially breaks numerous userspace scripts that might try to load the "vfio" module. I don't think that's acceptable. A brute force way to fix this would be to rename vfio.c to vfio_core.c and change the Makefile to: vfio-y := vfio_core.o virqfd.o obj-$(CONFIG_VFIO) += vfio.o Is there any other trickery available to us that could include virqfd.o in vfio.o w/o source file renaming? Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 19/20] vfio: initialize the virqfd workqueue in VFIO generic code
From: Antonios Motakis
Now we have finally completely decoupled virqfd from VFIO_PCI. We can
initialize it from the VFIO generic code, in order to safely use it from
multiple independent VFIO bus drivers.
Signed-off-by: Antonios Motakis
Signed-off-by: Baptiste Reynal
---
drivers/vfio/Makefile | 4 +++-
drivers/vfio/pci/Makefile | 3 +--
drivers/vfio/pci/vfio_pci.c | 8
drivers/vfio/vfio.c | 8
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index dadf0ca..d798b09 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,4 +1,6 @@
-obj-$(CONFIG_VFIO) += vfio.o
+vfio_core-y := vfio.o virqfd.o
+
+obj-$(CONFIG_VFIO) += vfio_core.o
obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
index c7c8644..1310792 100644
--- a/drivers/vfio/pci/Makefile
+++ b/drivers/vfio/pci/Makefile
@@ -1,5 +1,4 @@
-vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o \
- ../virqfd.o
+vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 668d37c..2f865d07 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1030,7 +1030,6 @@ put_devs:
static void __exit vfio_pci_cleanup(void)
{
pci_unregister_driver(&vfio_pci_driver);
- vfio_virqfd_exit();
vfio_pci_uninit_perm_bits();
}
@@ -1043,11 +1042,6 @@ static int __init vfio_pci_init(void)
if (ret)
return ret;
- /* Start the virqfd cleanup handler */
- ret = vfio_virqfd_init();
- if (ret)
- goto out_virqfd;
-
/* Register and scan for devices */
ret = pci_register_driver(&vfio_pci_driver);
if (ret)
@@ -1056,8 +1050,6 @@ static int __init vfio_pci_init(void)
return 0;
out_driver:
- vfio_virqfd_exit();
-out_virqfd:
vfio_pci_uninit_perm_bits();
return ret;
}
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 4cde855..23ba12a 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1553,6 +1553,11 @@ static int __init vfio_init(void)
if (ret)
goto err_cdev_add;
+ /* Start the virqfd cleanup handler used by some VFIO bus drivers */
+ ret = vfio_virqfd_init();
+ if (ret)
+ goto err_virqfd;
+
pr_info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
/*
@@ -1565,6 +1570,8 @@ static int __init vfio_init(void)
return 0;
+err_virqfd:
+ cdev_del(&vfio.group_cdev);
err_cdev_add:
unregister_chrdev_region(vfio.group_devt, MINORMASK);
err_alloc_chrdev:
@@ -1579,6 +1586,7 @@ static void __exit vfio_cleanup(void)
{
WARN_ON(!list_empty(&vfio.group_list));
+ vfio_virqfd_exit();
idr_destroy(&vfio.group_idr);
cdev_del(&vfio.group_cdev);
unregister_chrdev_region(vfio.group_devt, MINORMASK);
--
2.3.1
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
