Re: [PATCH] xen: remove unused Kconfig parameter

Re: [PATCH] xen: remove unused Kconfig parameter

[Konrad Rzeszutek Wilk]

On Mon, Jul 08, 2013 at 08:28:24AM +0200, Michael Opdenacker wrote:
> This patch proposes to remove the XEN_PRIVILEGED_GUEST kernel
> configuration parameter defined in arch/x86/xen/Kconfig, but used
> nowhere in the makefiles and source code.
> 
> This dummy parameter was added 3 years back, and it may
> make sense to remove it now, as the reasons to use it were not
> very clear.
> 
> Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com>

Can't remove it. It is used by grub2.

> ---
>  arch/x86/xen/Kconfig | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
> index 1a3c765..b6401dc 100644
> --- a/arch/x86/xen/Kconfig
> +++ b/arch/x86/xen/Kconfig
> @@ -19,11 +19,6 @@ config XEN_DOM0
>  	depends on XEN && PCI_XEN && SWIOTLB_XEN
>  	depends on X86_LOCAL_APIC && X86_IO_APIC && ACPI && PCI
>  
> -# Dummy symbol since people have come to rely on the PRIVILEGED_GUEST
> -# name in tools.
> -config XEN_PRIVILEGED_GUEST
> -	def_bool XEN_DOM0
> -
>  config XEN_PVHVM
>  	def_bool y
>  	depends on XEN && PCI && X86_LOCAL_APIC
> -- 
> 1.8.1.2
> 

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Matt Wilson]

On Mon, Jul 08, 2013 at 03:28:54PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Jul 08, 2013 at 08:28:24AM +0200, Michael Opdenacker wrote:
> > This patch proposes to remove the XEN_PRIVILEGED_GUEST kernel
> > configuration parameter defined in arch/x86/xen/Kconfig, but used
> > nowhere in the makefiles and source code.
> > 
> > This dummy parameter was added 3 years back, and it may
> > make sense to remove it now, as the reasons to use it were not
> > very clear.
> > 
> > Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com>
> 
> Can't remove it. It is used by grub2.

For reference, here's a related Debian bug report:
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=633127

Summary: grub2 scripts parse /boot/config-$(uname -r)

--msw

> > ---
> >  arch/x86/xen/Kconfig | 5 -----
> >  1 file changed, 5 deletions(-)
> > 
> > diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
> > index 1a3c765..b6401dc 100644
> > --- a/arch/x86/xen/Kconfig
> > +++ b/arch/x86/xen/Kconfig
> > @@ -19,11 +19,6 @@ config XEN_DOM0
> >  	depends on XEN && PCI_XEN && SWIOTLB_XEN
> >  	depends on X86_LOCAL_APIC && X86_IO_APIC && ACPI && PCI
> >  
> > -# Dummy symbol since people have come to rely on the PRIVILEGED_GUEST
> > -# name in tools.
> > -config XEN_PRIVILEGED_GUEST
> > -	def_bool XEN_DOM0
> > -
> >  config XEN_PVHVM
> >  	def_bool y
> >  	depends on XEN && PCI && X86_LOCAL_APIC
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] xen: remove unused Kconfig parameter

[H. Peter Anvin]

On 07/08/2013 12:28 PM, Konrad Rzeszutek Wilk wrote:
> On Mon, Jul 08, 2013 at 08:28:24AM +0200, Michael Opdenacker wrote:
>> This patch proposes to remove the XEN_PRIVILEGED_GUEST kernel
>> configuration parameter defined in arch/x86/xen/Kconfig, but used
>> nowhere in the makefiles and source code.
>>
>> This dummy parameter was added 3 years back, and it may
>> make sense to remove it now, as the reasons to use it were not
>> very clear.
>>
>> Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com>
> 
> Can't remove it. It is used by grub2.
> 

What the fsck does grub2 have to do with Kconfig?

	-hpa

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[H. Peter Anvin]

On 07/08/2013 12:34 PM, Matt Wilson wrote:
> 
> For reference, here's a related Debian bug report:
>   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=633127
> 
> Summary: grub2 scripts parse /boot/config-$(uname -r)
> 

What. The. Fuck.

	-hpa

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Borislav Petkov]

On Mon, Jul 08, 2013 at 01:29:40PM -0700, H. Peter Anvin wrote:
> What. The. Fuck.

