2014-02-16 20:07:44

by Paul Bolle

[permalink] [raw]
Subject: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

This patch removes the Kconfig symbol XEN_PRIVILEGED_GUEST which is
used nowhere in the tree. We do know grub2 has a script that greps
kernel configuration files for this symbol. It shouldn't do that. As
Linus summarized:
This is a grub bug. It really is that simple. Treat it as one.

So there's no reason to not remove it, like we do with all unused
Kconfig symbols.

[[email protected]: rewrote commit explanation.]
Signed-off-by: Michael Opdenacker <[email protected]>
Signed-off-by: Paul Bolle <[email protected]>
---
Tested with "git grep".

Michael's version can be found at https://lkml.org/lkml/2013/7/8/34 .
(This is the same patch, with a rewritten explanation, and my S-o-b
line.) The question whether this symbol can be removed was further
discussed in https://lkml.org/lkml/2013/7/15/308 .

I don't think a bug was ever filed against grub2 regarding its way to
check for Xen support. Should that be done first?

arch/x86/xen/Kconfig | 5 -----
1 file changed, 5 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 01b9026..512219d 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.5.3


2014-02-17 12:23:53

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST


On Feb 16, 2014 3:07 PM, Paul Bolle <[email protected]> wrote:
>
> This patch removes the Kconfig symbol XEN_PRIVILEGED_GUEST which is
> used nowhere in the tree. We do know grub2 has a script that greps
> kernel configuration files for this symbol. It shouldn't do that. As

Please look in the grub git tree. They have fixed their code to not do this anymore. This should be reflected in the patch description.

Lastly please check which distro has this new grub version so that we know which distros won't be affected.

Thanks.

> Linus summarized:
>     This is a grub bug. It really is that simple. Treat it as one.
>
> So there's no reason to not remove it, like we do with all unused
> Kconfig symbols.
>
> [[email protected]: rewrote commit explanation.]
> Signed-off-by: Michael Opdenacker <[email protected]>
> Signed-off-by: Paul Bolle <[email protected]>
> ---
> Tested with "git grep".
>
> Michael's version can be found at https://lkml.org/lkml/2013/7/8/34 .
> (This is the same patch, with a rewritten explanation, and my S-o-b
> line.) The question whether this symbol can be removed was further
> discussed in https://lkml.org/lkml/2013/7/15/308 .
>
> I don't think a bug was ever filed against grub2 regarding its way to
> check for Xen support. Should that be done first?

Had been done the moment I got Linus reply but instead of a bug it was on the mailing list.
>
> arch/x86/xen/Kconfig | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
> index 01b9026..512219d 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.5.3
>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-02-17 13:03:21

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

On Mon, 2014-02-17 at 07:23 -0500, Konrad Rzeszutek Wilk wrote:
> On Feb 16, 2014 3:07 PM, Paul Bolle <[email protected]> wrote:
> Please look in the grub git tree. They have fixed their code to not do
> this anymore. This should be reflected in the patch description.

