| Message ID | 20230602213315.2521442-3-alex.williamson@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio: Cleanup Kconfigs | expand |
On 6/2/23 23:33, Alex Williamson wrote: > Like vfio-pci, there's also a base module here where vfio-amba depends on > vfio-platform, when really it only needs vfio-platform-base. Create a > sub-menu for platform drivers and a nested menu for reset drivers. Cleanup > Makefile to make use of new CONFIG_VFIO_PLATFORM_BASE for building the > shared modules and traversing reset modules. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > drivers/vfio/Makefile | 2 +- > drivers/vfio/platform/Kconfig | 17 ++++++++++++++--- > drivers/vfio/platform/Makefile | 9 +++------ > 3 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > index 151e816b2ff9..8da44aa1ea16 100644 > --- a/drivers/vfio/Makefile > +++ b/drivers/vfio/Makefile > @@ -11,6 +11,6 @@ vfio-$(CONFIG_VFIO_VIRQFD) += 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_PCI_CORE) += pci/ > -obj-$(CONFIG_VFIO_PLATFORM) += platform/ > +obj-$(CONFIG_VFIO_PLATFORM_BASE) += platform/ > obj-$(CONFIG_VFIO_MDEV) += mdev/ > obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ > diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig > index 331a5920f5ab..6d18faa66a2e 100644 > --- a/drivers/vfio/platform/Kconfig > +++ b/drivers/vfio/platform/Kconfig > @@ -1,8 +1,14 @@ > # SPDX-License-Identifier: GPL-2.0-only > +menu "VFIO support for platform devices" > + > +config VFIO_PLATFORM_BASE > + tristate > + > config VFIO_PLATFORM > - tristate "VFIO support for platform devices" > + tristate "Generic VFIO support for any platform device" > depends on ARM || ARM64 || COMPILE_TEST > select VFIO_VIRQFD > + select VFIO_PLATFORM_BASE > help > Support for platform devices with VFIO. This is required to make > use of platform devices present on the system using the VFIO > @@ -10,10 +16,11 @@ config VFIO_PLATFORM > > If you don't know what to do here, say N. > > -if VFIO_PLATFORM > config VFIO_AMBA > tristate "VFIO support for AMBA devices" > depends on ARM_AMBA || COMPILE_TEST > + select VFIO_VIRQFD > + select VFIO_PLATFORM_BASE > 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 > @@ -21,5 +28,9 @@ config VFIO_AMBA > > If you don't know what to do here, say N. > > +menu "VFIO platform reset drivers" > + depends on VFIO_PLATFORM_BASE > + > source "drivers/vfio/platform/reset/Kconfig" > -endif > +endmenu > +endmenu > diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile > index 3f3a24e7c4ef..ee4fb6a82ca8 100644 > --- a/drivers/vfio/platform/Makefile > +++ b/drivers/vfio/platform/Makefile > @@ -1,13 +1,10 @@ > # SPDX-License-Identifier: GPL-2.0 > vfio-platform-base-y := vfio_platform_common.o vfio_platform_irq.o > -vfio-platform-y := vfio_platform.o > +obj-$(CONFIG_VFIO_PLATFORM_BASE) += vfio-platform-base.o > +obj-$(CONFIG_VFIO_PLATFORM_BASE) += reset/ > > +vfio-platform-y := vfio_platform.o > obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o > -obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o > -obj-$(CONFIG_VFIO_PLATFORM) += reset/ > > vfio-amba-y := vfio_amba.o > - > obj-$(CONFIG_VFIO_AMBA) += vfio-amba.o > -obj-$(CONFIG_VFIO_AMBA) += vfio-platform-base.o > -obj-$(CONFIG_VFIO_AMBA) += reset/
Hi Alex, On 6/2/23 23:33, Alex Williamson wrote: > Like vfio-pci, there's also a base module here where vfio-amba depends on > vfio-platform, when really it only needs vfio-platform-base. Create a > sub-menu for platform drivers and a nested menu for reset drivers. Cleanup > Makefile to make use of new CONFIG_VFIO_PLATFORM_BASE for building the > shared modules and traversing reset modules. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > drivers/vfio/Makefile | 2 +- > drivers/vfio/platform/Kconfig | 17 ++++++++++++++--- > drivers/vfio/platform/Makefile | 9 +++------ > 3 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > index 151e816b2ff9..8da44aa1ea16 100644 > --- a/drivers/vfio/Makefile > +++ b/drivers/vfio/Makefile > @@ -11,6 +11,6 @@ vfio-$(CONFIG_VFIO_VIRQFD) += 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_PCI_CORE) += pci/ > -obj-$(CONFIG_VFIO_PLATFORM) += platform/ > +obj-$(CONFIG_VFIO_PLATFORM_BASE) += platform/ > obj-$(CONFIG_VFIO_MDEV) += mdev/ > obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ > diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig > index 331a5920f5ab..6d18faa66a2e 100644 > --- a/drivers/vfio/platform/Kconfig > +++ b/drivers/vfio/platform/Kconfig > @@ -1,8 +1,14 @@ > # SPDX-License-Identifier: GPL-2.0-only > +menu "VFIO support for platform devices" > + > +config VFIO_PLATFORM_BASE > + tristate > + > config VFIO_PLATFORM > - tristate "VFIO support for platform devices" > + tristate "Generic VFIO support for any platform device" > depends on ARM || ARM64 || COMPILE_TEST I wonder if we couldn't put those dependencies at the menu level. I guess this also applies to AMBA. And just leave 'depends on ARM_AMBA ' in config VFIO_AMBA? > select VFIO_VIRQFD > + select VFIO_PLATFORM_BASE > help > Support for platform devices with VFIO. This is required to make > use of platform devices present on the system using the VFIO > @@ -10,10 +16,11 @@ config VFIO_PLATFORM > > If you don't know what to do here, say N. > > -if VFIO_PLATFORM > config VFIO_AMBA > tristate "VFIO support for AMBA devices" > depends on ARM_AMBA || COMPILE_TEST > + select VFIO_VIRQFD > + select VFIO_PLATFORM_BASE > 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 > @@ -21,5 +28,9 @@ config VFIO_AMBA > > If you don't know what to do here, say N. > > +menu "VFIO platform reset drivers" > + depends on VFIO_PLATFORM_BASE I wonder if this shouldn't depend on VFIO_PLATFORM instead? There are no amba reset devices at the moment so why whould be compile them if VFIO_AMBA is set (which is unlikely by the way)? Eric > + > source "drivers/vfio/platform/reset/Kconfig" > -endif > +endmenu > +endmenu > diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile > index 3f3a24e7c4ef..ee4fb6a82ca8 100644 > --- a/drivers/vfio/platform/Makefile > +++ b/drivers/vfio/platform/Makefile > @@ -1,13 +1,10 @@ > # SPDX-License-Identifier: GPL-2.0 > vfio-platform-base-y := vfio_platform_common.o vfio_platform_irq.o > -vfio-platform-y := vfio_platform.o > +obj-$(CONFIG_VFIO_PLATFORM_BASE) += vfio-platform-base.o > +obj-$(CONFIG_VFIO_PLATFORM_BASE) += reset/ > > +vfio-platform-y := vfio_platform.o > obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o > -obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o > -obj-$(CONFIG_VFIO_PLATFORM) += reset/ > > vfio-amba-y := vfio_amba.o > - > obj-$(CONFIG_VFIO_AMBA) += vfio-amba.o > -obj-$(CONFIG_VFIO_AMBA) += vfio-platform-base.o > -obj-$(CONFIG_VFIO_AMBA) += reset/
On Wed, 7 Jun 2023 15:32:07 +0200 Eric Auger <eric.auger@redhat.com> wrote: > Hi Alex, > > On 6/2/23 23:33, Alex Williamson wrote: > > Like vfio-pci, there's also a base module here where vfio-amba depends on > > vfio-platform, when really it only needs vfio-platform-base. Create a > > sub-menu for platform drivers and a nested menu for reset drivers. Cleanup > > Makefile to make use of new CONFIG_VFIO_PLATFORM_BASE for building the > > shared modules and traversing reset modules. > > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > > --- > > drivers/vfio/Makefile | 2 +- > > drivers/vfio/platform/Kconfig | 17 ++++++++++++++--- > > drivers/vfio/platform/Makefile | 9 +++------ > > 3 files changed, 18 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > > index 151e816b2ff9..8da44aa1ea16 100644 > > --- a/drivers/vfio/Makefile > > +++ b/drivers/vfio/Makefile > > @@ -11,6 +11,6 @@ vfio-$(CONFIG_VFIO_VIRQFD) += 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_PCI_CORE) += pci/ > > -obj-$(CONFIG_VFIO_PLATFORM) += platform/ > > +obj-$(CONFIG_VFIO_PLATFORM_BASE) += platform/ > > obj-$(CONFIG_VFIO_MDEV) += mdev/ > > obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ > > diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig > > index 331a5920f5ab..6d18faa66a2e 100644 > > --- a/drivers/vfio/platform/Kconfig > > +++ b/drivers/vfio/platform/Kconfig > > @@ -1,8 +1,14 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > +menu "VFIO support for platform devices" > > + > > +config VFIO_PLATFORM_BASE > > + tristate > > + > > config VFIO_PLATFORM > > - tristate "VFIO support for platform devices" > > + tristate "Generic VFIO support for any platform device" > > depends on ARM || ARM64 || COMPILE_TEST > I wonder if we couldn't put those dependencies at the menu level. I > guess this also applies to AMBA. And just leave 'depends on ARM_AMBA ' in > > config VFIO_AMBA? Yup, we could, something like: menu "VFIO support for platform devices" depends on ARM || ARM64 || COMPILE_TEST And we could move VFIO_VIRQFD to VFIO_PLATFORM_BASE config VFIO_PLATFORM_BASE tristate select VFIO_VIRQFD VFIO_AMBA would then only depend on ARM_AMBA and both would select VFIO_PLATFORM_BASE. > > > select VFIO_VIRQFD > > + select VFIO_PLATFORM_BASE > > help > > Support for platform devices with VFIO. This is required to make > > use of platform devices present on the system using the VFIO > > @@ -10,10 +16,11 @@ config VFIO_PLATFORM > > > > If you don't know what to do here, say N. > > > > -if VFIO_PLATFORM > > config VFIO_AMBA > > tristate "VFIO support for AMBA devices" > > depends on ARM_AMBA || COMPILE_TEST > > + select VFIO_VIRQFD > > + select VFIO_PLATFORM_BASE > > 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 > > @@ -21,5 +28,9 @@ config VFIO_AMBA > > > > If you don't know what to do here, say N. > > > > +menu "VFIO platform reset drivers" > > + depends on VFIO_PLATFORM_BASE > I wonder if this shouldn't depend on VFIO_PLATFORM instead? > There are no amba reset devices at the moment so why whould be compile > them if VFIO_AMBA is set (which is unlikely by the way)? I did see that AMBA sets reset_required = false, but at the same time the handling of reset modules is in the base driver, so if there were an AMBA reset driver, wouldn't it also live in the reset/ directory? It seems like we'd therefore want to traverse into reset/Kconfig, but maybe if all the current config options in there are non-AMBA we should wrap them in 'if VFIO_PLATFORM' (or 'depends on' for each, but the 'if' is marginally cleaner). Thanks, Alex
Hi Alex, On 6/7/23 21:04, Alex Williamson wrote: > On Wed, 7 Jun 2023 15:32:07 +0200 > Eric Auger <eric.auger@redhat.com> wrote: > >> Hi Alex, >> >> On 6/2/23 23:33, Alex Williamson wrote: >>> Like vfio-pci, there's also a base module here where vfio-amba depends on >>> vfio-platform, when really it only needs vfio-platform-base. Create a >>> sub-menu for platform drivers and a nested menu for reset drivers. Cleanup >>> Makefile to make use of new CONFIG_VFIO_PLATFORM_BASE for building the >>> shared modules and traversing reset modules. >>> >>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> >>> --- >>> drivers/vfio/Makefile | 2 +- >>> drivers/vfio/platform/Kconfig | 17 ++++++++++++++--- >>> drivers/vfio/platform/Makefile | 9 +++------ >>> 3 files changed, 18 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile >>> index 151e816b2ff9..8da44aa1ea16 100644 >>> --- a/drivers/vfio/Makefile >>> +++ b/drivers/vfio/Makefile >>> @@ -11,6 +11,6 @@ vfio-$(CONFIG_VFIO_VIRQFD) += 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_PCI_CORE) += pci/ >>> -obj-$(CONFIG_VFIO_PLATFORM) += platform/ >>> +obj-$(CONFIG_VFIO_PLATFORM_BASE) += platform/ >>> obj-$(CONFIG_VFIO_MDEV) += mdev/ >>> obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ >>> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig >>> index 331a5920f5ab..6d18faa66a2e 100644 >>> --- a/drivers/vfio/platform/Kconfig >>> +++ b/drivers/vfio/platform/Kconfig >>> @@ -1,8 +1,14 @@ >>> # SPDX-License-Identifier: GPL-2.0-only >>> +menu "VFIO support for platform devices" >>> + >>> +config VFIO_PLATFORM_BASE >>> + tristate >>> + >>> config VFIO_PLATFORM >>> - tristate "VFIO support for platform devices" >>> + tristate "Generic VFIO support for any platform device" >>> depends on ARM || ARM64 || COMPILE_TEST >> I wonder if we couldn't put those dependencies at the menu level. I >> guess this also applies to AMBA. And just leave 'depends on ARM_AMBA ' in >> >> config VFIO_AMBA? > Yup, we could, something like: > > menu "VFIO support for platform devices" > depends on ARM || ARM64 || COMPILE_TEST > > And we could move VFIO_VIRQFD to VFIO_PLATFORM_BASE > > config VFIO_PLATFORM_BASE > tristate > select VFIO_VIRQFD > > VFIO_AMBA would then only depend on ARM_AMBA and both would select > VFIO_PLATFORM_BASE. > >>> select VFIO_VIRQFD >>> + select VFIO_PLATFORM_BASE >>> help >>> Support for platform devices with VFIO. This is required to make >>> use of platform devices present on the system using the VFIO >>> @@ -10,10 +16,11 @@ config VFIO_PLATFORM >>> >>> If you don't know what to do here, say N. >>> >>> -if VFIO_PLATFORM >>> config VFIO_AMBA >>> tristate "VFIO support for AMBA devices" >>> depends on ARM_AMBA || COMPILE_TEST >>> + select VFIO_VIRQFD >>> + select VFIO_PLATFORM_BASE >>> 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 >>> @@ -21,5 +28,9 @@ config VFIO_AMBA >>> >>> If you don't know what to do here, say N. >>> >>> +menu "VFIO platform reset drivers" >>> + depends on VFIO_PLATFORM_BASE >> I wonder if this shouldn't depend on VFIO_PLATFORM instead? >> There are no amba reset devices at the moment so why whould be compile >> them if VFIO_AMBA is set (which is unlikely by the way)? > I did see that AMBA sets reset_required = false, but at the same time > the handling of reset modules is in the base driver, so if there were > an AMBA reset driver, wouldn't it also live in the reset/ directory? Yes I guess so. > It seems like we'd therefore want to traverse into reset/Kconfig, but > maybe if all the current config options in there are non-AMBA we should > wrap them in 'if VFIO_PLATFORM' (or 'depends on' for each, but the 'if' > is marginally cleaner). Thanks, Yes your v2 makes sense to me. Eric > > Alex >
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile index 151e816b2ff9..8da44aa1ea16 100644 --- a/drivers/vfio/Makefile +++ b/drivers/vfio/Makefile @@ -11,6 +11,6 @@ vfio-$(CONFIG_VFIO_VIRQFD) += 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_PCI_CORE) += pci/ -obj-$(CONFIG_VFIO_PLATFORM) += platform/ +obj-$(CONFIG_VFIO_PLATFORM_BASE) += platform/ obj-$(CONFIG_VFIO_MDEV) += mdev/ obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig index 331a5920f5ab..6d18faa66a2e 100644 --- a/drivers/vfio/platform/Kconfig +++ b/drivers/vfio/platform/Kconfig @@ -1,8 +1,14 @@ # SPDX-License-Identifier: GPL-2.0-only +menu "VFIO support for platform devices" + +config VFIO_PLATFORM_BASE + tristate + config VFIO_PLATFORM - tristate "VFIO support for platform devices" + tristate "Generic VFIO support for any platform device" depends on ARM || ARM64 || COMPILE_TEST select VFIO_VIRQFD + select VFIO_PLATFORM_BASE help Support for platform devices with VFIO. This is required to make use of platform devices present on the system using the VFIO @@ -10,10 +16,11 @@ config VFIO_PLATFORM If you don't know what to do here, say N. -if VFIO_PLATFORM config VFIO_AMBA tristate "VFIO support for AMBA devices" depends on ARM_AMBA || COMPILE_TEST + select VFIO_VIRQFD + select VFIO_PLATFORM_BASE 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 @@ -21,5 +28,9 @@ config VFIO_AMBA If you don't know what to do here, say N. +menu "VFIO platform reset drivers" + depends on VFIO_PLATFORM_BASE + source "drivers/vfio/platform/reset/Kconfig" -endif +endmenu +endmenu diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile index 3f3a24e7c4ef..ee4fb6a82ca8 100644 --- a/drivers/vfio/platform/Makefile +++ b/drivers/vfio/platform/Makefile @@ -1,13 +1,10 @@ # SPDX-License-Identifier: GPL-2.0 vfio-platform-base-y := vfio_platform_common.o vfio_platform_irq.o -vfio-platform-y := vfio_platform.o +obj-$(CONFIG_VFIO_PLATFORM_BASE) += vfio-platform-base.o +obj-$(CONFIG_VFIO_PLATFORM_BASE) += reset/ +vfio-platform-y := vfio_platform.o obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o -obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o -obj-$(CONFIG_VFIO_PLATFORM) += reset/ vfio-amba-y := vfio_amba.o - obj-$(CONFIG_VFIO_AMBA) += vfio-amba.o -obj-$(CONFIG_VFIO_AMBA) += vfio-platform-base.o -obj-$(CONFIG_VFIO_AMBA) += reset/
Like vfio-pci, there's also a base module here where vfio-amba depends on vfio-platform, when really it only needs vfio-platform-base. Create a sub-menu for platform drivers and a nested menu for reset drivers. Cleanup Makefile to make use of new CONFIG_VFIO_PLATFORM_BASE for building the shared modules and traversing reset modules. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- drivers/vfio/Makefile | 2 +- drivers/vfio/platform/Kconfig | 17 ++++++++++++++--- drivers/vfio/platform/Makefile | 9 +++------ 3 files changed, 18 insertions(+), 10 deletions(-)