This is just marvellous: grub2 has a bunch of scripts in /etc/grub.d
which rely on the presence of kernel config files in /boot or / and
greps them in order to do the menu entries based on the built-in
features it finds in them.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Paul Bolle]

On Mon, 2013-07-08 at 22:58 +0200, Borislav Petkov wrote:
> On Mon, Jul 08, 2013 at 01:29:40PM -0700, H. Peter Anvin wrote:
> > What. The. Fuck.
> 
> This is just marvellous: grub2 has a bunch of scripts in /etc/grub.d
> which rely on the presence of kernel config files in /boot or / and
> greps them in order to do the menu entries based on the built-in
> features it finds in them.

0) I've raised this issue a few months ago, but not on the LKML (see
http://thread.gmane.org/gmane.linux.kernel.virtualization/19126 ).

1) And I also asked whether "userspace [can] require the build system to
keep using some Kconfig symbol" (see
http://article.gmane.org/gmane.linux.kernel.virtualization/19129 ).
Peter and you clearly think userspace can't.

2) But anyhow, unless that grub2 configuration file has changed, this
Kconfig symbol can still be dropped, because grub2's check for it is
actually superfluous.


Paul Bolle

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Konrad Rzeszutek Wilk]

Paul Bolle <pebolle@tiscali.nl> wrote:
>On Mon, 2013-07-08 at 22:58 +0200, Borislav Petkov wrote:
>> On Mon, Jul 08, 2013 at 01:29:40PM -0700, H. Peter Anvin wrote:
>> > What. The. Fuck.
>> 
>> This is just marvellous: grub2 has a bunch of scripts in /etc/grub.d
>> which rely on the presence of kernel config files in /boot or / and
>> greps them in order to do the menu entries based on the built-in
>> features it finds in them.
>
>0) I've raised this issue a few months ago, but not on the LKML (see
>http://thread.gmane.org/gmane.linux.kernel.virtualization/19126 ).
>
>1) And I also asked whether "userspace [can] require the build system
>to
>keep using some Kconfig symbol" (see
>http://article.gmane.org/gmane.linux.kernel.virtualization/19129 ).
>Peter and you clearly think userspace can't.
>
>2) But anyhow, unless that grub2 configuration file has changed, this
>Kconfig symbol can still be dropped, because grub2's check for it is
>actually superfluous.
>
>
>Paul Bolle

Not so fast please. Linus absolutely abhors breaking user space and I am not comfortable with that idea either. I am not sure if that falls in that category but I am sure we can rope him after rc0 madness has stopped. 

Could you explain to me please why the check in the scripts is superfluous?

Especially as the grand plan is to get rid of CONFIG_XEN_DOM0 and more or less have a backend and fronted config option (since that makes more sense nowadays). And that would make the XEN_DOM0 be obsolete and the XEN_PRIV would be the one that turns a lot of the options needed to compile a kernel that can provide backend driver support. (I am hand waving here). 

I recall (and thank you for pointing to the link) that this raised some questions that never got answered such as are there tools that check /proc/config.gz for example for features? Otherwise should that be eliminated as well? 

-- 
Sent from my Android phone. Please excuse my brevity.

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Jan Beulich]

>>> On 09.07.13 at 02:26, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> Could you explain to me please why the check in the scripts is superfluous?

This is not really the question - _any_ such check can only be
wrong. The boot loader has absolutely no business caring about
kernel internals, and the kernel shouldn't be limited by it in how it
renames/adds/deletes Kconfig options and anything else.

> Especially as the grand plan is to get rid of CONFIG_XEN_DOM0 and more or 
> less have a backend and fronted config option (since that makes more sense 
> nowadays). And that would make the XEN_DOM0 be obsolete and the XEN_PRIV 
> would be the one that turns a lot of the options needed to compile a kernel 
> that can provide backend driver support. (I am hand waving here). 

That's pretty odd a plan, considering that Dom0 is more than just
an environment to provide backends. In fact, Dom0 may not be
providing any backends at all, and instead just serve the
"controlling hardware" and/or "control domain" purpose that it
really is meant to be. But of course, if none of _that_ functionality
depends on this config option, then it indeed could go away.

Jan

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Konrad Rzeszutek Wilk]

