[PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

The xen-platform-pci module is small and for PV on HVM guests is a
requirement for xenbus.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/xen/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 5f7ff8e..a64b8e8 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -138,9 +138,9 @@ config XEN_GRANT_DEV_ALLOC
 	  or as part of an inter-domain shared memory channel.
 
 config XEN_PLATFORM_PCI
-	tristate "xen platform pci device driver"
+	bool "xen platform pci device driver"
 	depends on XEN_PVHVM && PCI
-	default m
+	default y
 	help
 	  Driver for the Xen PCI Platform device: it is responsible for
 	  initializing xenbus and grant_table when running in a Xen HVM
-- 
1.7.2.3

Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Konrad Rzeszutek Wilk]

On Wed, Sep 21, 2011 at 01:37:50PM +0100, stefano.stabellini@eu.citrix.com wrote:
> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> The xen-platform-pci module is small and for PV on HVM guests is a

How small? Does it get removed from memory if it never gets loaded?

> requirement for xenbus.

Ok, should it then have a depency on XenBus as well?

Linus does not like the 'default y' very much. He actually dislikes
it quite much as I found when he tore Dan's behind about cleancache.

.. so I think making it 'default n' is a better option or perhaps
making it depend on some other functionality? Or perhaps just remove
the tristate/bool altogether so it gets activated if XEN_PVHVM
is set?

Or remove the XEN_PLATFORM_PCI config option completly and make the
config files that build this driver be CONFIG_XENPVHM dependent?

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  drivers/xen/Kconfig |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 5f7ff8e..a64b8e8 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -138,9 +138,9 @@ config XEN_GRANT_DEV_ALLOC
>  	  or as part of an inter-domain shared memory channel.
>  
>  config XEN_PLATFORM_PCI
> -	tristate "xen platform pci device driver"
> +	bool "xen platform pci device driver"
>  	depends on XEN_PVHVM && PCI
> -	default m
> +	default y
>  	help
>  	  Driver for the Xen PCI Platform device: it is responsible for
>  	  initializing xenbus and grant_table when running in a Xen HVM
> -- 
> 1.7.2.3

Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Ian Campbell]

On Wed, 2011-09-21 at 19:51 +0100, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 21, 2011 at 01:37:50PM +0100, stefano.stabellini@eu.citrix.com wrote:
> > From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > The xen-platform-pci module is small and for PV on HVM guests is a
> 
> How small?

IIRC it is single digit numbers of kb.

>  Does it get removed from memory if it never gets loaded?
> 
> > requirement for xenbus.
> 
> Ok, should it then have a depency on XenBus as well?

xenbus can't be a module (which is why allowing platform-pci to be is
causing problems).

> Linus does not like the 'default y' very much. He actually dislikes
> it quite much as I found when he tore Dan's behind about cleancache.

In particular case the option is gated on a dependency on another Xen
option (PVHVM) which doesn't default on. But if you do select PVHVM you
certainly want this option, so I think that's ok (why else would
'default y' even exist?)

> .. so I think making it 'default n' is a better option or perhaps
> making it depend on some other functionality? Or perhaps just remove
> the tristate/bool altogether so it gets activated if XEN_PVHVM
> is set?
> 
> Or remove the XEN_PLATFORM_PCI config option completly and make the
> config files that build this driver be CONFIG_XENPVHM dependent?

That would work too. Even better would be to make it an invisible
Kconfig symbol which PVHVM just selects.

Ian.

> 
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  drivers/xen/Kconfig |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > index 5f7ff8e..a64b8e8 100644
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -138,9 +138,9 @@ config XEN_GRANT_DEV_ALLOC
> >  	  or as part of an inter-domain shared memory channel.
> >  
> >  config XEN_PLATFORM_PCI
> > -	tristate "xen platform pci device driver"
> > +	bool "xen platform pci device driver"
> >  	depends on XEN_PVHVM && PCI
> > -	default m
> > +	default y
> >  	help
> >  	  Driver for the Xen PCI Platform device: it is responsible for
> >  	  initializing xenbus and grant_table when running in a Xen HVM
> > -- 
> > 1.7.2.3

Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Konrad Rzeszutek Wilk]

