Messages in this thread | | | From | "Hans-Peter Jansen" <> | Subject | Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2 | Date | Wed, 8 Sep 2010 11:15:15 +0200 |
| |
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/
| |