On Tue, Jul 09, 2013 at 08:41:12AM +0100, Jan Beulich wrote:
> >>> On 09.07.13 at 02:26, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > Could you explain to me please why the check in the scripts is superfluous?
> 
> This is not really the question - _any_ such check can only be
> wrong. The boot loader has absolutely no business caring about
> kernel internals, and the kernel shouldn't be limited by it in how it
> renames/adds/deletes Kconfig options and anything else.

Then that should be discussed on grub2 to remove said check and modify
the code so that it can properly work without regression.

> 
> > Especially as the grand plan is to get rid of CONFIG_XEN_DOM0 and more or 
> > less have a backend and fronted config option (since that makes more sense 
> > nowadays). And that would make the XEN_DOM0 be obsolete and the XEN_PRIV 
> > would be the one that turns a lot of the options needed to compile a kernel 
> > that can provide backend driver support. (I am hand waving here). 
> 
> That's pretty odd a plan, considering that Dom0 is more than just
> an environment to provide backends. In fact, Dom0 may not be
> providing any backends at all, and instead just serve the
> "controlling hardware" and/or "control domain" purpose that it
> really is meant to be. But of course, if none of _that_ functionality
> depends on this config option, then it indeed could go away.

Right, if I follow that train of logic then there is the 'controlling
domain' and 'hardware backend domain' and then the rest - the guests.

The 'controlling domain' would be just the one that is priviliged to launch
guests, setup the proper XSM labels, etc. Nothing to do with hardware.
But everything to deal with the toolstack.

The 'hardware backend domain' on the other hand has drivers and it also
needs a mechanism to export said devices to the guests. Otherwise it is
a pretty poor 'backend domain' without any way to export the devices
to guests.

Dom0 has been both - but there is nothing wrong with seperating these
two labels. And therein I was thinking that the 'hardware backend domain'
should be the introduced. I am not good with names so the best option
seems CONFIG_XEN_PRIVILIGED, but that sounds to be like 'controlling domain'.

Any good ideas for names? CONFIG_XEN_HARDWARE_DOMAIN ? CONFIG_XEN_PRIVILIGED_DOMAIN?

The items that would depend on CONFIG_XEN_PRIVILIGED_DOMAIN would be
the gntdev.c, xenfs.c and evtchn.c ?

The CONFIG_XEN_HARDWARE_DOMAIN would be mostly everything else - and
also require support for ACPI, PCI, SMP - the low-level thing, and
then the backends.

Lastly the guests. They should be able to be compiled and run without
setting either one of those two options (thought if one wants to set
them that is fine).

And this should also compile on ARM :-)

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Jan Beulich]

>>> On 09.07.13 at 16:48, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> Dom0 has been both - but there is nothing wrong with seperating these
> two labels. And therein I was thinking that the 'hardware backend domain'
> should be the introduced. I am not good with names so the best option
> seems CONFIG_XEN_PRIVILIGED, but that sounds to be like 'controlling 
> domain'.
> 
> Any good ideas for names? CONFIG_XEN_HARDWARE_DOMAIN ? 
> CONFIG_XEN_PRIVILIGED_DOMAIN?

Following the naming in the hypervisor, CONFIG_XEN_HARDWARE_DOMAIN
and CONFIG_XEN_CONTROL_DOMAIN would seem reasonable to me.

Jan

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Borislav Petkov]

On Tue, Jul 09, 2013 at 10:48:40AM -0400, Konrad Rzeszutek Wilk wrote:
> Then that should be discussed on grub2 to remove said check and modify
> the code so that it can properly work without regression.

Actually, the kernel patch removing that symbol should be applied so
that grub2 breaks faster. One can't possibly rely on kernel internals
for anything, as it is insanely insane (yep, the tautology is on purpose
:-)).

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[H. Peter Anvin]

On 07/09/2013 08:05 AM, Borislav Petkov wrote:
> On Tue, Jul 09, 2013 at 10:48:40AM -0400, Konrad Rzeszutek Wilk wrote:
>> Then that should be discussed on grub2 to remove said check and modify
>> the code so that it can properly work without regression.
> 
> Actually, the kernel patch removing that symbol should be applied so
> that grub2 breaks faster. One can't possibly rely on kernel internals
> for anything, as it is insanely insane (yep, the tautology is on purpose
> :-)).
> 

