Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[David Woodhouse]

On Fri, 2008-04-18 at 18:56 +0000, Linux Kernel Mailing List wrote:
> Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=6b6891f9c545ccd45d6d8ddfd33ce27c22c271a7
> Commit:     6b6891f9c545ccd45d6d8ddfd33ce27c22c271a7
> Parent:     6093015db2bd9e70cf20cdd23be1a50733baafdd
> Author:     gorcunov@gmail.com <gorcunov@gmail.com>
> AuthorDate: Fri Mar 28 17:56:57 2008 +0300
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Thu Apr 17 17:41:33 2008 +0200
> 
>     x86: cleanup - rename VM_MASK to X86_VM_MASK
>     
>     This patch renames VM_MASK to X86_VM_MASK (which
>     in turn defined as alias to X86_EFLAGS_VM) to better
>     distinguish from virtual memory flags. We can't just
>     use X86_EFLAGS_VM instead because it is also used
>     for conditional compilation
>     
>     Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
>     Signed-off-by: Ingo Molnar <mingo@elte.hu>


> --- a/include/asm-x86/vm86.h
> +++ b/include/asm-x86/vm86.h
> @@ -17,9 +17,9 @@
>  #define IOPL_MASK	0x00003000
>  #define NT_MASK		0x00004000
>  #ifdef CONFIG_VM86
> -#define VM_MASK		0x00020000
> +#define X86_VM_MASK	X86_EFLAGS_VM
>  #else
> -#define VM_MASK		0 /* ignored */
> +#define X86_VM_MASK	0 /* No VM86 support */
>  #endif
>  #define AC_MASK		0x00040000
>  #define VIF_MASK	0x00080000	/* virtual interrupt flag */

This is user-visible. Yet we're changing the name and also making it
depend on #ifdef CONFIG_VM86, which is not going to be set in userspace.
Perhaps it should be within #ifdef __KERNEL__?

-- 
dwmw2

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Cyrill Gorcunov]

On Wed, Apr 23, 2008 at 6:17 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Fri, 2008-04-18 at 18:56 +0000, Linux Kernel Mailing List wrote:
>  > Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=6b6891f9c545ccd45d6d8ddfd33ce27c22c271a7
>  > Commit:     6b6891f9c545ccd45d6d8ddfd33ce27c22c271a7
>  > Parent:     6093015db2bd9e70cf20cdd23be1a50733baafdd
>  > Author:     gorcunov@gmail.com <gorcunov@gmail.com>
>  > AuthorDate: Fri Mar 28 17:56:57 2008 +0300
>  > Committer:  Ingo Molnar <mingo@elte.hu>
>  > CommitDate: Thu Apr 17 17:41:33 2008 +0200
>  >
>  >     x86: cleanup - rename VM_MASK to X86_VM_MASK
>  >
>  >     This patch renames VM_MASK to X86_VM_MASK (which
>  >     in turn defined as alias to X86_EFLAGS_VM) to better
>  >     distinguish from virtual memory flags. We can't just
>  >     use X86_EFLAGS_VM instead because it is also used
>  >     for conditional compilation
>  >
>  >     Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
>  >     Signed-off-by: Ingo Molnar <mingo@elte.hu>
>
>
>  > --- a/include/asm-x86/vm86.h
>  > +++ b/include/asm-x86/vm86.h
>  > @@ -17,9 +17,9 @@
>  >  #define IOPL_MASK    0x00003000
>  >  #define NT_MASK              0x00004000
>  >  #ifdef CONFIG_VM86
>  > -#define VM_MASK              0x00020000
>  > +#define X86_VM_MASK  X86_EFLAGS_VM
>  >  #else
>  > -#define VM_MASK              0 /* ignored */
>  > +#define X86_VM_MASK  0 /* No VM86 support */
>  >  #endif
>  >  #define AC_MASK              0x00040000
>  >  #define VIF_MASK     0x00080000      /* virtual interrupt flag */
>
>  This is user-visible. Yet we're changing the name and also making it
>  depend on #ifdef CONFIG_VM86, which is not going to be set in userspace.
>  Perhaps it should be within #ifdef __KERNEL__?
>
>  --
>  dwmw2
>
>

