linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Christoph Hellwig <hch@lst.de>,
	Alex Williamson <alex.williamson@redhat.com>,
	Arnd Bergmann <arnd@arndb.de>, Yishai Hadas <yishaih@nvidia.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	diana.craciun@oss.nxp.com, kwankhede@nvidia.com,
	Eric Auger <eric.auger@redhat.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	linux-pci <linux-pci@vger.kernel.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	kvm list <kvm@vger.kernel.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	mgurtovoy@nvidia.com, maorg@nvidia.com, leonro@nvidia.com
Subject: Re: [PATCH 12/12] vfio/pci: Introduce vfio_pci_core.ko
Date: Wed, 28 Jul 2021 14:12:15 +0200	[thread overview]
Message-ID: <CAK8P3a1=sGwYqVoZzOdgrs7d6v3dwOFC9Q+pROnPB5F3Qe-5CA@mail.gmail.com> (raw)
In-Reply-To: <20210728120326.GQ1721383@nvidia.com>

On Wed, Jul 28, 2021 at 2:03 PM Jason Gunthorpe <jgg@nvidia.com> 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

  reply	other threads:[~2021-07-28 12:12 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21 16:15 [PATCH 00/12] Introduce vfio_pci_core subsystem Yishai Hadas
2021-07-21 16:15 ` [PATCH 01/12] vfio/pci: Rename vfio_pci.c to vfio_pci_core.c Yishai Hadas
2021-07-21 16:15 ` [PATCH 02/12] vfio/pci: Rename vfio_pci_private.h to vfio_pci_core.h Yishai Hadas
2021-07-21 16:16 ` [PATCH 03/12] vfio/pci: Rename vfio_pci_device to vfio_pci_core_device Yishai Hadas
2021-07-21 16:16 ` [PATCH 04/12] vfio/pci: Rename ops functions to fit core namings Yishai Hadas
2021-07-21 16:16 ` [PATCH 05/12] vfio/pci: Include vfio header in vfio_pci_core.h Yishai Hadas
2021-07-21 16:16 ` [PATCH 06/12] vfio/pci: Split the pci_driver code out of vfio_pci_core.c Yishai Hadas
2021-07-21 16:16 ` [PATCH 07/12] vfio/pci: Move igd initialization to vfio_pci.c Yishai Hadas
2021-07-21 16:16 ` [PATCH 08/12] vfio/pci: Move module parameters " Yishai Hadas
2021-07-21 16:16 ` [PATCH 09/12] PCI: Add a PCI_ID_F_VFIO_DRIVER_OVERRIDE flag to struct pci_device_id Yishai Hadas
2021-07-27 16:34   ` Alex Williamson
2021-07-27 17:14     ` Jason Gunthorpe
2021-07-27 23:02       ` Alex Williamson
2021-07-27 23:42         ` Jason Gunthorpe
2021-08-04 20:34   ` Bjorn Helgaas
2021-08-05 16:47     ` Max Gurtovoy
2021-08-06  0:23     ` Jason Gunthorpe
2021-08-11 12:22       ` Max Gurtovoy
2021-08-11 19:07       ` Bjorn Helgaas
2021-08-12 13:27         ` Jason Gunthorpe
2021-08-12 15:57           ` Bjorn Helgaas
2021-08-12 19:51             ` Jason Gunthorpe
2021-08-12 20:26               ` Bjorn Helgaas
2021-08-12 23:21                 ` Max Gurtovoy
2021-08-13 17:44                   ` Bjorn Helgaas
2021-08-14 23:27                     ` Max Gurtovoy
2021-08-16 17:21                       ` Bjorn Helgaas
2021-08-17 13:01                         ` Max Gurtovoy
2021-08-17 14:13                           ` Bjorn Helgaas
2021-08-17 14:44                             ` Max Gurtovoy
2021-08-12 15:42   ` Bjorn Helgaas
2021-07-21 16:16 ` [PATCH 10/12] vfio: Use select for eventfd Yishai Hadas
2021-07-21 16:16 ` [PATCH 11/12] vfio: Use kconfig if XX/endif blocks instead of repeating 'depends on' Yishai Hadas
2021-07-21 16:16 ` [PATCH 12/12] vfio/pci: Introduce vfio_pci_core.ko Yishai Hadas
2021-07-21 17:39   ` Leon Romanovsky
2021-07-22  9:06     ` Yishai Hadas
2021-07-22  9:22       ` Max Gurtovoy
2021-07-23 14:13         ` Leon Romanovsky
2021-07-25 10:45           ` Max Gurtovoy
2021-07-27 21:54   ` Alex Williamson
2021-07-27 23:09     ` Jason Gunthorpe
2021-07-28  4:56       ` Leon Romanovsky
2021-07-28  5:43       ` Christoph Hellwig
2021-07-28  7:04         ` Arnd Bergmann
2021-07-28  7:17           ` Leon Romanovsky
2021-07-28 12:03         ` Jason Gunthorpe
2021-07-28 12:12           ` Arnd Bergmann [this message]
2021-07-28 12:29           ` Christoph Hellwig
2021-07-28 12:47             ` Jason Gunthorpe
2021-07-28 12:55               ` Christoph Hellwig
2021-07-28 13:31                 ` Jason Gunthorpe
2021-07-28 13:08               ` Arnd Bergmann
2021-07-28 17:26                 ` Jason Gunthorpe
2021-08-04 13:41 ` [PATCH 00/12] Introduce vfio_pci_core subsystem Yishai Hadas
2021-08-04 15:27   ` Alex Williamson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAK8P3a1=sGwYqVoZzOdgrs7d6v3dwOFC9Q+pROnPB5F3Qe-5CA@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=alex.williamson@redhat.com \
    --cc=bhelgaas@google.com \
    --cc=corbet@lwn.net \
    --cc=diana.craciun@oss.nxp.com \
    --cc=eric.auger@redhat.com \
    --cc=hch@lst.de \
    --cc=jgg@nvidia.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=leonro@nvidia.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=maorg@nvidia.com \
    --cc=masahiroy@kernel.org \
    --cc=mgurtovoy@nvidia.com \
    --cc=michal.lkml@markovi.net \
    --cc=yishaih@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).