regression: grass turns red

regression: grass turns red

[Hans de Bruin]

commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old' 
somehow breaks the colors when I play 'civilization I' under xdosemu. 
During the intro of the game something the colors get messed up. When 
the game begins the grass of the earth is red. Reverting the  commit 
fixes the problem.

-- 
Hans

Re: regression: grass turns red

[Alexander van Heukelum]

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

Hi Hans,

Could you check if the attached patch solves your problem?

Greetings,
    Alexander van Heukelum

On Sun, Mar 24, 2013, at 22:19, Hans de Bruin wrote:
> commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old' 
> somehow breaks the colors when I play 'civilization I' under xdosemu. 
> During the intro of the game something the colors get messed up. When 
> the game begins the grass of the earth is red. Reverting the  commit 
> fixes the problem.
> 
> -- 
> Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-x86-vm86-fix-VM86-syscalls-use-asmlinkage-calling-co.patch --]
[-- Type: text/x-patch; name="0001-x86-vm86-fix-VM86-syscalls-use-asmlinkage-calling-co.patch", Size: 2490 bytes --]

From 2b09f37fd9defc02c6da9900e23418a401d7a2b9 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum <heukelum@fastmail.fm>
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use asmlinkage calling convention

This might solve the issue of the red grass in 'civilization I' under xdosemu. Commit
49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old' got rid of the pt_regs
stub for sys_vm86old and sys_vm86. The functions were, however, not changed to use the
asmlinkage calling convention.

Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/include/asm/syscalls.h | 4 ++--
 arch/x86/kernel/vm86_32.c       | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..a245b88 100644
--- a/arch/x86/include/asm/syscalls.h
+++ b/arch/x86/include/asm/syscalls.h
@@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struct user_desc __user *);
 unsigned long sys_sigreturn(void);
 
 /* kernel/vm86_32.c */
-int sys_vm86old(struct vm86_struct __user *);
-int sys_vm86(unsigned long, unsigned long);
+asmlinkage int sys_vm86old(struct vm86_struct __user *);
+asmlinkage int sys_vm86(unsigned long, unsigned long);
 
 #else /* CONFIG_X86_32 */
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 1cf5766..7f72807 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -202,7 +202,7 @@ out:
 static int do_vm86_irq_handling(int subfunction, int irqnumber);
 static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk);
 
-int sys_vm86old(struct vm86_struct __user *v86)
+asmlinkage int sys_vm86old(struct vm86_struct __user *v86)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -227,11 +227,12 @@ int sys_vm86old(struct vm86_struct __user *v86)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(1, ret, v86);
 	return ret;
 }
 
 
-int sys_vm86(unsigned long cmd, unsigned long arg)
+asmlinkage int sys_vm86(unsigned long cmd, unsigned long arg)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -278,6 +279,7 @@ int sys_vm86(unsigned long cmd, unsigned long arg)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(2, ret, cmd, arg);
 	return ret;
 }
 
-- 
1.8.1.2

Re: regression: grass turns red

[Hans de Bruin]

On 03/26/2013 10:15 PM, Alexander van Heukelum wrote:
> Hi Hans,
>
> Could you check if the attached patch solves your problem?
>

Yep, the grass is green again.

-- 
Hans

[PATCH] x86, vm86: fix VM86 syscalls: use asmlinkage calling convention

[Alexander van Heukelum]

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

Hi Al,

Hans de Bruin found a regression due to one of your changes. I asked him to test a fix and he reported back that it worked. (Thanks!) Can you see if you agree with the fix? Patch is attached due to webmail...

Greetings,
    Alexander

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-x86-vm86-fix-VM86-syscalls-use-asmlinkage-calling-co.patch --]
[-- Type: text/x-patch; name="0001-x86-vm86-fix-VM86-syscalls-use-asmlinkage-calling-co.patch", Size: 2903 bytes --]

From 961a1b130aa79acb54f556a0accfcc643d1d3ed1 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum <heukelum@fastmail.fm>
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use asmlinkage calling convention

Commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
got rid of the pt_regs stub for sys_vm86old and sys_vm86. The functions
were, however, not changed to use the asmlinkage calling convention.

The regression was reported and pinpointed by Hans de Bruin:
> commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
> somehow breaks the colors when I play 'civilization I' under xdosemu.
> During the intro of the game something the colors get messed up. When
> the game begins the grass of the earth is red. Reverting the commit
> fixes the problem.

And he tested the patch too:
> Yep, the grass is green again.

Reported-and-tested-by: Hans de Bruin <jmdebruin@xmsnet.nl>
Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/include/asm/syscalls.h | 4 ++--
 arch/x86/kernel/vm86_32.c       | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..a245b88 100644
--- a/arch/x86/include/asm/syscalls.h
+++ b/arch/x86/include/asm/syscalls.h
@@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struct user_desc __user *);
 unsigned long sys_sigreturn(void);
 
 /* kernel/vm86_32.c */
