Messages in this thread | | | Date | Wed, 8 Sep 2010 12:34:12 +0100 | Subject | Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2 | From | Nick Lowe <> |
| |
Hi,
If a process crashes because of NOPL usage, you get the feedback loop to fix it.
The beauty of open source is that we can recompile to correct the bad assumption; the latest version of binutils (GAS) finally corrects the bad semantic that generic i686 includes NOPL.
http://sourceware.org/ml/binutils-cvs/2010-08/msg00057.html http://gcc.gnu.org/ml/gcc/2010-08/msg00194.html
That fixes the vast vast majority of cases, so I really don't see the need for this.
I've posted a RFC on "Promoting Crusoe and Geode Processors to i686 Status" if you have any thoughts on that?
Cheers,
Nick
On Wed, Sep 8, 2010 at 10:15 AM, Hans-Peter Jansen <hpj@urpla.net> wrote: > On Tuesday 07 September 2010, 17:57:17 Nick Lowe wrote: >> In my opinion, this patch shouldn't be seen as a solution in the >> slightest. To me, it's a ugly hack and a kludge at best! >> >> Abstractly a NOP is meant to be exactly as it says on the tin, a No >> Operation. >> >> It's meant to do nothing at all - for a predefined number of clock >> cycles. A NOP is commonly used for timing purposes, this completely >> breaks that contract surely? >> >> NOPL is not standard i686, it was undocumented and has just been de facto >> supported since the Pentium Pro. >> >> The solution is surely to ensure that when something is compiled for >> generic i686, NOPL is nowhere to be seen... Any compiler that does >> otherwise is broken. > > From a user POV, this patch done right is the better solution, because you > cannot ensure, that all code _generally_ comply to your *policy*. This is > not a kernel only issue! Keep processes crashing is worse IMHO. > > With properly done, I mean: > - runtime check, if iopl opcode exists > - emulate, if not > > Hence, don't bind this to nor label it after a certain cpu model. > > Remember: we do not live in a perfect world. > > Pete > >> For example, until recently, the basic semantics for choosing the NOP >> sequence were completely wrong in binutils. This has, finally, been >> fixed! >> >> http://sourceware.org/ml/binutils-cvs/2010-08/msg00057.html >> http://gcc.gnu.org/ml/gcc/2010-08/msg00194.html >> >> On 27 Aug 2010, at 19:10, Matteo Croce wrote: >> > 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 <matteo@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 <willy@meta-x.org> >> > + * Matteo Croce <technoboy85@gmail.co> >> > + */ >> > + >> > +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 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/ >> >> -- >> 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/ > > > -- 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/
| |