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

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

[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

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

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