Indeed, especially since it inherently assumes something is a kernel
compile-time option... we routinely change things between compile time
and runtime.

	-hpa

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Konrad Rzeszutek Wilk]

On Tue, Jul 09, 2013 at 05:05:54PM +0200, Borislav Petkov wrote:
> On Tue, Jul 09, 2013 at 10:48:40AM -0400, Konrad Rzeszutek Wilk wrote:
> > Then that should be discussed on grub2 to remove said check and modify
> > the code so that it can properly work without regression.
> 
> Actually, the kernel patch removing that symbol should be applied so
> that grub2 breaks faster. One can't possibly rely on kernel internals
> for anything, as it is insanely insane (yep, the tautology is on purpose
> :-)).

I am not sure why you are advocating that path.

My thinking is that what should be done to have some sense of history
is that the patch in GRUB to not rely on kernel internals should be done.
Then that git commit of that tree should be mentioned in this kernel patch.

This way one can nicely follow the trail of changes and see what the
interdependencies are (or rather that they have been removed).

I wouldn't want GRUB2 to have regressions and stop generating the proper
menu options. That smells of userspace regressions and I am not too
keen to have Linus point this out to me.

Once that is done we can follow up on this patch and perhaps also
nicely convience the initial author of this patch to look at removing the
CONFIG_XEN_DOM0 and replacing them with the two other CONFIG options
that Jan and me have been discussing.

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Borislav Petkov]

On Tue, Jul 09, 2013 at 01:19:09PM -0400, Konrad Rzeszutek Wilk wrote:
> My thinking is that what should be done to have some sense of history
> is that the patch in GRUB to not rely on kernel internals should be
> done. Then that git commit of that tree should be mentioned in this
> kernel patch.

That's absolutely backwards and you know it. The two things - the kernel
and grub2 - have a defined interface and it is the only way they should
interact. Everything else is a bug.

> I wouldn't want GRUB2 to have regressions and stop generating the
> proper menu options. That smells of userspace regressions and I am not
> too keen to have Linus point this out to me.

You keep repeating that...

Dear Konrad, we need to talk about what a userspace regression is. And
in that case, grepping through the kernel .config and suddenly not
finding a certain symbol is not. That's like objdump-ing vmlinux and
relying on something there.

The patch removing this symbol is not violating a defined interface to
userspace. So you can't claim we're breaking anything here.

> Once that is done we can follow up on this patch and perhaps also
> nicely convience the initial author of this patch to look at removing
> the CONFIG_XEN_DOM0 and replacing them with the two other CONFIG
> options that Jan and me have been discussing.

Yes, but please make sure there's a bug opened against grub2 which lets
them know that grepping kernel configs is doomed to break sooner or
later.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Konrad Rzeszutek Wilk]

On Tue, Jul 09, 2013 at 10:01:50PM +0200, Borislav Petkov wrote:
> On Tue, Jul 09, 2013 at 01:19:09PM -0400, Konrad Rzeszutek Wilk wrote:
> > My thinking is that what should be done to have some sense of history
> > is that the patch in GRUB to not rely on kernel internals should be
> > done. Then that git commit of that tree should be mentioned in this
> > kernel patch.
> 
> That's absolutely backwards and you know it. The two things - the kernel
> and grub2 - have a defined interface and it is the only way they should
> interact. Everything else is a bug.
> 
> > I wouldn't want GRUB2 to have regressions and stop generating the
> > proper menu options. That smells of userspace regressions and I am not
> > too keen to have Linus point this out to me.
> 
> You keep repeating that...
> 
> Dear Konrad, we need to talk about what a userspace regression is. And
> in that case, grepping through the kernel .config and suddenly not
> finding a certain symbol is not. That's like objdump-ing vmlinux and
> relying on something there.
> 
> The patch removing this symbol is not violating a defined interface to
> userspace. So you can't claim we're breaking anything here.

I am not clear what the boundary is. Let me get Linus's guidance on this
after the rc0 madness.
> 
> > Once that is done we can follow up on this patch and perhaps also
> > nicely convience the initial author of this patch to look at removing
> > the CONFIG_XEN_DOM0 and replacing them with the two other CONFIG
> > options that Jan and me have been discussing.
> 
> Yes, but please make sure there's a bug opened against grub2 which lets
> them know that grepping kernel configs is doomed to break sooner or
> later.

