2010-08-27 18:07:27

by Matteo Croce

[permalink] [raw]
Subject: AMD Geode NOPL emulation for kernel 2.6.36-rc2

Hi,
I have received many mails asking to refresh the patch so I decided to
post them on the public mailing list
In this version such feature is configurable via a kernel config option

Signed-off-by: Matteo Croce <[email protected]>

--- a/arch/x86/kernel/Makefile 2010-08-27 00:27:50.101261000 +0200
+++ b/arch/x86/kernel/Makefile 2010-08-27 00:31:11.391261002 +0200
@@ -88,6 +88,8 @@
obj-$(CONFIG_APB_TIMER) += apb_timer.o

obj-$(CONFIG_K8_NB) += k8.o
+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
obj-$(CONFIG_DEBUG_RODATA_TEST) += test_rodata.o
obj-$(CONFIG_DEBUG_NX_TEST) += test_nx.o

--- a/arch/x86/kernel/cpu/amd.c 2010-08-27 00:27:50.161261000 +0200
+++ b/arch/x86/kernel/cpu/amd.c 2010-08-27 00:34:13.371261000 +0200
@@ -137,11 +137,15 @@
return;
}

+#ifdef CONFIG_GEODE_NOPL
if (c->x86_model == 10) {
- /* AMD Geode LX is model 10 */
- /* placeholder for any needed mods */
+ /* Geode only lacks the NOPL instruction to be i686,
+ but we can promote it to a i686 class cpu
+ and emulate NOPLs in the exception handler*/
+ boot_cpu_data.x86 = 6;
return;
}
+#endif
}

static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c)
--- a/arch/x86/kernel/entry_32.S 2010-08-27 00:27:50.051261000 +0200
+++ b/arch/x86/kernel/entry_32.S 2010-08-27 00:57:35.531261000 +0200
@@ -978,7 +978,11 @@
RING0_INT_FRAME
pushl $0
CFI_ADJUST_CFA_OFFSET 4
+#ifdef CONFIG_GEODE_NOPL
+ pushl $do_nopl_emu
+#else
pushl $do_invalid_op
+#endif
CFI_ADJUST_CFA_OFFSET 4
jmp error_code
CFI_ENDPROC
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ b/arch/x86/kernel/nopl_emu.c 2010-08-27 00:27:57.881261002 +0200
@@ -0,0 +1,110 @@
+/*
+ * linux/arch/x86/kernel/nopl_emu.c
+ *
+ * Copyright (C) 2002 Willy Tarreau
+ * Copyright (C) 2010 Matteo Croce
+ */
+
+#include <asm/processor.h>
+#include <asm/traps.h>
+
+/* This code can be used to allow the AMD Geode to hopefully correctly execute
+ * some code which was originally compiled for an i686, by emulating NOPL,
+ * the only missing i686 instruction in the CPU
+ *
+ * Willy Tarreau <[email protected]>
+ * Matteo Croce <[email protected]>
+ */
+
+static inline int do_1f(u8 *ip)
+{
+ int length = 3;
+ switch (*ip) {
+ case 0x84:
+ if (!ip[5])
+ length++;
+ else
+ return 0;
+ case 0x80:
+ if (!ip[4] && !ip[3])
+ length += 2;
+ else
+ return 0;
+ case 0x44:
+ if (!ip[2])
+ length++;
+ else
+ return 0;
+ case 0x40:
+ if (!ip[1])
+ length++;
+ else
+ return 0;
+ case 0x00:
+ return length;
+ }
+ return 0;
+}
+
+static inline int do_0f(u8 *ip)
+{
+ if (*ip == 0x1f)
+ return do_1f(ip + 1);
+ return 0;
+}
+
+static inline int do_66(u8 *ip)
+{
+ if (*ip == 0x90)
+ return 2;
+ if (*ip == 0x0f) {
+ int res = do_0f(ip + 1);
+ if (res)
+ return res + 1;
+ else
+ return 0;
+ }
+ return 0;
+}
+
+static inline int do_start(u8 *ip)
+{
+ if (*ip == 0x0f)
+ return do_0f(ip + 1);
+ if (*ip == 0x66)
+ return do_66(ip + 1);
+ return 0;
+}
+
+/* [do_nopl_emu] is called by exception 6 after an invalid opcode has been
+ * encountered. It will try to emulate it by doing nothing,
+ * and will send a SIGILL or SIGSEGV to the process if not possible.
+ * the NOPL can have variable length opcodes:
+
+bytes number opcode
+ 2 66 90
+ 3 0f 1f 00
+ 4 0f 1f 40 00
+ 5 0f 1f 44 00 00
+ 6 66 0f 1f 44 00 00
+ 7 0f 1f 80 00 00 00 00
+ 8 0f 1f 84 00 00 00 00 00
+ 9 66 0f 1f 84 00 00 00 00 00
+*/
+void do_nopl_emu(struct pt_regs *regs, long error_code)
+{
+ u8 *ip = (u8 *)instruction_pointer(regs);
+ int res = do_start(ip);
+
+ if (res) {
+ int i = 0;
+ do {
+ ip += res;
+ i++;
+ res = do_start(ip);
+ } while(res);
+ printk(KERN_DEBUG "geode_nopl: emulated %d instructions\n", i);
+ regs->ip = (typeof(regs->ip))ip;
+ } else
+ do_invalid_op(regs, error_code);
+}


--
Matteo Croce
OpenWrt developer
  _______                     ________        __
 |       |.-----.-----.-----.|  |  |  |.----.|  |_
 |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
 |_______||   __|_____|__|__||________||__|  |____|
          |__| W I R E L E S S   F R E E D O M
 KAMIKAZE (bleeding edge) ------------------
  * 10 oz Vodka       Shake well with ice and strain
  * 10 oz Triple sec  mixture into 10 shot glasses.
  * 10 oz lime juice  Salute!
 ---------------------------------------------------


2010-08-27 18:48:55

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

