On Tue, 08 Nov 2016 10:49:35 +1100 Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote: > Alex Williamson <alex.williamson@xxxxxxxxxx> writes: > > On Mon, 07 Nov 2016 19:34:42 +1100 > > Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote: > >> Paolo Bonzini <pbonzini@xxxxxxxxxx> writes: > >> > On 04/11/2016 06:48, Michael Ellerman wrote: > >> >> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > >> >> index da6e2ce77495..6b51a4ebed8a 100644 > >> >> --- a/drivers/vfio/Kconfig > >> >> +++ b/drivers/vfio/Kconfig > >> >> @@ -6,12 +6,12 @@ config VFIO_IOMMU_TYPE1 > >> >> config VFIO_IOMMU_SPAPR_TCE > >> >> tristate > >> >> depends on VFIO && SPAPR_TCE_IOMMU > >> >> - default n > >> >> + default VFIO > >> > > >> > No need to depend on VFIO since you already have it in "default". > > > > depends and defaults are different beasts though, if VFIO is not > > enabled and we're not on a powerpc system with SPAPR, > > VFIO_IOMMU_SPAPR_TCE should not be selectable, not just default to 'n'. > > Right. But dropping VFIO from the depends won't change that. In fact it > has no effect because the entire directory is not built if VFIO=n. > > See drivers/Makefile: > > obj-$(CONFIG_VFIO) += vfio/ > > > So we don't need the depends on VFIO there. > > >> > A shorthand is > >> > > >> > def_tristate VFIO && SPAPR_TCE_IOMMU > >> > >> Yep. My experience though is that a lot of folks don't really know what > >> that means. So I prefer to spell it out with an explicit type, depends > >> and default. > >> > >> But I'll respin it that way if Alex prefers the shorter style. > > > > Perhaps I'm one of those people. Non-powerpc archs should not have an > > option to select this, which is why the depends is there, AIUI. So > > long as we don't start exposing options that aren't relevant to a > > platform, I'm flexible on what shorthands we use, but you may need to > > teach me about them first. Thanks, > > Using the def_tristate trick won't expose the option to users, because > it has no description it's not user selectable. But it does make the > symbol exist on an x86 build, and it appears in the .config. > > eg, using def_tristate you get: > > # CONFIG_VFIO_IOMMU_SPAPR_TCE is not set > # CONFIG_VFIO is not set > > Whereas using depends all you get is: > > # CONFIG_VFIO is not set > > > So using def_tristate in this case is not entirely equivalent. I get a surprising number of users trying to manually hack on their .config file, so I'd prefer it not appear in the .config unless CONFIG_VFIO is y/m _and_ the user is actually building to a config where SPAPR is relevant. I'm sure it will save me at least a few questions in the future. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
