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

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

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

[Igor Mammedov]

On 01/23/2012 07:34 PM, Konrad Rzeszutek Wilk wrote:
> 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.

But why compile in something that one may never use? At least with this patch
I'll have a choice to turn it off if I don't need it.
For example when I build hardened kernel, I'd like to turn of all unnecessary
features for a particular config (i.e. reduce attack surface as much as possible).

-- 
Thanks,
  Igor

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

[Konrad Rzeszutek Wilk]

On Tue, Jan 24, 2012 at 09:30:15AM +0100, Igor Mammedov wrote:
> On 01/23/2012 07:34 PM, Konrad Rzeszutek Wilk wrote:
> >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.
> 
> But why compile in something that one may never use? At least with this patch
> I'll have a choice to turn it off if I don't need it.

Then this patch is misleading. It should state at the start
what its purpose is. It sounds like adding the description is just
a way for the real purpose of this patch - which is to disable tmem.

> For example when I build hardened kernel, I'd like to turn of all unnecessary
> features for a particular config (i.e. reduce attack surface as much as possible).

The 'tmem' gets turned off if you disable cleancache. Can't you just
disable cleancache in your hardened config?

Re: [Xen-devel] [PATCH 2/4] xen kconfig: relax INPUT_XEN_KBDDEV_FRONTEND deps

[Konrad Rzeszutek Wilk]

On Fri, Jan 06, 2012 at 10:43:09AM +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.
> if you're using xenfb, then you'll want xenkbd. Switch the dependencies.

You need to CC as well these people that have 'maintainer' field on them:

konrad@phenom:~/work/linux$ scripts/get_maintainer.pl -f
drivers/video/Kconfig
Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
(maintainer:FRAMEBUFFER LAYER)
linux-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
linux-kernel@vger.kernel.org (open list)
konrad@phenom:~/work/linux$ scripts/get_maintainer.pl -f
drivers/input/misc/Kconfig
Dmitry Torokhov <dmitry.torokhov@gmail.com> (maintainer:INPUT
(KEYBOARD,...,commit_signer:9/16=56%)
Samuel Ortiz <sameo@linux.intel.com> (commit_signer:3/16=19%)
Anirudh Ghayal <aghayal@codeaurora.org> (commit_signer:2/16=12%)
Peter Ujfalusi <peter.ujfalusi@ti.com> (commit_signer:2/16=12%)
Alan Cox <alan@linux.intel.com> (commit_signer:2/16=12%)
linux-input@vger.kernel.org (open list:INPUT (KEYBOARD,...)
linux-kernel@vger.kernel.org (open list)

> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  drivers/input/misc/Kconfig |    2 +-
>  drivers/video/Kconfig      |    1 +
>  2 files changed, 2 insertions(+), 1 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..269b299 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2269,6 +2269,7 @@ config XEN_FBDEV_FRONTEND
>  	select FB_SYS_IMAGEBLIT
>  	select FB_SYS_FOPS
>  	select FB_DEFERRED_IO
> +	select INPUT_XEN_KBDDEV_FRONTEND
>  	select XEN_XENBUS_FRONTEND
>  	default y
>  	help
> -- 
> 1.7.7.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [Xen-devel] [PATCH 2/4] xen kconfig: relax INPUT_XEN_KBDDEV_FRONTEND deps

[Dmitry Torokhov]

Hi Andrew,

On Fri, Jan 06, 2012 at 10:58:06AM -0500, Andrew Jones wrote:
> 
> 
> ----- Original Message -----
> > On Fri, Jan 06, 2012 at 10:43:09AM +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.
> > > if you're using xenfb, then you'll want xenkbd. Switch the
> > > dependencies.
> > 
> > You need to CC as well these people that have 'maintainer' field on
> > them:
> > 
> > konrad@phenom:~/work/linux$ scripts/get_maintainer.pl -f
> > drivers/video/Kconfig
> > Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> > (maintainer:FRAMEBUFFER LAYER)
> > linux-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
> > linux-kernel@vger.kernel.org (open list)
> > konrad@phenom:~/work/linux$ scripts/get_maintainer.pl -f
> > drivers/input/misc/Kconfig
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> (maintainer:INPUT
> > (KEYBOARD,...,commit_signer:9/16=56%)
> > Samuel Ortiz <sameo@linux.intel.com> (commit_signer:3/16=19%)
> > Anirudh Ghayal <aghayal@codeaurora.org> (commit_signer:2/16=12%)
> > Peter Ujfalusi <peter.ujfalusi@ti.com> (commit_signer:2/16=12%)
> > Alan Cox <alan@linux.intel.com> (commit_signer:2/16=12%)
> > linux-input@vger.kernel.org (open list:INPUT (KEYBOARD,...)
> > linux-kernel@vger.kernel.org (open list)
> > 
> 
> Thanks. Replied with them in CC.
> 
> Drew
> 
> > > 
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > ---
> > >  drivers/input/misc/Kconfig |    2 +-
> > >  drivers/video/Kconfig      |    1 +
> > >  2 files changed, 2 insertions(+), 1 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

This is OK with me.

> > >  	default y
> > >  	select XEN_XENBUS_FRONTEND
> > >  	help
> > > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > > index d83e967..269b299 100644
> > > --- a/drivers/video/Kconfig
> > > +++ b/drivers/video/Kconfig
> > > @@ -2269,6 +2269,7 @@ config XEN_FBDEV_FRONTEND
> > >  	select FB_SYS_IMAGEBLIT
> > >  	select FB_SYS_FOPS
> > >  	select FB_DEFERRED_IO
> > > +	select INPUT_XEN_KBDDEV_FRONTEND

But here you need to either depend on or select INPUT as select does not
resolve dependencies for selected symbol.

Thanks.

-- 
Dmitry

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

[Konrad Rzeszutek Wilk]

On Fri, Jan 06, 2012 at 10:43:09AM +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.
> if you're using xenfb, then you'll want xenkbd. Switch the dependencies.

This looks OK. Let me test this out to see if there is any silly
fallout.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  drivers/input/misc/Kconfig |    2 +-
>  drivers/video/Kconfig      |    1 +
>  2 files changed, 2 insertions(+), 1 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..269b299 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2269,6 +2269,7 @@ config XEN_FBDEV_FRONTEND
>  	select FB_SYS_IMAGEBLIT
>  	select FB_SYS_FOPS
>  	select FB_DEFERRED_IO
> +	select INPUT_XEN_KBDDEV_FRONTEND
>  	select XEN_XENBUS_FRONTEND
>  	default y
>  	help
> -- 
> 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

Re: [Xen-devel] [PATCH 3/4 v2] xen kconfig: add dom0 support help text

[Andrew Jones]

----- Original Message -----
> 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

There's currently an issue using EXPERT. It doesn't make this
patch wrong, but any users that would want to turn EXPERT on, in
order to turn DOM0 off, would find that their configs have gotten
twisted in other ways as well. I've posted a patch for EXPERT, and
if it gets merged, then I really can't see any good arguments against
this patch.

Drew

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

[Konrad Rzeszutek Wilk]

On Fri, Jan 06, 2012 at 10:43:10AM +0100, Andrew Jones wrote:
> Describe dom0 support in the config menu and supply help text for it.

So I had this reply in my draft and forgot to send it. Sorry about that.

My understanding from the converstion was that we try to "squash"
the XEN_DOM0 option so that it would not be present. But that did not
work as it lead to a string of X86_LOCAL_APIC, X86_IO_ACPI, ACPI, PCI,
and so on. Then the .h with #define XEN_something based on those
symbols, but that is not the job of a header file. It is the job
of Kconfig.

The other way was to squash this in with the backend support. Since
we are moving away from having one initial domain, to having
multitple "initial domains" (priviliged domains) where each can
server as a backend.  However (quoting Jeremy) "it is more of
disaggregating the privilege to reduce the amount concentrated in any one
part.  Backends don't need any more privilege than to be able to
access the specific device(s) they're being the backend for."

Interestingly, that means DOM0 is kind of .. well, it should be
no different from a normal HVM guest. The old-style PV dom0 remains
would be ..well, almost nothing. The Xen MMU, and Xen SWIOTLB - those
are the ones that pop in mind for Dom0, but they are also used
for PV PCI. In fact, all (I think?) of the CONFIG_XEN_DOM0 functionality
can be _used_ in a PV guest. The 'if (initial_xen_domain()' should
probably be addressed first and to figure which one of those can be
altered as the "backend domain" can run both frontend and backend drivers
(oh joy!). So more relaxing those config options and/or "if (xen..)".

Anyhow, back to the HVM dom0 - that is in the future - and it is going
to take a couple of years to get it. I would rather not shoot my
foot by removing these CONFIG_* option until we get a better grasp of how
we want to deal with the PV hybrid.

What I am saying is that I don't know what the right answer is,
but I don't believe the patch below is it. I wish I had a better
answer in terms of "do this instead", but none of those worked.

Perhaps we can brainstorm some of this at XenSummit by which time
I hope Mukesh's PV hybrid work will be completed and a lot of the
work on the toolstack for backend drivers will be laid out.

> 
> 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..88862d5 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"
> +	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

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 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