You're doing user-space references without get_user().

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2010-08-27 20:15:57

by Matteo Croce

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

On Fri, Aug 27, 2010 at 8:48 PM, H. Peter Anvin <[email protected]> wrote:
> You're doing user-space references without get_user().
>
>        -hpa
>
> --
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.
>
>

Here with get_user.
I CC Natale which is my beta tester, he have some Alix boards running 24/24
with my patch

--- a/arch/x86/kernel/Makefile 2010-08-27 19:42:01.795858001 +0200
+++ b/arch/x86/kernel/Makefile 2010-08-27 19:42:12.525858001 +0200
@@ -88,6 +88,8 @@
obj-$(CONFIG_APB_TIMER) += apb_timer.o

obj-$(CONFIG_K8_NB) += k8.o
+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
obj-$(CONFIG_DEBUG_RODATA_TEST) += test_rodata.o
obj-$(CONFIG_DEBUG_NX_TEST) += test_nx.o

--- a/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:01.855858001 +0200
+++ b/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:12.535858001 +0200
@@ -137,11 +137,15 @@
return;
}

+#ifdef CONFIG_GEODE_NOPL
if (c->x86_model == 10) {
- /* AMD Geode LX is model 10 */
- /* placeholder for any needed mods */
+ /* Geode only lacks the NOPL instruction to be i686,
+ but we can promote it to a i686 class cpu
+ and emulate NOPLs in the exception handler*/
+ boot_cpu_data.x86 = 6;
return;
}
+#endif
}

static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c)
--- a/arch/x86/kernel/entry_32.S 2010-08-27 19:42:01.735858001 +0200
+++ b/arch/x86/kernel/entry_32.S 2010-08-27 19:42:12.535858001 +0200
@@ -978,7 +978,11 @@
RING0_INT_FRAME
pushl $0
CFI_ADJUST_CFA_OFFSET 4
+#ifdef CONFIG_GEODE_NOPL
+ pushl $do_nopl_emu
+#else
pushl $do_invalid_op
+#endif
CFI_ADJUST_CFA_OFFSET 4
jmp error_code
CFI_ENDPROC
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ b/arch/x86/kernel/nopl_emu.c 2010-08-27 22:09:58.005858001 +0200
@@ -0,0 +1,124 @@
+/*
+ * linux/arch/x86/kernel/nopl_emu.c
+ *
+ * Copyright (C) 2002 Willy Tarreau
+ * Copyright (C) 2010 Matteo Croce
+ */
+
+#include <asm/processor.h>
+#include <asm/traps.h>
+#include <asm/uaccess.h>
+
+/* This code can be used to allow the AMD Geode to hopefully correctly execute
+ * some code which was originally compiled for an i686, by emulating NOPL,
+ * the only missing i686 instruction in the CPU
+ *
+ * Willy Tarreau <[email protected]>
+ * Matteo Croce <[email protected]>
+ */
+
+static inline int do_1f(u8 *ip)
+{
+ u8 val1, val2;
+ int length = 3;
+ get_user(val1, ip);
+ switch (val1) {
+ case 0x84:
+ get_user(val1, ip + 5);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x80:
+ get_user(val1, ip + 4);
+ get_user(val2, ip + 3);
+ if (!val1 && !val2)
+ length += 2;
+ else
+ return 0;
+ case 0x44:
+ get_user(val1, ip + 2);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x40:
+ get_user(val1, ip + 1);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x00:
+ return length;
+ }
+ return 0;
+}
+
+static inline int do_0f(u8 *ip)
+{
+ u8 val;
+ get_user(val, ip);
+ if (val == 0x1f)
+ return do_1f(ip + 1);
+ return 0;
+}
+
+static inline int do_66(u8 *ip)
+{
+ u8 val;
+ get_user(val, ip);
+ if (val == 0x90)
+ return 2;
+ if (val == 0x0f) {
+ int res = do_0f(ip + 1);
+ if (res)
+ return res + 1;
+ else
+ return 0;
+ }
+ return 0;
+}
+
+static inline int do_start(u8 *ip)
+{
+ u8 val;
+ get_user(val, ip);
+ if (val == 0x0f)
+ return do_0f(ip + 1);
+ if (val == 0x66)
+ return do_66(ip + 1);
+ return 0;
+}
+
+/* [do_nopl_emu] is called by exception 6 after an invalid opcode has been
+ * encountered. It will try to emulate it by doing nothing,
+ * and will send a SIGILL or SIGSEGV to the process if not possible.
+ * the NOPL can have variable length opcodes:
+
+bytes number opcode
+ 2 66 90
+ 3 0f 1f 00
+ 4 0f 1f 40 00
+ 5 0f 1f 44 00 00
+ 6 66 0f 1f 44 00 00
+ 7 0f 1f 80 00 00 00 00
+ 8 0f 1f 84 00 00 00 00 00
+ 9 66 0f 1f 84 00 00 00 00 00
+*/
+void do_nopl_emu(struct pt_regs *regs, long error_code)
+{
+ u8 *ip = (u8 *)instruction_pointer(regs);
+ int res = do_start(ip);
+
+ if (res) {
+ int i = 0;
+ do {
+ ip += res;
+ i++;
+ res = do_start(ip);
+ } while(res);
+ printk(KERN_DEBUG "geode_nopl: emulated %d instructions\n", i);
+ regs->ip = (typeof(regs->ip))ip;
+ } else
+ do_invalid_op(regs, error_code);
+}


--
Matteo Croce
OpenWrt developer
  _______                     ________        __
 |       |.-----.-----.-----.|  |  |  |.----.|  |_
 |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
 |_______||   __|_____|__|__||________||__|  |____|
          |__| W I R E L E S S   F R E E D O M
 KAMIKAZE (bleeding edge) ------------------
  * 10 oz Vodka       Shake well with ice and strain
  * 10 oz Triple sec  mixture into 10 shot glasses.
  * 10 oz lime juice  Salute!
 ---------------------------------------------------

2010-08-27 20:50:00