I presume that grub2 folks are as strained for resources as everybody else
- meaning they won't get to doing a patch until somebody screams.

If there is a bug I try to fix it. In this case it looks like the bug is grub2
and in the kernel. Hence the fix should be in both places. If it can be done
in parallel that would be great. Since we have some time to actually fix in
in both places why not do it? 

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Borislav Petkov]

On Tue, Jul 09, 2013 at 04:40:11PM -0400, Konrad Rzeszutek Wilk wrote:
> I am not clear what the boundary is. Let me get Linus's guidance on this
> after the rc0 madness.

You're joking, right?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Sander Eikelenboom]

Tuesday, July 9, 2013, 5:05:54 PM, you wrote:

> On Tue, Jul 09, 2013 at 10:48:40AM -0400, Konrad Rzeszutek Wilk wrote:
>> Then that should be discussed on grub2 to remove said check and modify
>> the code so that it can properly work without regression.

> Actually, the kernel patch removing that symbol should be applied so
> that grub2 breaks faster. One can't possibly rely on kernel internals
> for anything, as it is insanely insane (yep, the tautology is on purpose
> :-)).


How insanely insane is it to be able to determine whether a certain compiled kernel binary supports a certain function ?

Grub does this in it's update script to prevent adding a xen + kernel combination that has no chance of booting when dom0 support has not been configured in the kernel.
That doesn't seem to be a unreasonable thought.

Grepping the accompanied config file in /boot for the xen dom0 Kconfig parameter seems the best possible effort grub can do at the moment.
Especially since the Kconfig parameter naming doesn't change that often.

If you know a better way for grub to determine if a certain function for a kernel binary is supported then please elaborate ..

 --
 Sander

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Borislav Petkov]

On Wed, Jul 10, 2013 at 12:34:58AM +0200, Sander Eikelenboom wrote:
> Grub does this in it's update script to prevent adding a xen + kernel
> combination that has no chance of booting when dom0 support has not
> been configured in the kernel. That doesn't seem to be a unreasonable
> thought.

Actually, I don't see the problem - if there's no such support, then the
boot fails - plain and simple. That's like not building in or adding
into the initrd, support for your root filesystem - it is your own damn
fault. It is not the bootloader's job to sanity check whether your
kernel boots or not.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[H. Peter Anvin]

On 07/09/2013 03:34 PM, Sander Eikelenboom wrote:
> 
> Grub does this in it's update script to prevent adding a xen + kernel combination that has no chance of booting when dom0 support has not been configured in the kernel.
> That doesn't seem to be a unreasonable thought.
> 

Except it does it backwards.  The test it uses is sufficient but not
necessary.

	-hpa

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Matt Wilson]

On Wed, Jul 10, 2013 at 12:34:58AM +0200, Sander Eikelenboom wrote:
> 
> Tuesday, July 9, 2013, 5:05:54 PM, you wrote:
> 
> > On Tue, Jul 09, 2013 at 10:48:40AM -0400, Konrad Rzeszutek Wilk wrote:
> >> Then that should be discussed on grub2 to remove said check and modify
> >> the code so that it can properly work without regression.
> 
> > Actually, the kernel patch removing that symbol should be applied so
> > that grub2 breaks faster. One can't possibly rely on kernel internals
> > for anything, as it is insanely insane (yep, the tautology is on purpose
> > :-)).
>
> How insanely insane is it to be able to determine whether a certain
> compiled kernel binary supports a certain function ?
> 
> Grub does this in it's update script to prevent adding a xen +
> kernel combination that has no chance of booting when dom0 support
> has not been configured in the kernel.  That doesn't seem to be a
> unreasonable thought.
> 
> Grepping the accompanied config file in /boot for the xen dom0
> Kconfig parameter seems the best possible effort grub can do at the
> moment.

I think this can be improved, even with the situation today.

> Especially since the Kconfig parameter naming doesn't change that
> often.
> 
> If you know a better way for grub to determine if a certain function
> for a kernel binary is supported then please elaborate ..

Certainly. Parse the ELF notes that are present in a dom0-capable
Linux kernel binary itself.

$ readelf -n vmlinux

