On Wed, 7 Jun 2023 15:32:07 +0200 Eric Auger <eric.auger@xxxxxxxxxx> 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@xxxxxxxxxx> > > --- > > 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