On Wed, Sep 21, 2011 at 08:08:15PM +0100, Ian Campbell wrote:
> On Wed, 2011-09-21 at 19:51 +0100, Konrad Rzeszutek Wilk wrote:
> > On Wed, Sep 21, 2011 at 01:37:50PM +0100, stefano.stabellini@eu.citrix.com wrote:
> > > From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > 
> > > The xen-platform-pci module is small and for PV on HVM guests is a
> > 
> > How small?
> 
> IIRC it is single digit numbers of kb.
> 
> >  Does it get removed from memory if it never gets loaded?
> > 
> > > requirement for xenbus.
> > 
> > Ok, should it then have a depency on XenBus as well?
> 
> xenbus can't be a module (which is why allowing platform-pci to be is
> causing problems).
> 
> > Linus does not like the 'default y' very much. He actually dislikes
> > it quite much as I found when he tore Dan's behind about cleancache.
> 
> In particular case the option is gated on a dependency on another Xen
> option (PVHVM) which doesn't default on. But if you do select PVHVM you
> certainly want this option, so I think that's ok (why else would
> 'default y' even exist?)
> 
> > .. so I think making it 'default n' is a better option or perhaps
> > making it depend on some other functionality? Or perhaps just remove
> > the tristate/bool altogether so it gets activated if XEN_PVHVM
> > is set?
> > 
> > Or remove the XEN_PLATFORM_PCI config option completly and make the
> > config files that build this driver be CONFIG_XENPVHM dependent?
> 
> That would work too. Even better would be to make it an invisible
> Kconfig symbol which PVHVM just selects.
<nods>
Or that since you can't really do PVHVM without the platform PCI driver.

Re: Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Jeremy Fitzhardinge]

On 09/21/2011 12:08 PM, Ian Campbell wrote:
> On Wed, 2011-09-21 at 19:51 +0100, Konrad Rzeszutek Wilk wrote:
>> On Wed, Sep 21, 2011 at 01:37:50PM +0100, stefano.stabellini@eu.citrix.com wrote:
>>> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>
>>> The xen-platform-pci module is small and for PV on HVM guests is a
>> How small?
> IIRC it is single digit numbers of kb.

lsmod shows it's about 2.5k.

>>  Does it get removed from memory if it never gets loaded?

Nope.

>>> requirement for xenbus.
>> Ok, should it then have a depency on XenBus as well?
> xenbus can't be a module (which is why allowing platform-pci to be is
> causing problems).
>
>> Linus does not like the 'default y' very much. He actually dislikes
>> it quite much as I found when he tore Dan's behind about cleancache.
> In particular case the option is gated on a dependency on another Xen
> option (PVHVM) which doesn't default on. But if you do select PVHVM you
> certainly want this option, so I think that's ok (why else would
> 'default y' even exist?)

It was default 'm' before, so making it 'y' is just the logical mapping
of tristate->bool.

>> .. so I think making it 'default n' is a better option or perhaps
>> making it depend on some other functionality? Or perhaps just remove
>> the tristate/bool altogether so it gets activated if XEN_PVHVM
>> is set?
>>
>> Or remove the XEN_PLATFORM_PCI config option completly and make the
>> config files that build this driver be CONFIG_XENPVHM dependent?
> That would work too. Even better would be to make it an invisible
> Kconfig symbol which PVHVM just selects.

Eh, select is pretty nasty.

    J

Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Olaf Hering]

On Wed, Sep 21, stefano.stabellini@eu.citrix.com wrote:

> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> The xen-platform-pci module is small and for PV on HVM guests is a
> requirement for xenbus.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Because it depends on XEN_PVHVM and XEN_PVHVM usage depends on XEN_PLATFORM_PCI:

Acked-by: Olaf Hering <olaf@aepfle.de>

Re: Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Ian Campbell]

On Wed, 2011-09-21 at 23:49 +0100, Jeremy Fitzhardinge wrote:
> > That would work too. Even better would be to make it an invisible
> > Kconfig symbol which PVHVM just selects.
> 
> Eh, select is pretty nasty.

Select of a non user visible symbol is perfectly fine. It's only when
you select something a user can also set that things get nasty.

Ian.

Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Stefano Stabellini]

On Wed, 21 Sep 2011, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 21, 2011 at 08:08:15PM +0100, Ian Campbell wrote:
> > On Wed, 2011-09-21 at 19:51 +0100, Konrad Rzeszutek Wilk wrote:
> > > On Wed, Sep 21, 2011 at 01:37:50PM +0100, stefano.stabellini@eu.citrix.com wrote:
> > > > From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > 
> > > > The xen-platform-pci module is small and for PV on HVM guests is a
> > > 
> > > How small?
> > 
> > IIRC it is single digit numbers of kb.
> > 
> > >  Does it get removed from memory if it never gets loaded?
> > > 
> > > > requirement for xenbus.
> > > 
> > > Ok, should it then have a depency on XenBus as well?
> > 
> > xenbus can't be a module (which is why allowing platform-pci to be is
> > causing problems).
> > 
> > > Linus does not like the 'default y' very much. He actually dislikes
> > > it quite much as I found when he tore Dan's behind about cleancache.
> > 
> > In particular case the option is gated on a dependency on another Xen
> > option (PVHVM) which doesn't default on. But if you do select PVHVM you
> > certainly want this option, so I think that's ok (why else would
> > 'default y' even exist?)
> > 
> > > .. so I think making it 'default n' is a better option or perhaps
> > > making it depend on some other functionality? Or perhaps just remove
> > > the tristate/bool altogether so it gets activated if XEN_PVHVM
> > > is set?
> > > 
> > > Or remove the XEN_PLATFORM_PCI config option completly and make the
> > > config files that build this driver be CONFIG_XENPVHM dependent?
> > 
> > That would work too. Even better would be to make it an invisible
> > Kconfig symbol which PVHVM just selects.
> <nods>
> Or that since you can't really do PVHVM without the platform PCI driver.
> 

Considering that we all agree that XEN_PLATFORM_PCI is needed for PVHVM,
why should we keep around the old XEN_PLATFORM_PCI config option?
I think that Konrad's idea of just using CONFIG_XEN_PVHVM to build
xen-platform-pci.o is the best one.

Re: Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Jeremy Fitzhardinge]

On 09/21/2011 11:21 PM, Ian Campbell wrote:
> On Wed, 2011-09-21 at 23:49 +0100, Jeremy Fitzhardinge wrote:
>>> That would work too. Even better would be to make it an invisible
>>> Kconfig symbol which PVHVM just selects.
>> Eh, select is pretty nasty.
> Select of a non user visible symbol is perfectly fine. It's only when
> you select something a user can also set that things get nasty.

It doesn't matter if its user-visible.  If the selected symbol acquires
other dependencies, the selection won't set them.

    J

Re: Re: [PATCH] xen: change XEN_PLATFORM_PCI to bool default y

[Jan Beulich]

>>> On 22.09.11 at 21:35, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> On 09/21/2011 11:21 PM, Ian Campbell wrote:
>> On Wed, 2011-09-21 at 23:49 +0100, Jeremy Fitzhardinge wrote:
>>>> That would work too. Even better would be to make it an invisible
>>>> Kconfig symbol which PVHVM just selects.
>>> Eh, select is pretty nasty.
>> Select of a non user visible symbol is perfectly fine. It's only when
>> you select something a user can also set that things get nasty.
> 
> It doesn't matter if its user-visible.  If the selected symbol acquires
> other dependencies, the selection won't set them.

Despite there being numerous contrary examples in the tree: Either one
wants a select-only symbol (then putting dependencies on it is wrong) or
one wants an automatic symbol (then selecting it is very likely wrong).
Which all boils down to the bogus mixing of normal (forward) and reverse
dependencies (using the kconfig source/doc wording).

Jan