Thanks David, will check it today evening (i'm in office now)

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Cyrill Gorcunov]

On Wed, Apr 23, 2008 at 8:18 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
>
> On Wed, Apr 23, 2008 at 6:17 AM, David Woodhouse <dwmw2@infradead.org> wrote:
>  > On Fri, 2008-04-18 at 18:56 +0000, Linux Kernel Mailing List wrote:
>  >  > Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=6b6891f9c545ccd45d6d8ddfd33ce27c22c271a7
>  >  > Commit:     6b6891f9c545ccd45d6d8ddfd33ce27c22c271a7
>  >  > Parent:     6093015db2bd9e70cf20cdd23be1a50733baafdd
>  >  > Author:     gorcunov@gmail.com <gorcunov@gmail.com>
>  >  > AuthorDate: Fri Mar 28 17:56:57 2008 +0300
>  >  > Committer:  Ingo Molnar <mingo@elte.hu>
>  >  > CommitDate: Thu Apr 17 17:41:33 2008 +0200
>  >  >
>  >  >     x86: cleanup - rename VM_MASK to X86_VM_MASK
>  >  >
>  >  >     This patch renames VM_MASK to X86_VM_MASK (which
>  >  >     in turn defined as alias to X86_EFLAGS_VM) to better
>  >  >     distinguish from virtual memory flags. We can't just
>  >  >     use X86_EFLAGS_VM instead because it is also used
>  >  >     for conditional compilation
>  >  >
>  >  >     Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
>  >  >     Signed-off-by: Ingo Molnar <mingo@elte.hu>
>  >
>  >
>  >  > --- a/include/asm-x86/vm86.h
>  >  > +++ b/include/asm-x86/vm86.h
>  >  > @@ -17,9 +17,9 @@
>  >  >  #define IOPL_MASK    0x00003000
>  >  >  #define NT_MASK              0x00004000
>  >  >  #ifdef CONFIG_VM86
>  >  > -#define VM_MASK              0x00020000
>  >  > +#define X86_VM_MASK  X86_EFLAGS_VM
>  >  >  #else
>  >  > -#define VM_MASK              0 /* ignored */
>  >  > +#define X86_VM_MASK  0 /* No VM86 support */
>  >  >  #endif
>  >  >  #define AC_MASK              0x00040000
>  >  >  #define VIF_MASK     0x00080000      /* virtual interrupt flag */
>  >
>  >  This is user-visible. Yet we're changing the name and also making it
>  >  depend on #ifdef CONFIG_VM86, which is not going to be set in userspace.
>  >  Perhaps it should be within #ifdef __KERNEL__?
>  >
>  >  --
>  >  dwmw2
>  >
>  >
>
>  Thanks David, will check it today evening (i'm in office now)
>

Hi David,

actually, this CONFIG_VM86 was there even before the renaming was done.
The main questions (imo) - is there any user space application who uses
these flags? If they are - then even the idea of this patch was a bit bogus,
and I should *not* remove all these VM86 specific flags but better define them
as aliases on flags from processor-flags.h.

Ingo? Peter?

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[H. Peter Anvin]

Cyrill Gorcunov wrote:
> 
> Hi David,
> 
> actually, this CONFIG_VM86 was there even before the renaming was done.
> The main questions (imo) - is there any user space application who uses
> these flags? If they are - then even the idea of this patch was a bit bogus,
> and I should *not* remove all these VM86 specific flags but better define them
> as aliases on flags from processor-flags.h.
> 

Hard to know.  VM86 is only used by a handful of applications (DOSEMU, 
X.org, others?) but it's hard to know what exactly they are.

The fact that noone hollered when that CONFIG_VM86 went in is probably a 
good hint that they don't matter, but it's hard to say for sure.

	-hpa

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Cyrill Gorcunov]