Thanks, I didn't know that. That turned out to be grub commit
ec824e0f2a399ce2ab3a2e3353d372a236595059 ("Implement grub_file tool and
use it to implement generating of config"), see
http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=ec824e0f2a399ce2ab3a2e3353d372a236595059

> Lastly please check which distro has this new grub version so that we
> know which distros won't be affected.

No distro should be affected. See, the test that grub2 used to do was
(edited for clarity):
grep -qx "CONFIG_XEN_DOM0=y" "${config}" || grep -qx "CONFIG_XEN_PRIVILEGED_GUEST=y" "${config}"

But the Kconfig entry for XEN_PRIVILEGED_GUEST reads:
config XEN_PRIVILEGED_GUEST
def_bool XEN_DOM0

Ie, XEN_PRIVILEGED_GUEST is equal to XEN_DOM0 by definition, so the
second part of that test is superfluous. (We discussed this last year.
If lkml.org weren't down I'd provide a link.) Or am I misreading this
Kconfig entry?

I hope to send a v2, with an updated commit explanation, in a few days.


Paul Bolle

2014-02-17 14:43:43

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

On Mon, Feb 17, 2014 at 02:03:17PM +0100, Paul Bolle wrote:
> On Mon, 2014-02-17 at 07:23 -0500, Konrad Rzeszutek Wilk wrote:
> > On Feb 16, 2014 3:07 PM, Paul Bolle <[email protected]> wrote:
> > Please look in the grub git tree. They have fixed their code to not do
> > this anymore. This should be reflected in the patch description.
>
> Thanks, I didn't know that. That turned out to be grub commit
> ec824e0f2a399ce2ab3a2e3353d372a236595059 ("Implement grub_file tool and
> use it to implement generating of config"), see
> http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=ec824e0f2a399ce2ab3a2e3353d372a236595059
>
> > Lastly please check which distro has this new grub version so that we
> > know which distros won't be affected.
>
> No distro should be affected. See, the test that grub2 used to do was
> (edited for clarity):
> grep -qx "CONFIG_XEN_DOM0=y" "${config}" || grep -qx "CONFIG_XEN_PRIVILEGED_GUEST=y" "${config}"
>
> But the Kconfig entry for XEN_PRIVILEGED_GUEST reads:
> config XEN_PRIVILEGED_GUEST
> def_bool XEN_DOM0
>
> Ie, XEN_PRIVILEGED_GUEST is equal to XEN_DOM0 by definition, so the
> second part of that test is superfluous. (We discussed this last year.
> If lkml.org weren't down I'd provide a link.) Or am I misreading this
> Kconfig entry?

Ah, forgot about the second test for 'XEN_DOM0'. Yes that should work.

Thanks!
>
> I hope to send a v2, with an updated commit explanation, in a few days.
>
>
> Paul Bolle
>

2014-02-18 10:14:32

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

On Mon, 2014-02-17 at 09:43 -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Feb 17, 2014 at 02:03:17PM +0100, Paul Bolle wrote:
> > On Mon, 2014-02-17 at 07:23 -0500, Konrad Rzeszutek Wilk wrote:
> > > On Feb 16, 2014 3:07 PM, Paul Bolle <[email protected]> wrote:
> > > Please look in the grub git tree. They have fixed their code to not do
> > > this anymore. This should be reflected in the patch description.
> >
> > Thanks, I didn't know that. That turned out to be grub commit
> > ec824e0f2a399ce2ab3a2e3353d372a236595059 ("Implement grub_file tool and
> > use it to implement generating of config"), see
> > http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=ec824e0f2a399ce2ab3a2e3353d372a236595059

And that commit was reverted a week later in grub commit
faf4a65e1e1ce1d822d251c1e4b53d96ec7faec5 ("Revert grub-file usage in
grub-mkconfig."), see
http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=faf4a65e1e1ce1d822d251c1e4b53d96ec7faec5 .

That commit has no explanation (other than its one line summary). So
we're left guessing why this was done. Luckily, it doesn't matter here,
because the test for CONFIG_XEN_PRIVILEGED_GUEST is superfluous.

Anyhow, I hope to submit a second version of this patch later this day.


Paul Bolle

2014-02-18 13:07:47

by Paul Bolle

[permalink] [raw]
Subject: [PATCH v2] xen: remove XEN_PRIVILEGED_GUEST

This patch removes the Kconfig symbol XEN_PRIVILEGED_GUEST which is
used nowhere in the tree.

We do know grub2 has a script that greps kernel configuration files for
its macro. It shouldn't do that. As Linus summarized:
This is a grub bug. It really is that simple. Treat it as one.

Besides, grub2's grepping for that macro is actually superfluous. See,
that script currently contains this test (simplified):
grep -x CONFIG_XEN_DOM0=y $config || grep -x CONFIG_XEN_PRIVILEGED_GUEST=y $config

But since XEN_DOM0 and XEN_PRIVILEGED_GUEST are by definition equal,
removing XEN_PRIVILEGED_GUEST cannot influence this test.

So there's no reason to not remove this symbol, like we do with all
unused Kconfig symbols.

[[email protected]: rewrote commit explanation.]
Signed-off-by: Michael Opdenacker <[email protected]>
Signed-off-by: Paul Bolle <[email protected]>
---
v2: added a few lines to the commit explanation to show grub2's test for
this symbol is superfluous anyway. Still only git "grep tested".

arch/x86/xen/Kconfig | 5 -----
1 file changed, 5 deletions(-)

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 01b9026..512219d 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.5.3

2014-02-18 13:14:01

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH v2] xen: remove XEN_PRIVILEGED_GUEST

Here I should have added:

From: Michael Opdenacker <[email protected]>

in order for Michael to show up as author of the patch.

On Tue, 2014-02-18 at 14:07 +0100, Paul Bolle wrote:
> This patch removes the Kconfig symbol XEN_PRIVILEGED_GUEST which is
> used nowhere in the tree.
>
> We do know grub2 has a script that greps kernel configuration files for
> its macro. It shouldn't do that. As Linus summarized:
> This is a grub bug. It really is that simple. Treat it as one.
>
> Besides, grub2's grepping for that macro is actually superfluous. See,
> that script currently contains this test (simplified):
> grep -x CONFIG_XEN_DOM0=y $config || grep -x CONFIG_XEN_PRIVILEGED_GUEST=y $config
>
> But since XEN_DOM0 and XEN_PRIVILEGED_GUEST are by definition equal,
> removing XEN_PRIVILEGED_GUEST cannot influence this test.
>
> So there's no reason to not remove this symbol, like we do with all
> unused Kconfig symbols.
>
> [[email protected]: rewrote commit explanation.]
> Signed-off-by: Michael Opdenacker <[email protected]>
> Signed-off-by: Paul Bolle <[email protected]>


Paul Bolle

2014-02-24 18:40:48

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCH] xen: remove XEN_PRIVILEGED_GUEST

On Tue, Feb 18, 2014 at 11:14:27AM +0100, Paul Bolle wrote:
> On Mon, 2014-02-17 at 09:43 -0500, Konrad Rzeszutek Wilk wrote:
> > On Mon, Feb 17, 2014 at 02:03:17PM +0100, Paul Bolle wrote:
> > > On Mon, 2014-02-17 at 07:23 -0500, Konrad Rzeszutek Wilk wrote:
> > > > On Feb 16, 2014 3:07 PM, Paul Bolle <[email protected]> wrote:
> > > > Please look in the grub git tree. They have fixed their code to not do
> > > > this anymore. This should be reflected in the patch description.
> > >
> > > Thanks, I didn't know that. That turned out to be grub commit
> > > ec824e0f2a399ce2ab3a2e3353d372a236595059 ("Implement grub_file tool and
> > > use it to implement generating of config"), see
> > > http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=ec824e0f2a399ce2ab3a2e3353d372a236595059
>
> And that commit was reverted a week later in grub commit
> faf4a65e1e1ce1d822d251c1e4b53d96ec7faec5 ("Revert grub-file usage in
> grub-mkconfig."), see
> http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=faf4a65e1e1ce1d822d251c1e4b53d96ec7faec5 .
>
> That commit has no explanation (other than its one line summary). So
> we're left guessing why this was done. Luckily, it doesn't matter here,
> because the test for CONFIG_XEN_PRIVILEGED_GUEST is superfluous.

How about we ask Vladimir?

Vladimir - could you shed some light on it? Thanks!

>
> Anyhow, I hope to submit a second version of this patch later this day.
>
>
> Paul Bolle
>
>
> _______________________________________________
> Xen-devel mailing list
> [email protected]
> http://lists.xen.org/xen-devel

2014-02-24 18:51:39

by David Vrabel

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCH v2] xen: remove XEN_PRIVILEGED_GUEST

On 18/02/14 13:07, Paul Bolle wrote:
> This patch removes the Kconfig symbol XEN_PRIVILEGED_GUEST which is
> used nowhere in the tree.
>
> We do know grub2 has a script that greps kernel configuration files for
> its macro. It shouldn't do that. As Linus summarized:
> This is a grub bug. It really is that simple. Treat it as one.
>
> Besides, grub2's grepping for that macro is actually superfluous. See,
> that script currently contains this test (simplified):
> grep -x CONFIG_XEN_DOM0=y $config || grep -x CONFIG_XEN_PRIVILEGED_GUEST=y $config
>
> But since XEN_DOM0 and XEN_PRIVILEGED_GUEST are by definition equal,
> removing XEN_PRIVILEGED_GUEST cannot influence this test.
>
> So there's no reason to not remove this symbol, like we do with all
> unused Kconfig symbols.

Reviewed-by: David Vrabel <[email protected]>

David

Subject: Re: [Xen-devel] [PATCH] xen: remove XEN_PRIVILEGED_GUEST

On 24.02.2014 19:39, Konrad Rzeszutek Wilk wrote:
> On Tue, Feb 18, 2014 at 11:14:27AM +0100, Paul Bolle wrote:
>> On Mon, 2014-02-17 at 09:43 -0500, Konrad Rzeszutek Wilk wrote:
>>> On Mon, Feb 17, 2014 at 02:03:17PM +0100, Paul Bolle wrote:
>>>> On Mon, 2014-02-17 at 07:23 -0500, Konrad Rzeszutek Wilk wrote:
>>>>> On Feb 16, 2014 3:07 PM, Paul Bolle <[email protected]> wrote:
>>>>> Please look in the grub git tree. They have fixed their code to not do
>>>>> this anymore. This should be reflected in the patch description.
>>>>
>>>> Thanks, I didn't know that. That turned out to be grub commit
>>>> ec824e0f2a399ce2ab3a2e3353d372a236595059 ("Implement grub_file tool and
>>>> use it to implement generating of config"), see
>>>> http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=ec824e0f2a399ce2ab3a2e3353d372a236595059
>>
>> And that commit was reverted a week later in grub commit
>> faf4a65e1e1ce1d822d251c1e4b53d96ec7faec5 ("Revert grub-file usage in
>> grub-mkconfig."), see
>> http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=faf4a65e1e1ce1d822d251c1e4b53d96ec7faec5 .
>>
>> That commit has no explanation (other than its one line summary). So
>> we're left guessing why this was done. Luckily, it doesn't matter here,
>> because the test for CONFIG_XEN_PRIVILEGED_GUEST is superfluous.
>
> How about we ask Vladimir?
>
> Vladimir - could you shed some light on it? Thanks!
>
CONFIG_XEN_PRIVILEGED_GUEST is not present on Linux even though it
should be. The test was removed to accomodate this.
The usage of grub-file was removed because it wasn't release-ready.
>>
>> Anyhow, I hope to submit a second version of this patch later this day.
>>
>>
>> Paul Bolle
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> [email protected]
>> http://lists.xen.org/xen-devel
>



Attachments:
signature.asc (274.00 B)
OpenPGP digital signature

2014-02-28 20:54:50

by Paul Bolle

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCH] xen: remove XEN_PRIVILEGED_GUEST

On Mon, 2014-02-24 at 20:38 +0100, Vladimir 'φ-coder/phcoder' Serbinenko
wrote:
> On 24.02.2014 19:39, Konrad Rzeszutek Wilk wrote:
> > On Tue, Feb 18, 2014 at 11:14:27AM +0100, Paul Bolle wrote:
> >> And that commit was reverted a week later in grub commit
> >> faf4a65e1e1ce1d822d251c1e4b53d96ec7faec5 ("Revert grub-file usage in
> >> grub-mkconfig."), see
> >> http://git.savannah.gnu.org/cgit/grub.git/commit/util/grub.d/20_linux_xen.in?id=faf4a65e1e1ce1d822d251c1e4b53d96ec7faec5 .
> >>
> >> That commit has no explanation (other than its one line summary). So
> >> we're left guessing why this was done. Luckily, it doesn't matter here,
> >> because the test for CONFIG_XEN_PRIVILEGED_GUEST is superfluous.
> >
> > How about we ask Vladimir?
> >
> > Vladimir - could you shed some light on it? Thanks!
> >
> CONFIG_XEN_PRIVILEGED_GUEST is not present on Linux even though it
> should be. The test was removed to accomodate this.

It's not clear to me what this means, sorry.

> The usage of grub-file was removed because it wasn't release-ready.

I see.

Thanks.


Paul Bolle