by Thomas Backlund

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

27.08.2010 23:15, Matteo Croce skrev:
> On Fri, Aug 27, 2010 at 8:48 PM, H. Peter Anvin<[email protected]> wrote:
>> You're doing user-space references without get_user().
>>
>> -hpa
>>
>> --
>> H. Peter Anvin, Intel Open Source Technology Center
>> I work for Intel. I don't speak on their behalf.
>>
>>
>
> Here with get_user.
> I CC Natale which is my beta tester, he have some Alix boards running 24/24
> with my patch
>
> --- a/arch/x86/kernel/Makefile 2010-08-27 19:42:01.795858001 +0200
> +++ b/arch/x86/kernel/Makefile 2010-08-27 19:42:12.525858001 +0200
> @@ -88,6 +88,8 @@
> obj-$(CONFIG_APB_TIMER) += apb_timer.o
>
> obj-$(CONFIG_K8_NB) += k8.o
> +obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
> +obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o

Same line added twice...

--
Thomas

2010-08-27 20:55:27

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

You actually have to check the get_user return, or use exception brackets....

"Matteo Croce" <[email protected]> wrote:

>On Fri, Aug 27, 2010 at 8:48 PM, H. Peter Anvin <[email protected]> wrote:
>> You're doing user-space references without get_user().
>>
>>        -hpa
>>
>> --
>> H. Peter Anvin, Intel Open Source Technology Center
>> I work for Intel.  I don't speak on their behalf.
>>
>>
>
>Here with get_user.
>I CC Natale which is my beta tester, he have some Alix boards running 24/24
>with my patch
>
>--- a/arch/x86/kernel/Makefile 2010-08-27 19:42:01.795858001 +0200
>+++ b/arch/x86/kernel/Makefile 2010-08-27 19:42:12.525858001 +0200
>@@ -88,6 +88,8 @@
> obj-$(CONFIG_APB_TIMER) += apb_timer.o
>
> obj-$(CONFIG_K8_NB) += k8.o
>+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
>+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
> obj-$(CONFIG_DEBUG_RODATA_TEST) += test_rodata.o
> obj-$(CONFIG_DEBUG_NX_TEST) += test_nx.o
>
>--- a/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:01.855858001 +0200
>+++ b/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:12.535858001 +0200
>@@ -137,11 +137,15 @@
> return;
> }
>
>+#ifdef CONFIG_GEODE_NOPL
> if (c->x86_model == 10) {
>- /* AMD Geode LX is model 10 */
>- /* placeholder for any needed mods */
>+ /* Geode only lacks the NOPL instruction to be i686,
>+ but we can promote it to a i686 class cpu
>+ and emulate NOPLs in the exception handler*/
>+ boot_cpu_data.x86 = 6;
> return;
> }
>+#endif
> }
>
> static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c)
>--- a/arch/x86/kernel/entry_32.S 2010-08-27 19:42:01.735858001 +0200
>+++ b/arch/x86/kernel/entry_32.S 2010-08-27 19:42:12.535858001 +0200
>@@ -978,7 +978,11 @@
> RING0_INT_FRAME
> pushl $0
> CFI_ADJUST_CFA_OFFSET 4
>+#ifdef CONFIG_GEODE_NOPL
>+ pushl $do_nopl_emu
>+#else
> pushl $do_invalid_op
>+#endif
> CFI_ADJUST_CFA_OFFSET 4
> jmp error_code
> CFI_ENDPROC
>--- /dev/null 1970-01-01 00:00:00.000000000 +0000
>+++ b/arch/x86/kernel/nopl_emu.c 2010-08-27 22:09:58.005858001 +0200
>@@ -0,0 +1,124 @@
>+/*
>+ * linux/arch/x86/kernel/nopl_emu.c
>+ *
>+ * Copyright (C) 2002 Willy Tarreau
>+ * Copyright (C) 2010 Matteo Croce
>+ */
>+
>+#include <asm/processor.h>
>+#include <asm/traps.h>
>+#include <asm/uaccess.h>
>+
>+/* This code can be used to allow the AMD Geode to hopefully correctly execute
>+ * some code which was originally compiled for an i686, by emulating NOPL,
>+ * the only missing i686 instruction in the CPU
>+ *
>+ * Willy Tarreau <[email protected]>
>+ * Matteo Croce <[email protected]>
>+ */
>+
>+static inline int do_1f(u8 *ip)
>+{
>+ u8 val1, val2;
>+ int length = 3;
>+ get_user(val1, ip);
>+ switch (val1) {
>+ case 0x84:
>+ get_user(val1, ip + 5);
>+ if (!val1)
>+ length++;
>+ else
>+ return 0;
>+ case 0x80:
>+ get_user(val1, ip + 4);
>+ get_user(val2, ip + 3);
>+ if (!val1 && !val2)
>+ length += 2;
>+ else
>+ return 0;
>+ case 0x44:
>+ get_user(val1, ip + 2);
>+ if (!val1)
>+ length++;
>+ else
>+ return 0;
>+ case 0x40:
>+ get_user(val1, ip + 1);
>+ if (!val1)
>+ length++;
>+ else
>+ return 0;
>+ case 0x00:
>+ return length;
>+ }
>+ return 0;
>+}
>+
>+static inline int do_0f(u8 *ip)
>+{
>+ u8 val;
>+ get_user(val, ip);
>+ if (val == 0x1f)
>+ return do_1f(ip + 1);
>+ return 0;
>+}
>+
>+static inline int do_66(u8 *ip)
>+{
>+ u8 val;
>+ get_user(val, ip);
>+ if (val == 0x90)
>+ return 2;
>+ if (val == 0x0f) {
>+ int res = do_0f(ip + 1);
>+ if (res)
>+ return res + 1;
>+ else
>+ return 0;
>+ }
>+ return 0;
>+}
>+
>+static inline int do_start(u8 *ip)
>+{
>+ u8 val;
>+ get_user(val, ip);
>+ if (val == 0x0f)
>+ return do_0f(ip + 1);
>+ if (val == 0x66)
>+ return do_66(ip + 1);
>+ return 0;
>+}
>+
>+/* [do_nopl_emu] is called by exception 6 after an invalid opcode has been
>+ * encountered. It will try to emulate it by doing nothing,
>+ * and will send a SIGILL or SIGSEGV to the process if not possible.
>+ * the NOPL can have variable length opcodes:
>+
>+bytes number opcode
>+ 2 66 90
>+ 3 0f 1f 00
>+ 4 0f 1f 40 00
>+ 5 0f 1f 44 00 00
>+ 6 66 0f 1f 44 00 00
>+ 7 0f 1f 80 00 00 00 00
>+ 8 0f 1f 84 00 00 00 00 00
>+ 9 66 0f 1f 84 00 00 00 00 00
>+*/
>+void do_nopl_emu(struct pt_regs *regs, long error_code)
>+{
>+ u8 *ip = (u8 *)instruction_pointer(regs);
>+ int res = do_start(ip);
>+
>+ if (res) {
>+ int i = 0;
>+ do {
>+ ip += res;
>+ i++;
>+ res = do_start(ip);
>+ } while(res);
>+ printk(KERN_DEBUG "geode_nopl: emulated %d instructions\n", i);
>+ regs->ip = (typeof(regs->ip))ip;
>+ } else
>+ do_invalid_op(regs, error_code);
>+}
>
>
>--
>Matteo Croce
>OpenWrt developer
>  _______                     ________        __
> |       |.-----.-----.-----.|  |  |  |.----.|  |_
> |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
> |_______||   __|_____|__|__||________||__|  |____|
>          |__| W I R E L E S S   F R E E D O M
> KAMIKAZE (bleeding edge) ------------------
>  * 10 oz Vodka       Shake well with ice and strain
>  * 10 oz Triple sec  mixture into 10 shot glasses.
>  * 10 oz lime juice  Salute!
> ---------------------------------------------------
>