Notes at offset 0x0069be88 with length 0x0000017c:
  Owner		Data size	Description
  Xen		0x00000006	Unknown note type: (0x00000006)
  Xen		0x00000004	Unknown note type: (0x00000007)
  Xen		0x00000008	Unknown note type: (0x00000005)
  Xen		0x00000008	Unknown note type: (0x00000003)
  Xen		0x00000008	NT_VERSION (version)
  Xen		0x00000008	NT_ARCH (architecture)
  Xen		0x0000002a	Unknown note type: (0x0000000a)
  Xen		0x00000004	Unknown note type: (0x00000009)
  Xen		0x00000008	Unknown note type: (0x00000008)
  Xen		0x00000010	Unknown note type: (0x0000000d)
  Xen		0x00000004	Unknown note type: (0x0000000e)
  Xen		0x00000008	Unknown note type: (0x0000000c)
  Xen		0x00000008	Unknown note type: (0x00000004)
  GNU		0x00000014	NT_GNU_BUILD_ID (unique build ID bitstring)

See arch/x86/xen/xen-head.S.

There's a new note type (XEN_ELFNOTE_SUPPORTED_FEATURES) that we can
use to make dom0 support explicit.

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/domain_build.c;hb=HEAD#l415

--msw

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Sander Eikelenboom]

Wednesday, July 10, 2013, 8:19:34 AM, you wrote:

> On Wed, Jul 10, 2013 at 12:34:58AM +0200, Sander Eikelenboom wrote:
>> 
>> Tuesday, July 9, 2013, 5:05:54 PM, you wrote:
>> 
>> > On Tue, Jul 09, 2013 at 10:48:40AM -0400, Konrad Rzeszutek Wilk wrote:
>> >> Then that should be discussed on grub2 to remove said check and modify
>> >> the code so that it can properly work without regression.
>> 
>> > Actually, the kernel patch removing that symbol should be applied so
>> > that grub2 breaks faster. One can't possibly rely on kernel internals
>> > for anything, as it is insanely insane (yep, the tautology is on purpose
>> > :-)).
>>
>> How insanely insane is it to be able to determine whether a certain
>> compiled kernel binary supports a certain function ?
>> 
>> Grub does this in it's update script to prevent adding a xen +
>> kernel combination that has no chance of booting when dom0 support
>> has not been configured in the kernel.  That doesn't seem to be a
>> unreasonable thought.
>> 
>> Grepping the accompanied config file in /boot for the xen dom0
>> Kconfig parameter seems the best possible effort grub can do at the
>> moment.

> I think this can be improved, even with the situation today.

>> Especially since the Kconfig parameter naming doesn't change that
>> often.
>> 
>> If you know a better way for grub to determine if a certain function
>> for a kernel binary is supported then please elaborate ..

> Certainly. Parse the ELF notes that are present in a dom0-capable
> Linux kernel binary itself.

> $ readelf -n vmlinux

> Notes at offset 0x0069be88 with length 0x0000017c:
>   Owner         Data size       Description
>   Xen           0x00000006      Unknown note type: (0x00000006)
>   Xen           0x00000004      Unknown note type: (0x00000007)
>   Xen           0x00000008      Unknown note type: (0x00000005)
>   Xen           0x00000008      Unknown note type: (0x00000003)
>   Xen           0x00000008      NT_VERSION (version)
>   Xen           0x00000008      NT_ARCH (architecture)
>   Xen           0x0000002a      Unknown note type: (0x0000000a)
>   Xen           0x00000004      Unknown note type: (0x00000009)
>   Xen           0x00000008      Unknown note type: (0x00000008)
>   Xen           0x00000010      Unknown note type: (0x0000000d)
>   Xen           0x00000004      Unknown note type: (0x0000000e)
>   Xen           0x00000008      Unknown note type: (0x0000000c)
>   Xen           0x00000008      Unknown note type: (0x00000004)
>   GNU           0x00000014      NT_GNU_BUILD_ID (unique build ID bitstring)

> See arch/x86/xen/xen-head.S.

> There's a new note type (XEN_ELFNOTE_SUPPORTED_FEATURES) that we can
> use to make dom0 support explicit.

> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/domain_build.c;hb=HEAD#l415

