Messages in this thread Patch in this message | | | Date | Sat, 27 Nov 1999 16:23:38 +0100 (CET) | From | Ingo Molnar <> | Subject | [patch] string.h speedup, cld-2.3.30-A1 |
| |
x86 GCC and string.h generates lots of 'cld' instructions for string / memory copies. My 2.3.30-pre2 kernel contains 973 cld instructions, often in hot paths. Most of these cld's are unnecessery, and just add a 6 cycle overhead and pollute the icache. This is a problem which has been long known, but not addressed due to various subtle dangers related to the change.
'cld' is used so frequently on x86 CPUs because untrusted code is free to do a 'std' to change the direction of memory/string copies. This flag is subtle because it only affects string/memory copies, and can lead to security problems (and has lead to at least one exploit in the distant past) which go unnoticed during normal use. (normal code doesnt set std explicitly before calling the kernel)
but all Linux entry and 'untrusted exit' points already do a 'cld', so the change would be safe, conceptually.
To be 100% safe, i tried to find an 'indirect' way to ensure that no entry/exit point go unnoticed, and the key is segment registers. Whenever we change to another protection domain, we must expect '%ds' and '%es' to get changed by untrusted code. So the 'cld change' can be validated by checking all places that store/restore %ds. If any such place is missed then it's unsafe already, because untrusted code can change %ds and trick the kernel into either crashing or writing to the wrong area.
x86 places that save/restore %ds are easy to find (rgrep -rl '%ds' .):
./include/asm-i386/hw_irq.h ./drivers/char/nvram.c ./drivers/scsi/in2000.h ./arch/i386/boot/bootsect.S ./arch/i386/boot/setup.S ./arch/i386/boot/compressed/head.S ./arch/i386/boot/video.S ./arch/i386/kernel/entry.S ./arch/i386/kernel/head.S ./arch/i386/kernel/process.c ./arch/i386/kernel/trampoline.S ./arch/i386/kernel/apm.c ./arch/i386/kernel/pci-pc.c ./arch/i386/kernel/apm.c.orig
a few of these are trivially not to be protected (boot code does a final cld, reboot code never returns), the remaining ones are:
./include/asm-i386/hw_irq.h ./arch/i386/kernel/entry.S ./arch/i386/kernel/apm.c ./arch/i386/kernel/pci-pc.c
pci-pc.c is the part dealing with PCI BIOS callbacks, apm.c is calling the APM BIOS, entry.S has our main entry points (syscalls, traps, faults). hw_irq.h has the macros for IRQ entry points. All these entry/exit points are sufficiently protected by a 'cld' after applying my patch. pci-pc.c had to be fixed to do a cld after calling the BIOS.
string.h's only function which does an 'std' (memmove) has to have a matching 'cld'.
future exit/entry points will have to do a 'cld' when returning from untrusted domains, just like they have to restore segment registers.
a the same kernel after applying the attached patch contains 284 'cld's and is 4k smaller. (the 4k savings are not an error, they are due to alignment gains, the 600 clds themselves caused a 4k kernel code bloat)
much of the remaining 284 cld's is still unjustified, because '*a = *b;' type of structure copies are inlined by GCC. (GCC generates a cld because user-space has to be prepared to be interrupted by uncooperative signal handlers and the like). So i added a cpy(x,y) macro which copies one structure into another via memcpy. We may not want to do it this way though, if it's too ugly. I have not found any way to prevent GCC from using it's own memcpy function in the structure copy case.
the patch works just fine here, and i'm reasonably sure that no entry/exit point is missing. (i ran testcode for some time which does 'std' and 'cld' and thus tests the IRQ handler entry code and the syscall entry code. No problems whatsoever, as expected.)
-- mingo --- linux/include/asm-i386/string.h.orig Sat Nov 27 05:44:12 1999 +++ linux/include/asm-i386/string.h Sat Nov 27 06:26:17 1999 @@ -32,7 +32,6 @@ { int d0, d1, d2; __asm__ __volatile__( - "cld\n" "1:\tlodsb\n\t" "stosb\n\t" "testb %%al,%%al\n\t" @@ -47,7 +46,6 @@ { int d0, d1, d2, d3; __asm__ __volatile__( - "cld\n" "1:\tdecl %2\n\t" "js 2f\n\t" "lodsb\n\t" @@ -67,7 +65,6 @@ { int d0, d1, d2, d3; __asm__ __volatile__( - "cld\n\t" "repne\n\t" "scasb\n\t" "decl %1\n" @@ -85,7 +82,6 @@ { int d0, d1, d2, d3; __asm__ __volatile__( - "cld\n\t" "repne\n\t" "scasb\n\t" "decl %1\n\t" @@ -110,7 +106,6 @@ int d0, d1; register int __res; __asm__ __volatile__( - "cld\n" "1:\tlodsb\n\t" "scasb\n\t" "jne 2f\n\t" @@ -132,7 +127,6 @@ register int __res; int d0, d1, d2; __asm__ __volatile__( - "cld\n" "1:\tdecl %3\n\t" "js 2f\n\t" "lodsb\n\t" @@ -156,7 +150,6 @@ int d0; register char * __res; __asm__ __volatile__( - "cld\n\t" "movb %%al,%%ah\n" "1:\tlodsb\n\t" "cmpb %%ah,%%al\n\t" @@ -176,7 +169,6 @@ int d0, d1; register char * __res; __asm__ __volatile__( - "cld\n\t" "movb %%al,%%ah\n" "1:\tlodsb\n\t" "cmpb %%ah,%%al\n\t" @@ -194,7 +186,6 @@ int d0; register int __res; __asm__ __volatile__( - "cld\n\t" "repne\n\t" "scasb\n\t" "notl %0\n\t" @@ -207,7 +198,6 @@ { int d0, d1, d2; __asm__ __volatile__( - "cld\n\t" "rep ; movsl\n\t" "testb $2,%b4\n\t" "je 1f\n\t" @@ -273,7 +263,6 @@ } #define COMMON(x) \ __asm__ __volatile__( \ - "cld\n\t" \ "rep ; movsl" \ x \ : "=&c" (d0), "=&D" (d1), "=&S" (d2) \ @@ -343,13 +332,14 @@ #endif +#define cpy(x,y) memcpy(x, y, sizeof(*(x))) + #define __HAVE_ARCH_MEMMOVE extern inline void * memmove(void * dest,const void * src, size_t n) { int d0, d1, d2; if (dest<src) __asm__ __volatile__( - "cld\n\t" "rep\n\t" "movsb" : "=&c" (d0), "=&S" (d1), "=&D" (d2) @@ -379,7 +369,6 @@ if (!count) return NULL; __asm__ __volatile__( - "cld\n\t" "repne\n\t" "scasb\n\t" "je 1f\n\t" @@ -393,7 +382,6 @@ { int d0, d1; __asm__ __volatile__( - "cld\n\t" "rep\n\t" "stosb" : "=&c" (d0), "=&D" (d1) @@ -414,7 +402,6 @@ { int d0, d1; __asm__ __volatile__( - "cld\n\t" "rep ; stosl\n\t" "testb $2,%b3\n\t" "je 1f\n\t" @@ -475,7 +462,7 @@ return s; } #define COMMON(x) \ -__asm__ __volatile__("cld\n\t" \ +__asm__ __volatile__( \ "rep ; stosl" \ x \ : "=&c" (d0), "=&D" (d1) \ @@ -518,8 +505,7 @@ { if (!size) return addr; - __asm__("cld - repnz; scasb + __asm__("repnz; scasb jnz 1f dec %%edi 1: " --- linux/include/asm-i386/string-486.h.orig Sat Nov 27 05:45:53 1999 +++ linux/include/asm-i386/string-486.h Sat Nov 27 05:58:05 1999 @@ -187,7 +187,6 @@ int d0, d1; register char * __res; __asm__ __volatile__( - "cld\n\t" "movb %%al,%%ah\n" "1:\tlodsb\n\t" "cmpb %%ah,%%al\n\t" @@ -206,7 +205,6 @@ int d0, d1; register char * __res; __asm__ __volatile__( - "cld\n\t" "movl %6,%%edi\n\t" "repne\n\t" "scasb\n\t" @@ -234,7 +232,6 @@ int d0, d1; register char * __res; __asm__ __volatile__( - "cld\n\t" "movl %6,%%edi\n\t" "repne\n\t" "scasb\n\t" @@ -263,7 +260,6 @@ int d0, d1; register char * __res; __asm__ __volatile__( - "cld\n\t" "movl %6,%%edi\n\t" "repne\n\t" "scasb\n\t" @@ -296,7 +292,6 @@ int d0, d1; register char * __res; __asm__ __volatile__( - "cld\n\t" \ "movl %6,%%edi\n\t" "repne\n\t" "scasb\n\t" @@ -378,7 +373,6 @@ "1:\txorl %0,%0\n\t" "movl $-1,%%ecx\n\t" "xorl %%eax,%%eax\n\t" - "cld\n\t" "movl %4,%%edi\n\t" "repne\n\t" "scasb\n\t" @@ -474,7 +468,6 @@ int d0, d1, d2; register void *tmp = (void *)to; __asm__ __volatile__ ( - "cld\n\t" "shrl $1,%%ecx\n\t" "jnc 1f\n\t" "movsb\n" @@ -554,7 +547,6 @@ register void *tmp = (void *)dest; if (dest<src) __asm__ __volatile__ ( - "cld\n\t" "rep\n\t" "movsb" :"=&c" (d0), "=&S" (d1), "=&D" (d2) @@ -577,7 +569,6 @@ int d0, d1, d2; register int __res; __asm__ __volatile__( - "cld\n\t" "repe\n\t" "cmpsb\n\t" "je 1f\n\t" @@ -597,7 +588,6 @@ if (!count) return NULL; __asm__ __volatile__( - "cld\n\t" "repne\n\t" "scasb\n\t" "je 1f\n\t" @@ -753,8 +743,7 @@ { if (!size) return addr; - __asm__("cld - repnz; scasb + __asm__("repnz; scasb jnz 1f dec %%edi 1: " --- linux/include/asm-i386/io.h.orig Sat Nov 27 05:46:43 1999 +++ linux/include/asm-i386/io.h Sat Nov 27 06:26:27 1999 @@ -71,12 +71,12 @@ #define __INS(s) \ extern inline void ins##s(unsigned short port, void * addr, unsigned long count) \ -{ __asm__ __volatile__ ("cld ; rep ; ins" #s \ +{ __asm__ __volatile__ ("rep ; ins" #s \ : "=D" (addr), "=c" (count) : "d" (port),"0" (addr),"1" (count)); } #define __OUTS(s) \ extern inline void outs##s(unsigned short port, const void * addr, unsigned long count) \ -{ __asm__ __volatile__ ("cld ; rep ; outs" #s \ +{ __asm__ __volatile__ ("rep ; outs" #s \ : "=S" (addr), "=c" (count) : "d" (port),"0" (addr),"1" (count)); } #define RETURN_TYPE unsigned char --- linux/include/asm-i386/bitops.h.orig Sat Nov 27 05:46:55 1999 +++ linux/include/asm-i386/bitops.h Sat Nov 27 05:47:09 1999 @@ -132,8 +132,7 @@ if (!size) return 0; - __asm__("cld\n\t" - "movl $-1,%%eax\n\t" + __asm__("movl $-1,%%eax\n\t" "xorl %%edx,%%edx\n\t" "repe; scasl\n\t" "je 1f\n\t" --- linux/arch/i386/kernel/apm.c.orig Sat Nov 27 05:47:49 1999 +++ linux/arch/i386/kernel/apm.c Sat Nov 27 05:52:58 1999 @@ -380,7 +380,7 @@ __asm__ __volatile__(APM_DO_ZERO_SEGS "pushl %%edi\n\t" "pushl %%ebp\n\t" - "lcall %%cs:" SYMBOL_NAME_STR(apm_bios_entry) "\n\t" + "lcall %%cs:" SYMBOL_NAME_STR(apm_bios_entry) "; cld\n\t" "setc %%al\n\t" "popl %%ebp\n\t" "popl %%edi\n\t" @@ -413,7 +413,7 @@ __asm__ __volatile__(APM_DO_ZERO_SEGS "pushl %%edi\n\t" "pushl %%ebp\n\t" - "lcall %%cs:" SYMBOL_NAME_STR(apm_bios_entry) "\n\t" + "lcall %%cs:" SYMBOL_NAME_STR(apm_bios_entry)"cld;\n\t" "setc %%bl\n\t" "popl %%ebp\n\t" "popl %%edi\n\t" --- linux/arch/i386/kernel/pci-pc.c.orig Sat Nov 27 05:49:53 1999 +++ linux/arch/i386/kernel/pci-pc.c Sat Nov 27 05:51:44 1999 @@ -342,7 +342,7 @@ unsigned long flags; __save_flags(flags); __cli(); - __asm__("lcall (%%edi)" + __asm__("lcall (%%edi); cld" : "=a" (return_code), "=b" (address), "=c" (length), @@ -383,7 +383,7 @@ __save_flags(flags); __cli(); __asm__( - "lcall (%%edi)\n\t" + "lcall (%%edi); cld\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" "1:" @@ -427,7 +427,7 @@ unsigned short bx; unsigned short ret; - __asm__("lcall (%%edi)\n\t" + __asm__("lcall (%%edi); cld\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" "1:" @@ -448,7 +448,7 @@ unsigned long ret; unsigned long bx = (dev->bus->number << 8) | dev->devfn; - __asm__("lcall (%%esi)\n\t" + __asm__("lcall (%%esi); cld\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" "1:" @@ -466,7 +466,7 @@ unsigned long ret; unsigned long bx = (dev->bus->number << 8) | dev->devfn; - __asm__("lcall (%%esi)\n\t" + __asm__("lcall (%%esi); cld\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" "1:" @@ -484,7 +484,7 @@ unsigned long ret; unsigned long bx = (dev->bus->number << 8) | dev->devfn; - __asm__("lcall (%%esi)\n\t" + __asm__("lcall (%%esi); cld\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" "1:" @@ -502,7 +502,7 @@ unsigned long ret; unsigned long bx = (dev->bus->number << 8) | dev->devfn; - __asm__("lcall (%%esi)\n\t" + __asm__("lcall (%%esi); cld\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" "1:" @@ -520,7 +520,7 @@ unsigned long ret; unsigned long bx = (dev->bus->number << 8) | dev->devfn; - __asm__("lcall (%%esi)\n\t" + __asm__("lcall (%%esi); cld\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" "1:" @@ -538,7 +538,7 @@ unsigned long ret; unsigned long bx = (dev->bus->number << 8) | dev->devfn; - __asm__("lcall (%%esi)\n\t" + __asm__("lcall (%%esi); cld\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" "1:" @@ -702,7 +702,7 @@ __asm__("push %%es\n\t" "push %%ds\n\t" "pop %%es\n\t" - "lcall (%%esi)\n\t" + "lcall (%%esi); cld\n\t" "pop %%es\n\t" "jc 1f\n\t" "xor %%ah, %%ah\n" --- linux/arch/i386/kernel/process.c.orig Sat Nov 27 06:00:12 1999 +++ linux/arch/i386/kernel/process.c Sat Nov 27 06:26:25 1999 @@ -462,7 +462,7 @@ struct pt_regs * childregs; childregs = ((struct pt_regs *) (THREAD_SIZE + (unsigned long) p)) - 1; - *childregs = *regs; + cpy(childregs, regs); childregs->eax = 0; childregs->esp = esp; @@ -475,7 +475,7 @@ savesegment(gs,p->thread.gs); unlazy_fpu(current); - p->thread.i387 = current->thread.i387; + cpy(&p->thread.i387, ¤t->thread.i387); return 0; }
| |