--
Sent from my mobile phone. Please pardon any lack of formatting.

2010-08-27 21:32:28

by Matteo Croce

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

can I ignore the return value when I expect val to be non zero?
the doc says: "On error, the variable @x is set to zero."

On Fri, Aug 27, 2010 at 10:49 PM, Thomas Backlund <[email protected]> wrote:
> 27.08.2010 23:15, Matteo Croce skrev:
>>
>> On Fri, Aug 27, 2010 at 8:48 PM, H. Peter Anvin<[email protected]>  wrote:
>>>
>>> You're doing user-space references without get_user().
>>>
>>>        -hpa
>>>
>>> --
>>> H. Peter Anvin, Intel Open Source Technology Center
>>> I work for Intel.  I don't speak on their behalf.
>>>
>>>
>>
>> Here with get_user.
>> I CC Natale which is my beta tester, he have some Alix boards running
>> 24/24
>> with my patch
>>
>> --- a/arch/x86/kernel/Makefile  2010-08-27 19:42:01.795858001 +0200
>> +++ b/arch/x86/kernel/Makefile  2010-08-27 19:42:12.525858001 +0200
>> @@ -88,6 +88,8 @@
>>  obj-$(CONFIG_APB_TIMER)               += apb_timer.o
>>
>>  obj-$(CONFIG_K8_NB)           += k8.o
>> +obj-$(CONFIG_GEODE_NOPL)       += nopl_emu.o
>> +obj-$(CONFIG_GEODE_NOPL)       += nopl_emu.o
>
> Same line added twice...
>
> --
> Thomas
>



--
Matteo Croce
OpenWrt developer
  _______                     ________        __
 |       |.-----.-----.-----.|  |  |  |.----.|  |_
 |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
 |_______||   __|_____|__|__||________||__|  |____|
          |__| W I R E L E S S   F R E E D O M
 KAMIKAZE (bleeding edge) ------------------
  * 10 oz Vodka       Shake well with ice and strain
  * 10 oz Triple sec  mixture into 10 shot glasses.
  * 10 oz lime juice  Salute!
 ---------------------------------------------------

2010-08-27 22:17:22

by Matteo Croce

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

On Fri, Aug 27, 2010 at 11:32 PM, Matteo Croce <[email protected]> wrote:
> can I ignore the return value when I expect val to be non zero?
> the doc says: "On error, the variable @x is set to zero."
>
> On Fri, Aug 27, 2010 at 10:49 PM, Thomas Backlund <[email protected]> wrote:
>> 27.08.2010 23:15, Matteo Croce skrev:
>>>
>>> On Fri, Aug 27, 2010 at 8:48 PM, H. Peter Anvin<[email protected]>  wrote:
>>>>
>>>> You're doing user-space references without get_user().
>>>>
>>>>        -hpa
>>>>
>>>> --
>>>> H. Peter Anvin, Intel Open Source Technology Center
>>>> I work for Intel.  I don't speak on their behalf.
>>>>
>>>>
>>>
>>> Here with get_user.
>>> I CC Natale which is my beta tester, he have some Alix boards running
>>> 24/24
>>> with my patch
>>>
>>> --- a/arch/x86/kernel/Makefile  2010-08-27 19:42:01.795858001 +0200
>>> +++ b/arch/x86/kernel/Makefile  2010-08-27 19:42:12.525858001 +0200
>>> @@ -88,6 +88,8 @@
>>>  obj-$(CONFIG_APB_TIMER)               += apb_timer.o
>>>
>>>  obj-$(CONFIG_K8_NB)           += k8.o
>>> +obj-$(CONFIG_GEODE_NOPL)       += nopl_emu.o
>>> +obj-$(CONFIG_GEODE_NOPL)       += nopl_emu.o
>>
>> Same line added twice...
>>
>> --
>> Thomas
>>
>
>
>
> --
> Matteo Croce
> OpenWrt developer
>   _______                     ________        __
>  |       |.-----.-----.-----.|  |  |  |.----.|  |_
>  |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
>  |_______||   __|_____|__|__||________||__|  |____|
>           |__| W I R E L E S S   F R E E D O M
>  KAMIKAZE (bleeding edge) ------------------
>   * 10 oz Vodka       Shake well with ice and strain
>   * 10 oz Triple sec  mixture into 10 shot glasses.
>   * 10 oz lime juice  Salute!
>  ---------------------------------------------------
>

