From owner-freebsd-stable Sat Sep 15 12: 7:49 2001 Delivered-To: freebsd-stable@freebsd.org Received: from wall.polstra.com (rtrwan160.accessone.com [206.213.115.74]) by hub.freebsd.org (Postfix) with ESMTP id E68A637B40C for ; Sat, 15 Sep 2001 12:07:43 -0700 (PDT) Received: from vashon.polstra.com (vashon.polstra.com [206.213.73.13]) by wall.polstra.com (8.11.3/8.11.3) with ESMTP id f8FJ6Qg38087; Sat, 15 Sep 2001 12:06:26 -0700 (PDT) (envelope-from jdp@wall.polstra.com) Received: (from jdp@localhost) by vashon.polstra.com (8.11.6/8.11.0) id f8FJ6Kk74842; Sat, 15 Sep 2001 12:06:20 -0700 (PDT) (envelope-from jdp) Date: Sat, 15 Sep 2001 12:06:20 -0700 (PDT) Message-Id: <200109151906.f8FJ6Kk74842@vashon.polstra.com> To: stable@freebsd.org From: John Polstra Cc: dres@earth.serd.org Subject: Re: sys/i386/i386/identcpu.c bugfix In-Reply-To: <20010905160610.U1222-100000@earth.serd.org> References: <20010905160610.U1222-100000@earth.serd.org> Organization: Polstra & Co., Seattle, WA Sender: owner-freebsd-stable@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In article <20010905160610.U1222-100000@earth.serd.org>, Stefan Keller wrote: > > the do_cpuid() inline asm routine in identcpu.c has an incomplete > description of clobbered registers, which shows up when the kernel is > compiled with higher optimisations turned on. > The fix: > > --- sys/i386/i386/identcpu.c.org Mon Sep 3 23:44:25 2001 > +++ sys/i386/i386/identcpu.c Wed Sep 5 03:49:16 2001 > @@ -121,7 +121,7 @@ > "movl %%edx, 12(%2);" > : "=a" (ax) > : "0" (ax), "S" (p) > - : "bx", "cx", "dx" > + : "bx", "cx", "dx", "cc", "memory" > ); > } > > Not specifying "memory" causes gcc to cache the contents of > the memory pointed to by p across do_cpuid() calls. > On my system (Athlon processor) this only showed up on the > CPU name; everything else was ok. Nice catch! I really encourage anybody who can do so to check our asm statements. I'm sure there are a lot more of them with incorrect constraints, and they are the main reason we have problems at -O2 and higher. These problems often get blamed on compiler bugs, but I don't believe it. Both Linux and BSD/OS use -O2 without problems. About your patch: I made a different version which I think is slightly better, because it specifies the memory clobbers more specifically. It's simpler, too. I checked the compiler's assembly language output, and it looks like it's doing the right thing. Could you please give it a try? Note, I removed the clobber of "cc" because the Intel book says that no flags are affected by the CPUID instruction. This is relative to today's -stable: Index: identcpu.c =================================================================== RCS file: /home/ncvs/src/sys/i386/i386/identcpu.c,v retrieving revision 1.80.2.6 diff -c -r1.80.2.6 identcpu.c *** identcpu.c 2001/07/19 09:12:07 1.80.2.6 --- identcpu.c 2001/09/15 19:04:23 *************** *** 114,127 **** do_cpuid(u_int ax, u_int *p) { __asm __volatile( ! ".byte 0x0f, 0xa2;" ! "movl %%eax, (%2);" ! "movl %%ebx, 4(%2);" ! "movl %%ecx, 8(%2);" ! "movl %%edx, 12(%2);" ! : "=a" (ax) : "0" (ax), "S" (p) - : "bx", "cx", "dx" ); } --- 114,122 ---- do_cpuid(u_int ax, u_int *p) { __asm __volatile( ! ".byte 0x0f, 0xa2" ! : "=a" (p[0]), "=b" (p[1]), "=c" (p[2]), "=d" (p[3]) : "0" (ax), "S" (p) ); } John -- John Polstra jdp@polstra.com John D. Polstra & Co., Inc. Seattle, Washington USA "Disappointment is a good sign of basic intelligence." -- Chögyam Trungpa To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-stable" in the body of the message