[H. Peter Anvin - Wed, Apr 23, 2008 at 11:12:33AM -0700]
> Cyrill Gorcunov wrote:
>> Hi David,
>> actually, this CONFIG_VM86 was there even before the renaming was done.
>> The main questions (imo) - is there any user space application who uses
>> these flags? If they are - then even the idea of this patch was a bit 
>> bogus,
>> and I should *not* remove all these VM86 specific flags but better define 
>> them
>> as aliases on flags from processor-flags.h.
>
> Hard to know.  VM86 is only used by a handful of applications (DOSEMU, 
> X.org, others?) but it's hard to know what exactly they are.
>
> The fact that noone hollered when that CONFIG_VM86 went in is probably a 
> good hint that they don't matter, but it's hard to say for sure.
>
> 	-hpa
>

Actually, being google a bit I found that most of emulators
redefine their own equvalence of these flags. So I think
we could remain all as is. But if we wanna be sure I could
make a patch like this

...
#define CF_MASK X86_EFLAGS_CF
...

		- Cyrill -

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Cyrill Gorcunov]

[H. Peter Anvin - Wed, Apr 23, 2008 at 11:12:33AM -0700]
> Cyrill Gorcunov wrote:
>> Hi David,
>> actually, this CONFIG_VM86 was there even before the renaming was done.
>> The main questions (imo) - is there any user space application who uses
>> these flags? If they are - then even the idea of this patch was a bit 
>> bogus,
>> and I should *not* remove all these VM86 specific flags but better define 
>> them
>> as aliases on flags from processor-flags.h.
>
> Hard to know.  VM86 is only used by a handful of applications (DOSEMU, 
> X.org, others?) but it's hard to know what exactly they are.
>
> The fact that noone hollered when that CONFIG_VM86 went in is probably a 
> good hint that they don't matter, but it's hard to say for sure.
>
> 	-hpa
>

Peter, Ingo,

any objections on the patch like that?

---
From: Cyrill Gorcunov <gorcunov@gmail.com>
Subject: [PATCH] reintroduce old VM86 flags for userlang backward compatibility

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

Index: linux-2.6.git/include/asm-x86/vm86.h
===================================================================
--- linux-2.6.git.orig/include/asm-x86/vm86.h	2008-04-23 22:38:58.000000000 +0400
+++ linux-2.6.git/include/asm-x86/vm86.h	2008-04-23 22:41:06.000000000 +0400
@@ -14,6 +14,18 @@
 
 #include <asm/processor-flags.h>
 
+/* backward compatibility for userland */
+#ifndef __KERNEL__
+#define TF_MASK		X86_EFLAGS_TF
+#define IF_MASK		X86_EFLAGS_IF
+#define IOPL_MASK	X86_EFLAGS_IOPL
+#define NT_MASK		X86_EFLAGS_NT
+#define AC_MASK		X86_EFLAGS_AC
+#define VIF_MASK	X86_EFLAGS_VIF
+#define VIP_MASK	X86_EFLAGS_VIP
+#define ID_MASK		X86_EFLAGS_ID
+#endif
+
 #ifdef CONFIG_VM86
 #define X86_VM_MASK	X86_EFLAGS_VM
 #else

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[H. Peter Anvin]

Cyrill Gorcunov wrote:
> 
> Peter, Ingo,
> 
> any objections on the patch like that?
> 

Well, I would prefer to try to get rid of them; as you said, most 
userland don't rely on these, and if they aren't used they are just 
namespace pollution, and rather bad such.

	-hpa

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[David Woodhouse]

On Wed, 2008-04-23 at 15:38 -0700, H. Peter Anvin wrote:
> Cyrill Gorcunov wrote:
> > 
> > Peter, Ingo,
> > 
> > any objections on the patch like that?
> > 
> 
> Well, I would prefer to try to get rid of them; as you said, most 
> userland don't rely on these, and if they aren't used they are just 
> namespace pollution, and rather bad such.

Yeah, I'd prefer just to kill them off if we think we can get away with
it. These are hardware-defined flags, not Linux-specific, so userspace
should probably get them from elsewhere anyway.

-- 
dwmw2

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Cyrill Gorcunov]

Ok, lets drop these all and forget :)

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[David Woodhouse]

On Thu, 2008-04-24 at 07:25 +0400, Cyrill Gorcunov wrote:
> Ok, lets drop these all and forget :)

We still want to shift the definition of X86_VM_MASK inside __KERNEL__,
don't we?

-- 
dwmw2

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[H. Peter Anvin]

David Woodhouse wrote:
> On Thu, 2008-04-24 at 07:25 +0400, Cyrill Gorcunov wrote:
>> Ok, lets drop these all and forget :)
> 
> We still want to shift the definition of X86_VM_MASK inside __KERNEL__,
> don't we?

Yes.

	-hpa

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Cyrill Gorcunov]

[H. Peter Anvin - Wed, Apr 23, 2008 at 10:34:26PM -0700]
> David Woodhouse wrote:
>> On Thu, 2008-04-24 at 07:25 +0400, Cyrill Gorcunov wrote:
>>> Ok, lets drop these all and forget :)
>> We still want to shift the definition of X86_VM_MASK inside __KERNEL__,
>> don't we?
>
> Yes.
>
> 	-hpa
>

I've just posted the patch, please review (i'm to go out of
my work machine so if there would be any objections I read
them today evening, thanks)

		- Cyrill -

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Pavel Machek]

On Wed 2008-04-23 11:12:33, H. Peter Anvin wrote:
> Cyrill Gorcunov wrote:
> >
> >Hi David,
> >
> >actually, this CONFIG_VM86 was there even before the 
> >renaming was done.
> >The main questions (imo) - is there any user space 
> >application who uses
> >these flags? If they are - then even the idea of this 
> >patch was a bit bogus,
> >and I should *not* remove all these VM86 specific flags 
> >but better define them
> >as aliases on flags from processor-flags.h.
> >
> 
> Hard to know.  VM86 is only used by a handful of 
> applications (DOSEMU, X.org, others?) but it's hard to 
> know what exactly they are.

I believe vbetool and thus s2ram uses vm86, too.

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Rafael J. Wysocki]

On Sunday, 27 of April 2008, Pavel Machek wrote:
> On Wed 2008-04-23 11:12:33, H. Peter Anvin wrote:
> > Cyrill Gorcunov wrote:
> > >
> > >Hi David,
> > >
> > >actually, this CONFIG_VM86 was there even before the 
> > >renaming was done.
> > >The main questions (imo) - is there any user space 
> > >application who uses
> > >these flags? If they are - then even the idea of this 
> > >patch was a bit bogus,
> > >and I should *not* remove all these VM86 specific flags 
> > >but better define them
> > >as aliases on flags from processor-flags.h.
> > >
> > 
> > Hard to know.  VM86 is only used by a handful of 
> > applications (DOSEMU, X.org, others?) but it's hard to 
> > know what exactly they are.
> 
> I believe vbetool and thus s2ram uses vm86, too.

Well, I think so too. Matthew?

Rafael

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Matthew Garrett]

On Sun, Apr 27, 2008 at 01:31:17PM +0200, Rafael J. Wysocki wrote:
> On Sunday, 27 of April 2008, Pavel Machek wrote:
> > I believe vbetool and thus s2ram uses vm86, too.
> 
> Well, I think so too. Matthew?

Yes, though I don't think any of them use these internal flags.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: x86: cleanup - rename VM_MASK to X86_VM_MASK

[Cyrill Gorcunov]

[Matthew Garrett - Sun, Apr 27, 2008 at 12:55:37PM +0100]
| On Sun, Apr 27, 2008 at 01:31:17PM +0200, Rafael J. Wysocki wrote:
| > On Sunday, 27 of April 2008, Pavel Machek wrote:
| > > I believe vbetool and thus s2ram uses vm86, too.
| > 
| > Well, I think so too. Matthew?
| 
| Yes, though I don't think any of them use these internal flags.
| 
| -- 
| Matthew Garrett | mjg59@srcf.ucam.org
| 

Originally this flag was bounded by CONFIG_VM86 which is
kernel internal feature and if userland program relies on
this it's a bit buggy assumption meguess. Anyway, i've posted
second version of the patch (don't remember lkml.org link for
that).
		- Cyrill -