Here is with proper checks

--- a/arch/x86/kernel/Makefile 2010-08-27 19:42:01.795858001 +0200
+++ b/arch/x86/kernel/Makefile 2010-08-27 19:42:12.525858001 +0200
@@ -88,6 +88,8 @@
obj-$(CONFIG_APB_TIMER) += apb_timer.o

obj-$(CONFIG_K8_NB) += k8.o
+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
obj-$(CONFIG_DEBUG_RODATA_TEST) += test_rodata.o
obj-$(CONFIG_DEBUG_NX_TEST) += test_nx.o

--- a/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:01.855858001 +0200
+++ b/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:12.535858001 +0200
@@ -137,11 +137,15 @@
return;
}

+#ifdef CONFIG_GEODE_NOPL
if (c->x86_model == 10) {
- /* AMD Geode LX is model 10 */
- /* placeholder for any needed mods */
+ /* Geode only lacks the NOPL instruction to be i686,
+ but we can promote it to a i686 class cpu
+ and emulate NOPLs in the exception handler*/
+ boot_cpu_data.x86 = 6;
return;
}
+#endif
}

static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c)
--- a/arch/x86/kernel/entry_32.S 2010-08-27 19:42:01.735858001 +0200
+++ b/arch/x86/kernel/entry_32.S 2010-08-27 19:42:12.535858001 +0200
@@ -978,7 +978,11 @@
RING0_INT_FRAME
pushl $0
CFI_ADJUST_CFA_OFFSET 4
+#ifdef CONFIG_GEODE_NOPL
+ pushl $do_nopl_emu
+#else
pushl $do_invalid_op
+#endif
CFI_ADJUST_CFA_OFFSET 4
jmp error_code
CFI_ENDPROC
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ b/arch/x86/kernel/nopl_emu.c 2010-08-28 00:11:52.627085002 +0200
@@ -0,0 +1,128 @@
+/*
+ * linux/arch/x86/kernel/nopl_emu.c
+ *
+ * Copyright (C) 2002 Willy Tarreau
+ * Copyright (C) 2010 Matteo Croce
+ */
+
+#include <asm/processor.h>
+#include <asm/traps.h>
+#include <asm/uaccess.h>
+
+/* This code can be used to allow the AMD Geode to hopefully correctly execute
+ * some code which was originally compiled for an i686, by emulating NOPL,
+ * the only missing i686 instruction in the CPU
+ *
+ * Willy Tarreau <[email protected]>
+ * Matteo Croce <[email protected]>
+ */
+
+static inline int do_1f(u8 *ip)
+{
+ u8 val1, val2;
+ int length = 3;
+ if (get_user(val1, ip))
+ return 0;
+ switch (val1) {
+ case 0x84:
+ get_user(val1, ip + 5);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x80:
+ get_user(val1, ip + 4);
+ get_user(val2, ip + 3);
+ if (!val1 && !val2)
+ length += 2;
+ else
+ return 0;
+ case 0x44:
+ get_user(val1, ip + 2);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x40:
+ get_user(val1, ip + 1);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x00:
+ return length;
+ }
+ return 0;
+}
+
+static inline int do_0f(u8 *ip)
+{
+ u8 val;
+ if (get_user(val, ip))
+ return 0;
+ if (val == 0x1f)
+ return do_1f(ip + 1);
+ return 0;
+}
+
+static inline int do_66(u8 *ip)
+{
+ u8 val;
+ if (get_user(val, ip))
+ return 0;
+ if (val == 0x90)
+ return 2;
+ if (val == 0x0f) {
+ int res = do_0f(ip + 1);
+ if (res)
+ return res + 1;
+ else
+ return 0;
+ }
+ return 0;
+}
+
+static inline int do_start(u8 *ip)
+{
+ u8 val;
+ if (get_user(val, ip))
+ return 0;
+ if (val == 0x0f)
+ return do_0f(ip + 1);
+ if (val == 0x66)
+ return do_66(ip + 1);
+ return 0;
+}
+
+/* [do_nopl_emu] is called by exception 6 after an invalid opcode has been
+ * encountered. It will try to emulate it by doing nothing,
+ * and will send a SIGILL or SIGSEGV to the process if not possible.
+ * the NOPL can have variable length opcodes:
+
+bytes number opcode
+ 2 66 90
+ 3 0f 1f 00
+ 4 0f 1f 40 00
+ 5 0f 1f 44 00 00
+ 6 66 0f 1f 44 00 00
+ 7 0f 1f 80 00 00 00 00
+ 8 0f 1f 84 00 00 00 00 00
+ 9 66 0f 1f 84 00 00 00 00 00
+*/
+void do_nopl_emu(struct pt_regs *regs, long error_code)
+{
+ u8 *ip = (u8 *)instruction_pointer(regs);
+ int res = do_start(ip);
+
+ if (res) {
+ int i = 0;
+ do {
+ ip += res;
+ i++;
+ res = do_start(ip);
+ } while(res);
+ printk(KERN_DEBUG "geode_nopl: emulated %d instructions\n", i);
+ regs->ip = (typeof(regs->ip))ip;
+ } else
+ do_invalid_op(regs, error_code);
+}


--
Matteo Croce
OpenWrt developer
  _______                     ________        __
 |       |.-----.-----.-----.|  |  |  |.----.|  |_
 |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
 |_______||   __|_____|__|__||________||__|  |____|
          |__| W I R E L E S S   F R E E D O M
 KAMIKAZE (bleeding edge) ------------------
  * 10 oz Vodka       Shake well with ice and strain
  * 10 oz Triple sec  mixture into 10 shot glasses.
  * 10 oz lime juice  Salute!
 ---------------------------------------------------