Seems like a better option, although completely dropping the check could be a option too.
Since dom0 support is in mainline distributions (at least Debian, haven't checked the other main yet) don't supply a seperate xen enabled kernel anymore,
so any distro supplied kernel has xen support. For the self-building case Borislav is probably right in that you have to watchout yourself.

So it would be nice to have at least some time to address this with upstream grub and the main distributions to patch their grub.

--
Sander


> --msw

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Konrad Rzeszutek Wilk]

> So it would be nice to have at least some time to address this with upstream grub and the main distributions to patch their grub.

<nods> Sounds quite sensible. Michael would you be OK doing this?

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Paul Bolle]

On Mon, 2013-07-08 at 20:26 -0400, Konrad Rzeszutek Wilk wrote:
> Could you explain to me please why the check in the scripts is
> superfluous?

The discussion has since moved on a bit, but I haven't answered this
question yet.

The check grub2 currently performs in one of its configuration scripts
is (reformatted):
    if (grep -qx "CONFIG_XEN_DOM0=y" "${config}" 2> /dev/null ||
        grep -qx "CONFIG_XEN_PRIVILEGED_GUEST=y" "${config}" 2> /dev/null);
    then echo -n "$i " ;
    fi

But the Kconfig entry for XEN_PRIVILEGED_GUEST reads: 
    # Dummy symbol since people have come to rely on the PRIVILEGED_GUEST
    # name in tools.
    config XEN_PRIVILEGED_GUEST
            def_bool XEN_DOM0

In other words: CONFIG_XEN_PRIVILEGED_GUEST should always be equal to
CONFIG_XEN_DOM0. So the two grep commands should always both evaluate to
true or both evaluate to false. One of these two commands can safely be
dropped.

Another consequence is that dropping XEN_PRIVILEGED_GUEST doesn't break
this configuration script. It will still behave as it does now.

(Whether that script should grep for Kconfig macros in the first place
is another discussion.)


Paul Bolle

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[H. Peter Anvin]

On 07/11/2013 03:08 AM, Paul Bolle wrote:
> On Mon, 2013-07-08 at 20:26 -0400, Konrad Rzeszutek Wilk wrote:
>> Could you explain to me please why the check in the scripts is
>> superfluous?
> 
> The discussion has since moved on a bit, but I haven't answered this
> question yet.
> 
> The check grub2 currently performs in one of its configuration scripts
> is (reformatted):
>     if (grep -qx "CONFIG_XEN_DOM0=y" "${config}" 2> /dev/null ||
>         grep -qx "CONFIG_XEN_PRIVILEGED_GUEST=y" "${config}" 2> /dev/null);
>     then echo -n "$i " ;
>     fi
> 

If only grep supported looking for more than one string at a time.
Maybe we could construct some kind of pattern expression syntax for it,
perhaps based on the theory of regular languages?

> But the Kconfig entry for XEN_PRIVILEGED_GUEST reads: 
>     # Dummy symbol since people have come to rely on the PRIVILEGED_GUEST
>     # name in tools.
>     config XEN_PRIVILEGED_GUEST
>             def_bool XEN_DOM0
> 
> In other words: CONFIG_XEN_PRIVILEGED_GUEST should always be equal to
> CONFIG_XEN_DOM0. So the two grep commands should always both evaluate to
> true or both evaluate to false. One of these two commands can safely be
> dropped.

Not necessarily true across kernel versions.

> Another consequence is that dropping XEN_PRIVILEGED_GUEST doesn't break
> this configuration script. It will still behave as it does now.
> 
> (Whether that script should grep for Kconfig macros in the first place
> is another discussion.)

"Hell no".

	-hpa

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Paul Bolle]

On Thu, 2013-07-11 at 10:57 -0700, H. Peter Anvin wrote:
> On 07/11/2013 03:08 AM, Paul Bolle wrote:
> > But the Kconfig entry for XEN_PRIVILEGED_GUEST reads: 
> >     # Dummy symbol since people have come to rely on the PRIVILEGED_GUEST
> >     # name in tools.
> >     config XEN_PRIVILEGED_GUEST
> >             def_bool XEN_DOM0
> > 
> > In other words: CONFIG_XEN_PRIVILEGED_GUEST should always be equal to
> > CONFIG_XEN_DOM0. So the two grep commands should always both evaluate to
> > true or both evaluate to false. One of these two commands can safely be
> > dropped.
> 
> Not necessarily true across kernel versions.