-int sys_vm86old(struct vm86_struct __user *);
-int sys_vm86(unsigned long, unsigned long);
+asmlinkage int sys_vm86old(struct vm86_struct __user *);
+asmlinkage int sys_vm86(unsigned long, unsigned long);
 
 #else /* CONFIG_X86_32 */
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 1cf5766..7f72807 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -202,7 +202,7 @@ out:
 static int do_vm86_irq_handling(int subfunction, int irqnumber);
 static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk);
 
-int sys_vm86old(struct vm86_struct __user *v86)
+asmlinkage int sys_vm86old(struct vm86_struct __user *v86)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -227,11 +227,12 @@ int sys_vm86old(struct vm86_struct __user *v86)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(1, ret, v86);
 	return ret;
 }
 
 
-int sys_vm86(unsigned long cmd, unsigned long arg)
+asmlinkage int sys_vm86(unsigned long cmd, unsigned long arg)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -278,6 +279,7 @@ int sys_vm86(unsigned long cmd, unsigned long arg)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(2, ret, cmd, arg);
 	return ret;
 }
 
-- 
1.8.1.2

Re: [PATCH] x86, vm86: fix VM86 syscalls: use asmlinkage calling convention

[Al Viro]

On Wed, Mar 27, 2013 at 08:31:02PM +0100, Alexander van Heukelum wrote:
> Hi Al,
> 
> Hans de Bruin found a regression due to one of your changes. I asked him to test a fix and he reported back that it worked. (Thanks!) Can you see if you agree with the fix? Patch is attached due to webmail...

Use SYSCALL_DEFINE{1,2} for vm86_old and vm86, please.

[PATCH v2] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

[Alexander van Heukelum]

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

On Wed, Mar 27, 2013, at 20:46, Al Viro wrote:
> On Wed, Mar 27, 2013 at 08:31:02PM +0100, Alexander van Heukelum wrote:
> > Hi Al,
> > 
> > Hans de Bruin found a regression due to one of your changes. I asked him to test a fix and he reported back that it worked. (Thanks!) Can you see if you agree with the fix? Patch is attached due to webmail...
> 
> Use SYSCALL_DEFINE{1,2} for vm86_old and vm86, please.

Like this?

Greetings,
    Alexander

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-x86-vm86-fix-VM86-syscalls-use-SYSCALL_DEFINEx.patch --]
[-- Type: text/x-patch; name="0001-x86-vm86-fix-VM86-syscalls-use-SYSCALL_DEFINEx.patch", Size: 3315 bytes --]

From 450d86e6ad0a7d387cf706714c1fc030bb4b13a5 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum <heukelum@fastmail.fm>
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

Commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
got rid of the pt_regs stub for sys_vm86old and sys_vm86. The functions
were, however, not changed to use the calling convention for syscalls.

[v2] Use SYSCALL_DEFINEx(...). Compiles to identical code.

The regression was reported and pinpointed by Hans de Bruin:
> commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
> somehow breaks the colors when I play 'civilization I' under xdosemu.
> During the intro of the game something the colors get messed up. When
> the game begins the grass of the earth is red. Reverting the commit
> fixes the problem.

And he tested the patch too:
> Yep, the grass is green again.

Reported-and-tested-by: Hans de Bruin <jmdebruin@xmsnet.nl>
Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/include/asm/syscalls.h | 4 ++--
 arch/x86/kernel/vm86_32.c       | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..5a0be0a 100644
--- a/arch/x86/include/asm/syscalls.h
+++ b/arch/x86/include/asm/syscalls.h
@@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struct user_desc __user *);
 unsigned long sys_sigreturn(void);
 
 /* kernel/vm86_32.c */
-int sys_vm86old(struct vm86_struct __user *);
-int sys_vm86(unsigned long, unsigned long);
+asmlinkage long sys_vm86old(struct vm86_struct __user *);
+asmlinkage long sys_vm86(unsigned long, unsigned long);
 
 #else /* CONFIG_X86_32 */
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 1cf5766..a67cb2b 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -33,6 +33,7 @@
 #include <linux/capability.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
+#include <linux/syscalls.h>
 #include <linux/sched.h>
 #include <linux/kernel.h>
 #include <linux/signal.h>
@@ -48,7 +49,6 @@
 #include <asm/io.h>
 #include <asm/tlbflush.h>
 #include <asm/irq.h>