2010-08-27 22:19:28

by Matteo Croce

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

On Sat, Aug 28, 2010 at 12:16 AM, Matteo Croce <[email protected]> wrote:
> On Fri, Aug 27, 2010 at 11:32 PM, Matteo Croce <[email protected]> wrote:
>> can I ignore the return value when I expect val to be non zero?
>> the doc says: "On error, the variable @x is set to zero."
>>
>> On Fri, Aug 27, 2010 at 10:49 PM, Thomas Backlund <[email protected]> wrote:
>>> 27.08.2010 23:15, Matteo Croce skrev:
>>>>
>>>> On Fri, Aug 27, 2010 at 8:48 PM, H. Peter Anvin<[email protected]>  wrote:
>>>>>
>>>>> You're doing user-space references without get_user().
>>>>>
>>>>>        -hpa
>>>>>
>>>>> --
>>>>> H. Peter Anvin, Intel Open Source Technology Center
>>>>> I work for Intel.  I don't speak on their behalf.
>>>>>
>>>>>
>>>>
>>>> Here with get_user.
>>>> I CC Natale which is my beta tester, he have some Alix boards running
>>>> 24/24
>>>> with my patch
>>>>
>>>> --- a/arch/x86/kernel/Makefile  2010-08-27 19:42:01.795858001 +0200
>>>> +++ b/arch/x86/kernel/Makefile  2010-08-27 19:42:12.525858001 +0200
>>>> @@ -88,6 +88,8 @@
>>>>  obj-$(CONFIG_APB_TIMER)               += apb_timer.o
>>>>
>>>>  obj-$(CONFIG_K8_NB)           += k8.o
>>>> +obj-$(CONFIG_GEODE_NOPL)       += nopl_emu.o
>>>> +obj-$(CONFIG_GEODE_NOPL)       += nopl_emu.o
>>>
>>> Same line added twice...
>>>
>>> --
>>> Thomas
>>>
>>
>>
>>
>> --
>> Matteo Croce
>> OpenWrt developer
>>   _______                     ________        __
>>  |       |.-----.-----.-----.|  |  |  |.----.|  |_
>>  |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
>>  |_______||   __|_____|__|__||________||__|  |____|
>>           |__| W I R E L E S S   F R E E D O M
>>  KAMIKAZE (bleeding edge) ------------------
>>   * 10 oz Vodka       Shake well with ice and strain
>>   * 10 oz Triple sec  mixture into 10 shot glasses.
>>   * 10 oz lime juice  Salute!
>>  ---------------------------------------------------
>>
>
> Here is with proper checks
>
> --- a/arch/x86/kernel/Makefile  2010-08-27 19:42:01.795858001 +0200
> +++ b/arch/x86/kernel/Makefile  2010-08-27 19:42:12.525858001 +0200
> @@ -88,6 +88,8 @@
>  obj-$(CONFIG_APB_TIMER)                += apb_timer.o
>
>  obj-$(CONFIG_K8_NB)            += k8.o
> +obj-$(CONFIG_GEODE_NOPL)       += nopl_emu.o
> +obj-$(CONFIG_GEODE_NOPL)       += nopl_emu.o
>  obj-$(CONFIG_DEBUG_RODATA_TEST)        += test_rodata.o
>  obj-$(CONFIG_DEBUG_NX_TEST)    += test_nx.o
>
> --- a/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:01.855858001 +0200
> +++ b/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:12.535858001 +0200
> @@ -137,11 +137,15 @@
>                return;
>        }
>
> +#ifdef CONFIG_GEODE_NOPL
>        if (c->x86_model == 10) {
> -               /* AMD Geode LX is model 10 */
> -               /* placeholder for any needed mods */
> +               /* Geode only lacks the NOPL instruction to be i686,
> +                  but we can promote it to a i686 class cpu
> +                  and emulate NOPLs in the exception handler*/
> +               boot_cpu_data.x86 = 6;
>                return;
>        }
> +#endif
>  }
>
>  static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c)
> --- a/arch/x86/kernel/entry_32.S        2010-08-27 19:42:01.735858001 +0200
> +++ b/arch/x86/kernel/entry_32.S        2010-08-27 19:42:12.535858001 +0200
> @@ -978,7 +978,11 @@
>        RING0_INT_FRAME
>        pushl $0
>        CFI_ADJUST_CFA_OFFSET 4
> +#ifdef CONFIG_GEODE_NOPL
> +       pushl $do_nopl_emu
> +#else
>        pushl $do_invalid_op
> +#endif
>        CFI_ADJUST_CFA_OFFSET 4
>        jmp error_code
>        CFI_ENDPROC
> --- /dev/null   1970-01-01 00:00:00.000000000 +0000
> +++ b/arch/x86/kernel/nopl_emu.c        2010-08-28 00:11:52.627085002 +0200
> @@ -0,0 +1,128 @@
> +/*
> + *  linux/arch/x86/kernel/nopl_emu.c
> + *
> + *  Copyright (C) 2002  Willy Tarreau
> + *  Copyright (C) 2010  Matteo Croce
> + */
> +
> +#include <asm/processor.h>
> +#include <asm/traps.h>
> +#include <asm/uaccess.h>
> +
> +/* This code can be used to allow the AMD Geode to hopefully correctly execute
> + * some code which was originally compiled for an i686, by emulating NOPL,
> + * the only missing i686 instruction in the CPU
> + *
> + * Willy Tarreau <[email protected]>
> + * Matteo Croce <[email protected]>
> + */
> +
> +static inline int do_1f(u8 *ip)
> +{
> +       u8 val1, val2;
> +       int length = 3;
> +       if (get_user(val1, ip))
> +               return 0;
> +       switch (val1) {
> +       case 0x84:
> +               get_user(val1, ip + 5);
> +               if (!val1)
> +                       length++;
> +               else
> +                       return 0;
> +       case 0x80:
> +               get_user(val1, ip + 4);
> +               get_user(val2, ip + 3);
> +               if (!val1 && !val2)
> +                       length += 2;
> +               else
> +                       return 0;
> +       case 0x44:
> +               get_user(val1, ip + 2);
> +               if (!val1)
> +                       length++;
> +               else
> +                       return 0;
> +       case 0x40:
> +               get_user(val1, ip + 1);
> +               if (!val1)
> +                       length++;
> +               else
> +                       return 0;
> +       case 0x00:
> +               return length;
> +       }
> +       return 0;
> +}
> +
> +static inline int do_0f(u8 *ip)
> +{
> +       u8 val;
> +       if (get_user(val, ip))
> +               return 0;
> +       if (val == 0x1f)
> +               return do_1f(ip + 1);
> +       return 0;
> +}
> +
> +static inline int do_66(u8 *ip)
> +{
> +       u8 val;
> +       if (get_user(val, ip))
> +               return 0;
> +       if (val == 0x90)
> +               return 2;
> +       if (val == 0x0f) {
> +               int res = do_0f(ip + 1);
> +               if (res)
> +                       return res + 1;
> +               else
> +                       return 0;
> +       }
> +       return 0;
> +}
> +
> +static inline int do_start(u8 *ip)
> +{
> +       u8 val;
> +       if (get_user(val, ip))
> +               return 0;
> +       if (val == 0x0f)
> +               return do_0f(ip + 1);
> +       if (val == 0x66)
> +               return do_66(ip + 1);
> +       return 0;
> +}
> +
> +/* [do_nopl_emu] is called by exception 6 after an invalid opcode has been
> + * encountered. It will try to emulate it by doing nothing,
> + * and will send a SIGILL or SIGSEGV to the process if not possible.
> + * the NOPL can have variable length opcodes:
> +
> +bytes number   opcode
> +       2       66 90
> +       3       0f 1f 00
> +       4       0f 1f 40 00
> +       5       0f 1f 44 00 00
> +       6       66 0f 1f 44 00 00
> +       7       0f 1f 80 00 00 00 00
> +       8       0f 1f 84 00 00 00 00 00
> +       9       66 0f 1f 84 00 00 00 00 00
> +*/
> +void do_nopl_emu(struct pt_regs *regs, long error_code)
> +{
> +       u8 *ip = (u8 *)instruction_pointer(regs);
> +       int res = do_start(ip);
> +
> +       if (res) {
> +               int i = 0;
> +               do {
> +                       ip += res;
> +                       i++;
> +                       res = do_start(ip);
> +               } while(res);
> +               printk(KERN_DEBUG "geode_nopl: emulated %d instructions\n", i);
> +               regs->ip = (typeof(regs->ip))ip;
> +       } else
> +               do_invalid_op(regs, error_code);
> +}
>
>
> --
> Matteo Croce
> OpenWrt developer
>   _______                     ________        __
>  |       |.-----.-----.-----.|  |  |  |.----.|  |_
>  |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
>  |_______||   __|_____|__|__||________||__|  |____|
>           |__| W I R E L E S S   F R E E D O M
>  KAMIKAZE (bleeding edge) ------------------
>   * 10 oz Vodka       Shake well with ice and strain
>   * 10 oz Triple sec  mixture into 10 shot glasses.
>   * 10 oz lime juice  Salute!
>  ---------------------------------------------------
>

