Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

[Konrad Rzeszutek Wilk]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 2205 bytes --]


On Feb 16, 2014 3:07 PM, Paul Bolle <pebolle@tiscali.nl> 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. 
>
> [pebolle@tiscali.nl: rewrote commit explanation.] 
> Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl> 
> --- 
> 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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

[Paul Bolle]

On Mon, 2014-02-17 at 07:23 -0500, Konrad Rzeszutek Wilk wrote:
> On Feb 16, 2014 3:07 PM, Paul Bolle <pebolle@tiscali.nl> 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

Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

[Konrad Rzeszutek Wilk]

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 <pebolle@tiscali.nl> 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
> 

Re: [PATCH] xen: remove XEN_PRIVILEGED_GUEST

[Paul Bolle]

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 <pebolle@tiscali.nl> 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

[PATCH v2] xen: remove XEN_PRIVILEGED_GUEST

[Paul Bolle]

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.

[pebolle@tiscali.nl: rewrote commit explanation.]
Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com>
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
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

Re: [PATCH v2] xen: remove XEN_PRIVILEGED_GUEST

[Paul Bolle]

Here I should have added:

From:  Michael Opdenacker <michael.opdenacker@free-electrons.com>

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.
> 
> [pebolle@tiscali.nl: rewrote commit explanation.]
> Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>


Paul Bolle

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

[David Vrabel]

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 <david.vrabel@citrix.com>

David

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

[Konrad Rzeszutek Wilk]

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 <pebolle@tiscali.nl> 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
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

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 <pebolle@tiscali.nl> 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
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 274 bytes --]

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

[Paul Bolle]

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