On Tue, 6 Jun 2023 11:25:01 -0300 Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > On Mon, Jun 05, 2023 at 01:25:18PM -0600, Alex Williamson wrote: > > On Mon, 5 Jun 2023 14:01:28 -0300 > > Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > > > > > On Fri, Jun 02, 2023 at 03:33:13PM -0600, Alex Williamson wrote: > > > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > > > > index 70e7dcb302ef..151e816b2ff9 100644 > > > > --- a/drivers/vfio/Makefile > > > > +++ b/drivers/vfio/Makefile > > > > @@ -10,7 +10,7 @@ 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) += pci/ > > > > +obj-$(CONFIG_VFIO_PCI_CORE) += pci/ > > > > obj-$(CONFIG_VFIO_PLATFORM) += platform/ > > > > obj-$(CONFIG_VFIO_MDEV) += mdev/ > > > > obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/ > > > > > > This makes sense on its own even today > > > > It's only an academic fix today, currently nothing in pci/ can be > > selected without VFIO_PCI. > > > > > > diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig > > > > index f9d0c908e738..86bb7835cf3c 100644 > > > > --- a/drivers/vfio/pci/Kconfig > > > > +++ b/drivers/vfio/pci/Kconfig > > > > @@ -1,5 +1,7 @@ > > > > # SPDX-License-Identifier: GPL-2.0-only > > > > -if PCI && MMU > > > > +menu "VFIO support for PCI devices" > > > > + depends on PCI && MMU > > > > > > > > > I still think this is excessive, it is normal to hang the makefile > > > components off the kconfig for the "core". Even VFIO is already doing this: > > > > > > menuconfig VFIO > > > tristate "VFIO Non-Privileged userspace driver framework" > > > select IOMMU_API > > > depends on IOMMUFD || !IOMMUFD > > > select INTERVAL_TREE > > > select VFIO_CONTAINER if IOMMUFD=n > > > > > > [..] > > > > > > obj-$(CONFIG_VFIO) += vfio.o > > > > I think the "core" usually does something on its own though without > > out-of-tree drivers, > > Not really, maybe it creates a sysfs class, but it certainly doesn't > do anything useful unless there is a vfio driver also selected. Sorry, I wasn't referring to vfio "core" here, I was thinking more along the lines of when we include the PCI or IOMMU subsystem there's a degree of base functionality included there regardless of what additional options or drivers are selected. OTOH, if we enable CONFIG_VFIO without any in-kernel drivers for it, it's simply a waste of space. > > so I don't see this as an example of how things > > should work as much as it is another target for improvement. > > It is the common pattern in the kernel, I'm not sure where you are > getting this "improvement" idea from. Common practice or not, configurations that build and install a module that has no possibility of an in-kernel user is a waste of time and space, which leaves room for improvement. > > Ideally I think we'd still have a top level menuconfig, but it should > > look more like VIRT_DRIVERS, which just enables Makefile traversal and > > unhides menu options. It should be things like VFIO_PCI_CORE or > > VFIO_MDEV that actually select VFIO. > > There are many ways to use kconfig, but I think this is not typical > usage and becomes over complicated to solve an unimportant problem. > > The menu configs follow the makefiles which is nice and simple to > understand and implement. But leaves open the possibility of building and installing modules that have no users, which therefore make them open for improvement. I don't see anything overly complicated in this series. We certainly have more important topics to quibble about than a select or depend, but here we are. The current state is that we cannot build vfio-pci-core.ko without vfio-pci.ko, so there's always an in-kernel user. The proposal which allows building vfio-pci-core.ko w/o any in-kernel users is therefore a regression (imo) prompting this alternative. CONFIG_VFIO is a separate pre-existing issue. Thanks, Alex