Correct. But it has actually been true ever since this Kconfig entry was
introduced in v2.6.37 (commit 6b0661a5e6fbfb159b78a39c0476905aa9b575fe,
"xen: introduce XEN_DOM0 as a silent option").

So people need not worry about breaking grub2 by dropping
XEN_PRIVILEGED_GUEST.


Paul Bolle

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Konrad Rzeszutek Wilk]

On Thu, Jul 11, 2013 at 08:13:58PM +0200, Paul Bolle wrote:
> On Thu, 2013-07-11 at 10:57 -0700, H. Peter Anvin wrote:
> > On 07/11/2013 03:08 AM, Paul Bolle wrote:
> > > But the Kconfig entry for XEN_PRIVILEGED_GUEST reads: 
> > >     # Dummy symbol since people have come to rely on the PRIVILEGED_GUEST
> > >     # name in tools.
> > >     config XEN_PRIVILEGED_GUEST
> > >             def_bool XEN_DOM0
> > > 
> > > In other words: CONFIG_XEN_PRIVILEGED_GUEST should always be equal to
> > > CONFIG_XEN_DOM0. So the two grep commands should always both evaluate to
> > > true or both evaluate to false. One of these two commands can safely be
> > > dropped.
> > 
> > Not necessarily true across kernel versions.
> 
> Correct. But it has actually been true ever since this Kconfig entry was
> introduced in v2.6.37 (commit 6b0661a5e6fbfb159b78a39c0476905aa9b575fe,
> "xen: introduce XEN_DOM0 as a silent option").
> 
> So people need not worry about breaking grub2 by dropping
> XEN_PRIVILEGED_GUEST.

Right, but when we drop the CONFIG_XEN_DOM0 as well (and have instead a
CONFIG_XEN_HARDWARE_DOMAIN_SOMETHING_LIKE_THAT_I_FORGOT_NOW_THE_NAME),
then this will be a problem. Sander's proposal on fixing it "right" in
grub2 using whatever is the proper way (whatever that is) is the right
thing to do first.

Then we can make the Kconfig entries be more in line with the different
divisions of guest types - instead of the simplified
'dump-it-all-in-dom0'. This means more surgery in the Kconfig than the
initial patch posted here. This assuming that both Peter's and Boris's
assertion that CONFIG_* entries do not fall in the "must not break
user-space" category.

Re: [Xen-devel] [PATCH] xen: remove unused Kconfig parameter

[Geert Uytterhoeven]

On Thu, Jul 11, 2013 at 7:57 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> The check grub2 currently performs in one of its configuration scripts
>> is (reformatted):
>>     if (grep -qx "CONFIG_XEN_DOM0=y" "${config}" 2> /dev/null ||
>>         grep -qx "CONFIG_XEN_PRIVILEGED_GUEST=y" "${config}" 2> /dev/null);
>>     then echo -n "$i " ;
>>     fi
>>
>
> If only grep supported looking for more than one string at a time.
> Maybe we could construct some kind of pattern expression syntax for it,
> perhaps based on the theory of regular languages?

grep -E "(pat1|pat2|pat3|...)"?

Or am I missing something?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH] xen: remove unused Kconfig parameter

[Michael Opdenacker]

This patch proposes to remove the XEN_PRIVILEGED_GUEST kernel
configuration parameter defined in arch/x86/xen/Kconfig, but used
nowhere in the makefiles and source code.

This dummy parameter was added 3 years back, and it may
make sense to remove it now, as the reasons to use it were not
very clear.

Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com>
---
 arch/x86/xen/Kconfig | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 1a3c765..b6401dc 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -19,11 +19,6 @@ config XEN_DOM0
 	depends on XEN && PCI_XEN && SWIOTLB_XEN
 	depends on X86_LOCAL_APIC && X86_IO_APIC && ACPI && PCI
 
-# Dummy symbol since people have come to rely on the PRIVILEGED_GUEST
-# name in tools.
-config XEN_PRIVILEGED_GUEST
-	def_bool XEN_DOM0
-
 config XEN_PVHVM
 	def_bool y
 	depends on XEN && PCI && X86_LOCAL_APIC
-- 
1.8.1.2