AMD Geode NOPL emulation for kernel 2.6.36-rc2
Matteo Croce
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 <mat...@openwrt.org>
--- 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 <wi...@meta-x.org>
+ * Matteo Croce <techn...@gmail.com>
+ */
+
+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!
---------------------------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
H. Peter Anvin
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
Matteo Croce
> 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 <wi...@meta-x.org>
+ * Matteo Croce <techn...@gmail.com>
+ */
+
+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)
H. Peter Anvin
"Matteo Croce" <mat...@openwrt.org> wrote:
--
Sent from my mobile phone. Please pardon any lack of formatting.
Thomas Backlund
> On Fri, Aug 27, 2010 at 8:48 PM, H. Peter Anvin<h...@zytor.com> 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
the doc says: "On error, the variable @x is set to zero."
On Fri, Aug 27, 2010 at 10:49 PM, Thomas Backlund <t...@mandriva.org> wrote:
> 27.08.2010 23:15, Matteo Croce skrev:
>>
>> On Fri, Aug 27, 2010 at 8:48 PM, H. Peter Anvin<h...@zytor.com> 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!
---------------------------------------------------
Matteo Croce
>
> --- 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
> @@ -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 <wi...@meta-x.org>
> + * Matteo Croce <techn...@gmail.com>
> + */
> +
> +static inline int do_1f(u8 *ip)
> +{
> + u8 val1, val2;
> + int length = 3;
> + return 0;
> + return 0;
> + if (val == 0x1f)
> + return do_1f(ip + 1);
> + return 0;
> +}
> +
> +static inline int do_66(u8 *ip)
> +{
> + u8 val;
> + 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;
> + return 0;
> 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
+++ 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 <wi...@meta-x.org>
+ * Matteo Croce <techn...@gmail.com>
+ */
+
+static inline int do_1f(u8 *ip)
+{
+ u8 val1, val2;
+ int length = 3;
+ if (get_user(val1, ip))
+ return 0;
+ 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;
Matteo Croce
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
+++ 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 <wi...@meta-x.org>
+ * Matteo Croce <techn...@gmail.com>
+ */
+
+static inline int do_1f(u8 *ip)
+{
+ u8 val1, val2;
+ int length = 3;
+ if (get_user(val1, ip))
+ return 0;
+ 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;
H. Peter Anvin
> 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.
--
Avi Kivity
> 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
Matteo Croce
> 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!
---------------------------------------------------
H. Peter Anvin
> 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.
No, it doesn't. It does an SIGILL, not a SIGSEGV. An application which
does its own VM management depends on the difference.
Also, you only test for specific forms of NOPL, whereas the right thing
is to recognize the overall forms, not just byte sequences.
> 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?
get_user() will check for the validity of a linear address, and yes,
convert_ip_to_linear() should give you the linear address to check for.
However, you also have to check for the CPU mode, since the byte
sequences mean different things in 16-, 32- and 64-bit mode.
All of this is why I'm extremely reluctant to allow in an ad hoc hack
like this one ... there just are way too many pitfalls, any of which can
turn into a security hole.
-hpa