Sorry still left the duplicated line

--- a/arch/x86/kernel/Makefile 2010-08-27 19:42:01.795858001 +0200
+++ b/arch/x86/kernel/Makefile 2010-08-28 00:16:53.607507000 +0200
@@ -88,6 +88,7 @@
obj-$(CONFIG_APB_TIMER) += apb_timer.o

obj-$(CONFIG_K8_NB) += k8.o
+obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
obj-$(CONFIG_DEBUG_RODATA_TEST) += test_rodata.o
obj-$(CONFIG_DEBUG_NX_TEST) += test_nx.o

--- a/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:01.855858001 +0200
+++ b/arch/x86/kernel/cpu/amd.c 2010-08-27 19:42:12.535858001 +0200
@@ -137,11 +137,15 @@
return;
}

+#ifdef CONFIG_GEODE_NOPL
if (c->x86_model == 10) {
- /* AMD Geode LX is model 10 */
- /* placeholder for any needed mods */
+ /* Geode only lacks the NOPL instruction to be i686,
+ but we can promote it to a i686 class cpu
+ and emulate NOPLs in the exception handler*/
+ boot_cpu_data.x86 = 6;
return;
}
+#endif
}

static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c)
--- a/arch/x86/kernel/entry_32.S 2010-08-27 19:42:01.735858001 +0200
+++ b/arch/x86/kernel/entry_32.S 2010-08-27 19:42:12.535858001 +0200
@@ -978,7 +978,11 @@
RING0_INT_FRAME
pushl $0
CFI_ADJUST_CFA_OFFSET 4
+#ifdef CONFIG_GEODE_NOPL
+ pushl $do_nopl_emu
+#else
pushl $do_invalid_op
+#endif
CFI_ADJUST_CFA_OFFSET 4
jmp error_code
CFI_ENDPROC
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ b/arch/x86/kernel/nopl_emu.c 2010-08-28 00:11:52.627085002 +0200
@@ -0,0 +1,128 @@
+/*
+ * linux/arch/x86/kernel/nopl_emu.c
+ *
+ * Copyright (C) 2002 Willy Tarreau
+ * Copyright (C) 2010 Matteo Croce
+ */
+
+#include <asm/processor.h>
+#include <asm/traps.h>
+#include <asm/uaccess.h>
+
+/* This code can be used to allow the AMD Geode to hopefully correctly execute
+ * some code which was originally compiled for an i686, by emulating NOPL,
+ * the only missing i686 instruction in the CPU
+ *
+ * Willy Tarreau <[email protected]>
+ * Matteo Croce <[email protected]>
+ */
+
+static inline int do_1f(u8 *ip)
+{
+ u8 val1, val2;
+ int length = 3;
+ if (get_user(val1, ip))
+ return 0;
+ switch (val1) {
+ case 0x84:
+ get_user(val1, ip + 5);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x80:
+ get_user(val1, ip + 4);
+ get_user(val2, ip + 3);
+ if (!val1 && !val2)
+ length += 2;
+ else
+ return 0;
+ case 0x44:
+ get_user(val1, ip + 2);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x40:
+ get_user(val1, ip + 1);
+ if (!val1)
+ length++;
+ else
+ return 0;
+ case 0x00:
+ return length;
+ }
+ return 0;
+}
+
+static inline int do_0f(u8 *ip)
+{
+ u8 val;
+ if (get_user(val, ip))
+ return 0;
+ if (val == 0x1f)
+ return do_1f(ip + 1);
+ return 0;
+}
+
+static inline int do_66(u8 *ip)
+{
+ u8 val;
+ if (get_user(val, ip))
+ return 0;
+ if (val == 0x90)
+ return 2;
+ if (val == 0x0f) {
+ int res = do_0f(ip + 1);
+ if (res)
+ return res + 1;
+ else
+ return 0;
+ }
+ return 0;
+}
+
+static inline int do_start(u8 *ip)
+{
+ u8 val;
+ if (get_user(val, ip))
+ return 0;
+ if (val == 0x0f)
+ return do_0f(ip + 1);
+ if (val == 0x66)
+ return do_66(ip + 1);
+ return 0;
+}
+
+/* [do_nopl_emu] is called by exception 6 after an invalid opcode has been
+ * encountered. It will try to emulate it by doing nothing,
+ * and will send a SIGILL or SIGSEGV to the process if not possible.
+ * the NOPL can have variable length opcodes:
+
+bytes number opcode
+ 2 66 90
+ 3 0f 1f 00
+ 4 0f 1f 40 00
+ 5 0f 1f 44 00 00
+ 6 66 0f 1f 44 00 00
+ 7 0f 1f 80 00 00 00 00
+ 8 0f 1f 84 00 00 00 00 00
+ 9 66 0f 1f 84 00 00 00 00 00
+*/
+void do_nopl_emu(struct pt_regs *regs, long error_code)
+{
+ u8 *ip = (u8 *)instruction_pointer(regs);
+ int res = do_start(ip);
+
+ if (res) {
+ int i = 0;
+ do {
+ ip += res;
+ i++;
+ res = do_start(ip);
+ } while(res);
+ printk(KERN_DEBUG "geode_nopl: emulated %d instructions\n", i);
+ regs->ip = (typeof(regs->ip))ip;
+ } else
+ do_invalid_op(regs, error_code);
+}


--
Matteo Croce
OpenWrt developer
  _______                     ________        __
 |       |.-----.-----.-----.|  |  |  |.----.|  |_
 |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
 |_______||   __|_____|__|__||________||__|  |____|
          |__| W I R E L E S S   F R E E D O M
 KAMIKAZE (bleeding edge) ------------------
  * 10 oz Vodka       Shake well with ice and strain
  * 10 oz Triple sec  mixture into 10 shot glasses.
  * 10 oz lime juice  Salute!
 ---------------------------------------------------

2010-08-27 23:08:05

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

On 08/27/2010 02:32 PM, Matteo Croce wrote:
> can I ignore the return value when I expect val to be non zero?
> the doc says: "On error, the variable @x is set to zero."

No. You need to deliver a page fault to the application in this case.

The *real* test for this kind of crap is correct page fault behavior,
and so forth.

Also, at the very least you need to check for:

- CS == USER_CS
- IP in the proper range for user space

Your patch in its current form is one big security hole.

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2010-08-29 12:53:06

by Avi Kivity

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

On 08/28/2010 02:07 AM, H. Peter Anvin wrote:
> On 08/27/2010 02:32 PM, Matteo Croce wrote:
>> can I ignore the return value when I expect val to be non zero?
>> the doc says: "On error, the variable @x is set to zero."
> No. You need to deliver a page fault to the application in this case.
>
> The *real* test for this kind of crap is correct page fault behavior,
> and so forth.
>
> Also, at the very least you need to check for:
>
> - CS == USER_CS

Is that mandated by the ABI? Is it illegal for an application to load a
segment with a nonzero base into the LDT and use it for CS?

What about vm86?

--
error compiling committee.c: too many arguments to function

2010-08-29 13:39:31

by Matteo Croce

[permalink] [raw]
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2

On Sat, Aug 28, 2010 at 1:07 AM, H. Peter Anvin <[email protected]> wrote:
> On 08/27/2010 02:32 PM, Matteo Croce wrote:
>> can I ignore the return value when I expect val to be non zero?
>> the doc says: "On error, the variable @x is set to zero."
>
> No.  You need to deliver a page fault to the application in this case.
>
> The *real* test for this kind of crap is correct page fault behavior,
> and so forth.
>
> Also, at the very least you need to check for:
>
> - CS == USER_CS
> - IP in the proper range for user space
>
> Your patch in its current form is one big security hole.
>
>        -hpa
>
> --
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.
>
>

If the parsing fails due get_user returning error I call
`do_invalid_op(regs, error_code);`
which is the default handler, which does the page fault.
to check the CS I do `regs->cs != __USER_CS` but how to check the IP value?
convert_ip_to_linear() and then check something?

--
Matteo Croce
OpenWrt developer
  _______                     ________        __
 |       |.-----.-----.-----.|  |  |  |.----.|  |_
 |   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
 |_______||   __|_____|__|__||________||__|  |____|
          |__| W I R E L E S S   F R E E D O M
 KAMIKAZE (bleeding edge) ------------------
  * 10 oz Vodka       Shake well with ice and strain
  * 10 oz Triple sec  mixture into 10 shot glasses.
  * 10 oz lime juice  Salute!
 ---------------------------------------------------