-#include <asm/syscalls.h>
 
 /*
  * Known problems:
@@ -202,7 +202,7 @@ out:
 static int do_vm86_irq_handling(int subfunction, int irqnumber);
 static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk);
 
-int sys_vm86old(struct vm86_struct __user *v86)
+SYSCALL_DEFINE1(vm86old, struct vm86_struct __user *, v86)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -227,11 +227,12 @@ int sys_vm86old(struct vm86_struct __user *v86)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(1, ret, v86);
 	return ret;
 }
 
 
-int sys_vm86(unsigned long cmd, unsigned long arg)
+SYSCALL_DEFINE2(vm86, unsigned long, cmd, unsigned long, arg)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -278,6 +279,7 @@ int sys_vm86(unsigned long cmd, unsigned long arg)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(2, ret, cmd, arg);
 	return ret;
 }
 
-- 
1.8.1.2

Re: [PATCH v2] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

[Hans de Bruin]

On 03/27/2013 10:18 PM, Alexander van Heukelum wrote:
> On Wed, Mar 27, 2013, at 20:46, Al Viro wrote:
>> On Wed, Mar 27, 2013 at 08:31:02PM +0100, Alexander van Heukelum wrote:
>>> Hi Al,
>>>
>>> Hans de Bruin found a regression due to one of your changes. I asked him to test a fix and he reported back that it worked. (Thanks!) Can you see if you agree with the fix? Patch is attached due to webmail...
>>
>> Use SYSCALL_DEFINE{1,2} for vm86_old and vm86, please.
>
> Like this?
>
> Greetings,
>      Alexander
>

Is there any progress?

-- 
Hans

Re: [PATCH v2] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

[Alexander van Heukelum]

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

On Fri, Apr 12, 2013, at 22:15, Hans de Bruin wrote:
> On 03/27/2013 10:18 PM, Alexander van Heukelum wrote:
> > On Wed, Mar 27, 2013, at 20:46, Al Viro wrote:
> >> On Wed, Mar 27, 2013 at 08:31:02PM +0100, Alexander van Heukelum wrote:
> >>> Hi Al,
> >>>
> >>> Hans de Bruin found a regression due to one of your changes. I asked him to test a fix and he reported back that it worked. (Thanks!) Can you see if you agree with the fix? Patch is attached due to webmail...
> >>
> >> Use SYSCALL_DEFINE{1,2} for vm86_old and vm86, please.
> >
> > Like this?

Hi Al,

Could you comment on the patch for the problem with the VM86 calls? I changed it to use SYSCALL_DEFINEx at the definition sites, and I changed the declarations in asm/syscalls.h to fit. If things are ok, can you take care of sending it to Linus?

Greetings,
    Alexander

> > Greetings,
> >      Alexander
> >
> 
> Is there any progress?
>
> -- 
> Hans

[-- Attachment #2: 0001-x86-vm86-fix-VM86-syscalls-use-SYSCALL_DEFINEx.patch --]
[-- Type: application/octet-stream, Size: 3315 bytes --]

From 450d86e6ad0a7d387cf706714c1fc030bb4b13a5 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum <heukelum@fastmail.fm>
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

Commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
got rid of the pt_regs stub for sys_vm86old and sys_vm86. The functions
were, however, not changed to use the calling convention for syscalls.

[v2] Use SYSCALL_DEFINEx(...). Compiles to identical code.

The regression was reported and pinpointed by Hans de Bruin:
> commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
> somehow breaks the colors when I play 'civilization I' under xdosemu.
> During the intro of the game something the colors get messed up. When
> the game begins the grass of the earth is red. Reverting the commit
> fixes the problem.

And he tested the patch too:
> Yep, the grass is green again.

Reported-and-tested-by: Hans de Bruin <jmdebruin@xmsnet.nl>
Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/include/asm/syscalls.h | 4 ++--
 arch/x86/kernel/vm86_32.c       | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..5a0be0a 100644
--- a/arch/x86/include/asm/syscalls.h
+++ b/arch/x86/include/asm/syscalls.h
@@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struct user_desc __user *);
 unsigned long sys_sigreturn(void);
 
 /* kernel/vm86_32.c */
-int sys_vm86old(struct vm86_struct __user *);
-int sys_vm86(unsigned long, unsigned long);
+asmlinkage long sys_vm86old(struct vm86_struct __user *);
+asmlinkage long sys_vm86(unsigned long, unsigned long);
 
 #else /* CONFIG_X86_32 */
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 1cf5766..a67cb2b 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -33,6 +33,7 @@
 #include <linux/capability.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
+#include <linux/syscalls.h>
 #include <linux/sched.h>
 #include <linux/kernel.h>
 #include <linux/signal.h>
@@ -48,7 +49,6 @@
 #include <asm/io.h>
 #include <asm/tlbflush.h>
 #include <asm/irq.h>
-#include <asm/syscalls.h>
 
 /*
  * Known problems:
@@ -202,7 +202,7 @@ out:
 static int do_vm86_irq_handling(int subfunction, int irqnumber);
 static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk);
 
-int sys_vm86old(struct vm86_struct __user *v86)
+SYSCALL_DEFINE1(vm86old, struct vm86_struct __user *, v86)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -227,11 +227,12 @@ int sys_vm86old(struct vm86_struct __user *v86)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(1, ret, v86);
 	return ret;
 }
 
 
-int sys_vm86(unsigned long cmd, unsigned long arg)
+SYSCALL_DEFINE2(vm86, unsigned long, cmd, unsigned long, arg)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -278,6 +279,7 @@ int sys_vm86(unsigned long cmd, unsigned long arg)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(2, ret, cmd, arg);
 	return ret;
 }
 
-- 
1.8.1.2