Re: [PATCH 12/12] vfio/pci: Introduce vfio_pci_core.ko

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 28, 2021 at 2:03 PM Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
>
> On Wed, Jul 28, 2021 at 07:43:06AM +0200, Christoph Hellwig wrote:
>
> > > Which might reasonably be from an old kernel. 'make oldconfig' prompts:
> > >
> > > VFIO Non-Privileged userspace driver framework (VFIO) [Y/n/m/?] y
> > >   VFIO No-IOMMU support (VFIO_NOIOMMU) [Y/n/?] y
> > >   VFIO support for PCI devices (VFIO_PCI_CORE) [N/m/y/?] (NEW)
> > >
> > > Which is completely fine, IMHO.
> >
> > Why do we need to have VFIO_PCI_CORE as a user visible option?
> > I'd just select it.
>
> I'm not great with kconfig, but AFAIK:
>
> - It controls building a module so it needs to be a tristate
>
> - tristates need to be exposed in the menu structure
>
> - As it builds a module it also has depends on other things
>
> - Select should not be used to target tristates
>
> - Select should not be used to target options in the menu tree
>
> - Select should not be used to target options that have depends
>
> Which leaves us with this arrangement unless we delete the
> vfio_pci_core.ko module - which seems like a bad direction just for
> kconfig backwards compatibility.

I have not looked at the requirements for this particular patch, but
generally speaking there is no problem with using 'select' on
a tristate symbol.

The other points are correct though: you can not 'select' a symbol
that has dependencies, unless the symbol selecting it already
depends on those same options, and you should not 'select' user
visible options or other subsystems.

One common mistake is to have a reverse dependency, where
A uses 'select B' or 'depends on B', but then exports an ELF
symbol that is consumed by B, as opposed to the other way round.
I don't think that is a problem here though.

            Arnd



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux