Re: [Xen-devel] [PATCH] xen: remove CONFIG_XEN_DOM0 compile option

Re: [Xen-devel] [PATCH] xen: remove CONFIG_XEN_DOM0 compile option

[Andrew Jones]

----- Original Message -----
> On 01/10/2012 05:44 AM, Stefano Stabellini wrote:
> > On Mon, 9 Jan 2012, Andrew Jones wrote:
> >> I guess if we did the s/XEN_DOM0/LOCAL_APIC && IO_APIC && ACPI/ in
> >> arch/x86/pci/xen.c it would be pretty easy to review for
> >> equivalence.
> >> Then keep CONFIG_PRIVILIGED, but drop XEN_DOM0 from everywhere
> >> else and
> >> compile in the 3 files. I don't think it makes much sense to do it
> >> though. XEN_DOM0 keeps things tidier now and might be useful
> >> later.
> > we can keep things clean with the following:
> >
> > #ifdef CONFIG_LOCAL_APIC && CONFIG_IO_APIC && CONFIG_ACPI &&
> > CONFIG_PCI_XEN
> >
> > #define XEN_DOM0
> >
> > #endif
> >
> > in include/xen/xen.h.
> >
> > So in the source files we can still '#ifdef XEN_DOM0', but at the
> > same
> > time we can get rid of the build symbol: everybody wins.
> 
> No, really, I think this is silly.  This kind of dependency
> information
> should be encoded in the Kconfig system, and not reimplemented in an
> ad-hoc way.
> 
> If there were a clean way to do what Andrew wants then I'd support
> it,
> but this thread has descended into a spiral of madness, which makes
> me
> think its a lost cause.
> 
> If the root complaint is that "customers think that anything set in
> .config is a supported feature", then the solutions are to support
> all
> the features in .config, re-educate the customers that they're wrong,
> or
> maintain a local patch to do this stuff.

If only re-educating people was free, like preempting questions is.
Local patches are of course always an option, and perhaps in this
case it's the best one. However, I think we already made a case for
better xen configurability for the driver domains, so I'm not 100%
convinced my initial patch (making dom0 configurable) isn't worthy
of upstream. Also, I didn't see any comments on my v2[*] of that
patch, which I believe satisfies the menu complexity issue and
brings in more configurability. That said, I'm about to reply to
that patch myself, since there's an issue with it.

Drew

[*] http://article.gmane.org/gmane.linux.kernel.virtualization/14635

Re: [Xen-devel] [PATCH] xen: remove CONFIG_XEN_DOM0 compile option

[Konrad Rzeszutek Wilk]

> > If the root complaint is that "customers think that anything set in
> > .config is a supported feature", then the solutions are to support
> > all
> > the features in .config, re-educate the customers that they're wrong,
> > or
> > maintain a local patch to do this stuff.
> 
> If only re-educating people was free, like preempting questions is.
> Local patches are of course always an option, and perhaps in this
> case it's the best one. However, I think we already made a case for
> better xen configurability for the driver domains, so I'm not 100%

Could you repost those backend patches please? At this point I am not
sure which one we have discarded?

> convinced my initial patch (making dom0 configurable) isn't worthy
> of upstream. Also, I didn't see any comments on my v2[*] of that
> patch, which I believe satisfies the menu complexity issue and
> brings in more configurability. That said, I'm about to reply to
> that patch myself, since there's an issue with it.
> 
> Drew
> 
> [*] http://article.gmane.org/gmane.linux.kernel.virtualization/14635
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 1/4] xen kconfig: keep XEN_XENBUS_FRONTEND builtin

[Andrew Jones]

When XEN_XENBUS_FRONTEND gets selected as a module it can lead to
unbootable configs. If we need it, then we should just build it in.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 drivers/xen/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 8795480..1d24061 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -118,7 +118,7 @@ config XEN_SYS_HYPERVISOR
 	 but will have no xen contents.
 
 config XEN_XENBUS_FRONTEND
-	tristate
+	bool
 
 config XEN_GNTDEV
 	tristate "userspace grant access device driver"
-- 
1.7.7.5

[PATCH 2/4 v2] xen kconfig: relax INPUT_XEN_KBDDEV_FRONTEND deps

[Andrew Jones]

PV-on-HVM guests may want to use the xen keyboard/mouse frontend, but
they don't use the xen frame buffer frontend. For this case it doesn't
make much sense for INPUT_XEN_KBDDEV_FRONTEND to depend on
XEN_FBDEV_FRONTEND. The opposite direction always makes more sense, i.e.
if you're using xenfb, then you'll want xenkbd. Switch the dependencies.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 drivers/input/misc/Kconfig |    2 +-
 drivers/video/Kconfig      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 22d875f..36c15bf 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -533,7 +533,7 @@ config INPUT_CMA3000_I2C
 
 config INPUT_XEN_KBDDEV_FRONTEND
 	tristate "Xen virtual keyboard and mouse support"
-	depends on XEN_FBDEV_FRONTEND
+	depends on XEN
 	default y
 	select XEN_XENBUS_FRONTEND
 	help
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d83e967..3e38c2f 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2263,7 +2263,7 @@ config FB_VIRTUAL
 
 config XEN_FBDEV_FRONTEND
 	tristate "Xen virtual frame buffer support"
-	depends on FB && XEN
+	depends on FB && XEN && INPUT_XEN_KBDDEV_FRONTEND
 	select FB_SYS_FILLRECT
 	select FB_SYS_COPYAREA
 	select FB_SYS_IMAGEBLIT
-- 
1.7.7.5

[PATCH 3/4 v2] xen kconfig: add dom0 support help text

[Andrew Jones]

Describe dom0 support in the config menu and supply help text for it.

v2 adds 'if EXPERT' to keep it out of the "standard" menu.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arch/x86/xen/Kconfig |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 26c731a..31ec975 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -14,9 +14,14 @@ config XEN
 	  Xen hypervisor.
 
 config XEN_DOM0
-	def_bool y
+	bool "Xen Initial Domain (Dom0) support" if EXPERT
+	default y
 	depends on XEN && PCI_XEN && SWIOTLB_XEN
 	depends on X86_LOCAL_APIC && X86_IO_APIC && ACPI && PCI
+	help
+	  This allows the kernel to be used for the initial Xen domain,
+	  Domain0. This is a privileged guest that supplies backends
+	  and is used to manage the other Xen domains.
 
 # Dummy symbol since people have come to rely on the PRIVILEGED_GUEST
 # name in tools.
-- 
1.7.7.5

[PATCH 4/4] xen kconfig: describe xen tmem in the config menu

[Andrew Jones]

Add a description to the config menu for xen tmem.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 drivers/xen/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 1d24061..7e8d728 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -143,7 +143,7 @@ config SWIOTLB_XEN
 	select SWIOTLB
 
 config XEN_TMEM
-	bool
+	bool "Xen Transcendent Memory (tmem)"
 	default y if (CLEANCACHE || FRONTSWAP)
 	help
 	  Shim to interface in-kernel Transcendent Memory hooks
-- 
1.7.7.5

Re: [Xen-devel] [PATCH] xen: remove CONFIG_XEN_DOM0 compile option

[Konrad Rzeszutek Wilk]

On Wed, Jan 11, 2012 at 12:19:11PM -0400, Konrad Rzeszutek Wilk wrote:
> > > If the root complaint is that "customers think that anything set in
> > > .config is a supported feature", then the solutions are to support
> > > all
> > > the features in .config, re-educate the customers that they're wrong,
> > > or
> > > maintain a local patch to do this stuff.
> > 
> > If only re-educating people was free, like preempting questions is.
> > Local patches are of course always an option, and perhaps in this
> > case it's the best one. However, I think we already made a case for
> > better xen configurability for the driver domains, so I'm not 100%
> 
> Could you repost those backend patches please? At this point I am not
> sure which one we have discarded?

hm, I was thinking in terms of the XenBus ones. We had somewhere in this
converstion something about seperating the backend's from depending on
CONFIG_XEN_DOM0 as they can be run in any domain nowadays.

> 
> > convinced my initial patch (making dom0 configurable) isn't worthy
> > of upstream. Also, I didn't see any comments on my v2[*] of that
> > patch, which I believe satisfies the menu complexity issue and
> > brings in more configurability. That said, I'm about to reply to
> > that patch myself, since there's an issue with it.
> > 
> > Drew
> > 
> > [*] http://article.gmane.org/gmane.linux.kernel.virtualization/14635
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel

Re: [PATCH 1/4] xen kconfig: keep XEN_XENBUS_FRONTEND builtin

[Konrad Rzeszutek Wilk]

On Wed, Jan 11, 2012 at 05:36:38PM +0100, Andrew Jones wrote:
> When XEN_XENBUS_FRONTEND gets selected as a module it can lead to
> unbootable configs. If we need it, then we should just build it in.

Hm, don't the frontends by themsevles load this module? So if you
do 'modprobe xen-pcifront' it would load this automatically?

> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  drivers/xen/Kconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 8795480..1d24061 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -118,7 +118,7 @@ config XEN_SYS_HYPERVISOR
>  	 but will have no xen contents.
>  
>  config XEN_XENBUS_FRONTEND
> -	tristate
> +	bool
>  
>  config XEN_GNTDEV
>  	tristate "userspace grant access device driver"
> -- 
> 1.7.7.5

Re: [PATCH 2/4 v2] xen kconfig: relax INPUT_XEN_KBDDEV_FRONTEND deps

[Konrad Rzeszutek Wilk]

On Wed, Jan 11, 2012 at 05:36:39PM +0100, Andrew Jones wrote:
> PV-on-HVM guests may want to use the xen keyboard/mouse frontend, but
> they don't use the xen frame buffer frontend. For this case it doesn't
> make much sense for INPUT_XEN_KBDDEV_FRONTEND to depend on
> XEN_FBDEV_FRONTEND. The opposite direction always makes more sense, i.e.

What is the disadvantege of keeping it as is?

> if you're using xenfb, then you'll want xenkbd. Switch the dependencies.
> 
You are missing the CC to the proper maintainer.

Also, did you test this with PV and PVonHVM guests?

> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  drivers/input/misc/Kconfig |    2 +-
>  drivers/video/Kconfig      |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 22d875f..36c15bf 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -533,7 +533,7 @@ config INPUT_CMA3000_I2C
>  
>  config INPUT_XEN_KBDDEV_FRONTEND
>  	tristate "Xen virtual keyboard and mouse support"
> -	depends on XEN_FBDEV_FRONTEND
> +	depends on XEN
>  	default y
>  	select XEN_XENBUS_FRONTEND
>  	help
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index d83e967..3e38c2f 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2263,7 +2263,7 @@ config FB_VIRTUAL
>  
>  config XEN_FBDEV_FRONTEND
>  	tristate "Xen virtual frame buffer support"
> -	depends on FB && XEN
> +	depends on FB && XEN && INPUT_XEN_KBDDEV_FRONTEND
>  	select FB_SYS_FILLRECT
>  	select FB_SYS_COPYAREA
>  	select FB_SYS_IMAGEBLIT
> -- 
> 1.7.7.5

Re: [PATCH 4/4] xen kconfig: describe xen tmem in the config menu

[Konrad Rzeszutek Wilk]

On Wed, Jan 11, 2012 at 05:36:41PM +0100, Andrew Jones wrote:
> Add a description to the config menu for xen tmem.

We will have another config option asked during 'make oldconfig' right?

I thought part of this cleanup patch is to remove some of this
complexity - not make it _more_ complex.

> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  drivers/xen/Kconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 1d24061..7e8d728 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -143,7 +143,7 @@ config SWIOTLB_XEN
>  	select SWIOTLB
>  
>  config XEN_TMEM
> -	bool
> +	bool "Xen Transcendent Memory (tmem)"
>  	default y if (CLEANCACHE || FRONTSWAP)
>  	help
>  	  Shim to interface in-kernel Transcendent Memory hooks
> -- 
> 1.7.7.5

Re: [PATCH 4/4] xen kconfig: describe xen tmem in the config menu

[Andrew Jones]

----- Original Message -----
> On Wed, Jan 11, 2012 at 05:36:41PM +0100, Andrew Jones wrote:
> > Add a description to the config menu for xen tmem.
> 
> We will have another config option asked during 'make oldconfig'
> right?
> 
> I thought part of this cleanup patch is to remove some of this
> complexity - not make it _more_ complex.
> 

Another candidate for 'if EXPERT' then, IMHO.

> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  drivers/xen/Kconfig |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > index 1d24061..7e8d728 100644
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -143,7 +143,7 @@ config SWIOTLB_XEN
> >  	select SWIOTLB
> >  
> >  config XEN_TMEM
> > -	bool
> > +	bool "Xen Transcendent Memory (tmem)"
> >  	default y if (CLEANCACHE || FRONTSWAP)
> >  	help
> >  	  Shim to interface in-kernel Transcendent Memory hooks
> > --
> > 1.7.7.5
> 

Re: [PATCH 2/4 v2] xen kconfig: relax INPUT_XEN_KBDDEV_FRONTEND deps

[Andrew Jones]

----- Original Message -----
> On Wed, Jan 11, 2012 at 05:36:39PM +0100, Andrew Jones wrote:
> > PV-on-HVM guests may want to use the xen keyboard/mouse frontend,
> > but
> > they don't use the xen frame buffer frontend. For this case it
> > doesn't
> > make much sense for INPUT_XEN_KBDDEV_FRONTEND to depend on
> > XEN_FBDEV_FRONTEND. The opposite direction always makes more sense,
> > i.e.
> 
> What is the disadvantege of keeping it as is?

If you don't want FB, but you do want KBD, then you're stuck with FB
anyway, even though it isn't necessary. Perhaps I'm the only one who
ever considering building a config without FB?

> 
> > if you're using xenfb, then you'll want xenkbd. Switch the
> > dependencies.
> > 
> You are missing the CC to the proper maintainer.
> 

I added them the last time you reminded me. See the following
message-ids for the thread where this patch was discussed and then
led to the V2 here.

20120109075911.GA4049@core.coreip.homeip.net
725950ad-d5cf-4ddb-9870-a5c8d75cfa51@zmail13.collab.prod.int.phx2.redhat.com
1326131501-9610-1-git-send-email-drjones@redhat.com

I've re-added them to this thread, since the multiple
posting/reposting must have lost them again.

> Also, did you test this with PV and PVonHVM guests?

Testing-wise you don't really need to do much more then
'make oldconfig'. FB doesn't work on pv-on-hvm, the mod wouldn't
even init if you tried as there's a

        if (!xen_pv_domain())
                return -ENODEV;

I have tested a pv-on-hvm guest after this patch with FB off though,
which worked. For PV guests using FB, they'll work the same, as you
wouldn't want to change your config.

Drew

> 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  drivers/input/misc/Kconfig |    2 +-
> >  drivers/video/Kconfig      |    2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/input/misc/Kconfig
> > b/drivers/input/misc/Kconfig
> > index 22d875f..36c15bf 100644
> > --- a/drivers/input/misc/Kconfig
> > +++ b/drivers/input/misc/Kconfig
> > @@ -533,7 +533,7 @@ config INPUT_CMA3000_I2C
> >  
> >  config INPUT_XEN_KBDDEV_FRONTEND
> >  	tristate "Xen virtual keyboard and mouse support"
> > -	depends on XEN_FBDEV_FRONTEND
> > +	depends on XEN
> >  	default y
> >  	select XEN_XENBUS_FRONTEND
> >  	help
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index d83e967..3e38c2f 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -2263,7 +2263,7 @@ config FB_VIRTUAL
> >  
> >  config XEN_FBDEV_FRONTEND
> >  	tristate "Xen virtual frame buffer support"
> > -	depends on FB && XEN
> > +	depends on FB && XEN && INPUT_XEN_KBDDEV_FRONTEND
> >  	select FB_SYS_FILLRECT
> >  	select FB_SYS_COPYAREA
> >  	select FB_SYS_IMAGEBLIT
> > --
> > 1.7.7.5
> 

Re: [PATCH 4/4] xen kconfig: describe xen tmem in the config menu

[Konrad Rzeszutek Wilk]

On Fri, Jan 06, 2012 at 10:43:11AM +0100, Andrew Jones wrote:
> Add a description to the config menu for xen tmem.

I am not sure what this patch gets us. If this is to minimize the
size of the module - so say it gets loaded, but tmem-enabled is 
not set nor cleancache and we just have it consuming memory - we can do it
via returning -ENODEV on the module load.

Like this (completley untested nor compiled tested):

From 45b49b5d565c52e4396dae036ddb4f4094d914ec Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 23 Jan 2012 13:32:17 -0500
Subject: [PATCH] xen/tmem: Return -ENODEV if the backends are not registered.

.. otherwise the driver might still reside in the memory consuming
memory (that is the theory at least).

CC: Dan Magenheimer <dan.magenheimer@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/tmem.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/tmem.c b/drivers/xen/tmem.c
index d369965..e61b5a3 100644
--- a/drivers/xen/tmem.c
+++ b/drivers/xen/tmem.c
@@ -377,8 +377,10 @@ static struct frontswap_ops tmem_frontswap_ops = {
 
 static int __init xen_tmem_init(void)
 {
+	bool loaded = false;
+
 	if (!xen_domain())
-		return 0;
+		return -ENODEV;
 #ifdef CONFIG_FRONTSWAP
 	if (tmem_enabled && use_frontswap) {
 		char *s = "";
@@ -390,6 +392,7 @@ static int __init xen_tmem_init(void)
 			s = " (WARNING: frontswap_ops overridden)";
 		printk(KERN_INFO "frontswap enabled, RAM provided by "
 				 "Xen Transcendent Memory\n");
+		loaded = true;
 	}
 #endif
 #ifdef CONFIG_CLEANCACHE
@@ -402,9 +405,10 @@ static int __init xen_tmem_init(void)
 			s = " (WARNING: cleancache_ops overridden)";
 		printk(KERN_INFO "cleancache enabled, RAM provided by "
 				 "Xen Transcendent Memory%s\n", s);
+		loaded = true;
 	}
 #endif
-	return 0;
+	return loaded ? 0 : -ENODEV;
 }
 
 module_init(xen_tmem_init)
-- 
1.7.7.5


> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  drivers/xen/Kconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 1d24061..7e8d728 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -143,7 +143,7 @@ config SWIOTLB_XEN
>  	select SWIOTLB
>  
>  config XEN_TMEM
> -	bool
> +	bool "Xen Transcendent Memory (tmem)"
>  	default y if (CLEANCACHE || FRONTSWAP)
>  	help
>  	  Shim to interface in-kernel Transcendent Memory hooks
> -- 
> 1.7.7.5