From nobody Tue Jul 7 13:05:41 1998 Received: (from nobody@localhost) by hub.freebsd.org (8.8.8/8.8.8) id NAA26120; Tue, 7 Jul 1998 13:05:41 -0700 (PDT) (envelope-from nobody) Message-Id: <199807072005.NAA26120@hub.freebsd.org> Date: Tue, 7 Jul 1998 13:05:41 -0700 (PDT) From: gallatin@cs.duke.edu To: freebsd-gnats-submit@freebsd.org Subject: (cpu == CPU_686) in pmap.c shoud also apply to CPU_PII, pmap_setdevram() disabled in wrong place X-Send-Pr-Version: www-1.0 >Number: 7201 >Category: i386 >Synopsis: (cpu == CPU_686) in pmap.c shoud also apply to CPU_PII, pmap_setdevram() disabled in wrong place >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-bugs >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Jul 7 13:10:01 PDT 1998 >Closed-Date: Wed May 26 16:09:52 PDT 1999 >Last-Modified: Wed May 26 16:11:29 PDT 1999 >Originator: Andrew Gallatin >Release: FreeBSD 3.0-CURRENT >Organization: Duke University, Department of Computer Science >Environment: FreeBSD ladybug 3.0-CURRENT FreeBSD 3.0-CURRENT #10: Tue Jul 7 15:18:52 EDT 1998 gallatin@grasshopper.cs.duke.edu:/usr/src/sys/compile/TPZ i386 >Description: o Many Pentium Pro class features in pmap.c are enabled if cpu == CPU_686. With the recent addition of the CPU_PII, the CPU_PII cpu needs to be added to those tests. o The dangerous call to pmap_setdevram() which enables write combining on video cards in P6/PII machines is already disabled in vga_probe(). Others may want to enable write combining on other devices (like Myrinet cards) -- the immediate return in pmap_setdevram() should go away. >How-To-Repeat: Try enabling WC on a memory mapped PCI device on a PII ;-) >Fix: Index: pmap.c =================================================================== RCS file: /scratch/freebsd-cvs/src/sys/i386/i386/pmap.c,v retrieving revision 1.202 diff -c -r1.202 pmap.c *** pmap.c 1998/05/21 07:47:34 1.202 --- pmap.c 1998/07/07 18:42:08 *************** *** 458,464 **** { int i; ! if (cpu == CPU_686) { for(i = 0; i < NPPROVMTRR; i++) { PPro_vmtrr[i].base = rdmsr(PPRO_VMTRRphysBase0 + i * 2); PPro_vmtrr[i].mask = rdmsr(PPRO_VMTRRphysMask0 + i * 2); --- 458,464 ---- { int i; ! if ((cpu == CPU_686) || (cpu == CPU_PII)) { for(i = 0; i < NPPROVMTRR; i++) { PPro_vmtrr[i].base = rdmsr(PPRO_VMTRRphysBase0 + i * 2); PPro_vmtrr[i].mask = rdmsr(PPRO_VMTRRphysMask0 + i * 2); *************** *** 471,477 **** { int i; ! if (cpu == CPU_686) { wbinvd(); for(i = 0; i < NPPROVMTRR; i++) { wrmsr(PPRO_VMTRRphysBase0 + i * 2, PPro_vmtrr[i].base); --- 471,477 ---- { int i; ! if ((cpu == CPU_686) || (cpu == CPU_PII)) { wbinvd(); for(i = 0; i < NPPROVMTRR; i++) { wrmsr(PPRO_VMTRRphysBase0 + i * 2, PPro_vmtrr[i].base); *************** *** 483,489 **** void pmap_setvidram(void) { ! if (cpu == CPU_686) { wbinvd(); /* * Set memory between 0-640K to be WB --- 483,489 ---- void pmap_setvidram(void) { ! if ((cpu == CPU_686) || (cpu == CPU_PII)) { wbinvd(); /* * Set memory between 0-640K to be WB *************** *** 505,512 **** unsigned long long base; unsigned long long mask; ! return; ! if (cpu != CPU_686) return; free = -1; --- 505,511 ---- unsigned long long base; unsigned long long mask; ! if ((cpu != CPU_686) && (cpu != CPU_PII)) return; free = -1; *************** *** 2799,2805 **** cpu_invlpg(&prv_CPAGE3); #if defined(I686_CPU) ! if (cpu == CPU_686) i686_pagezero(&prv_CPAGE3); else #endif --- 2798,2804 ---- cpu_invlpg(&prv_CPAGE3); #if defined(I686_CPU) ! if ((cpu == CPU_686) || (cpu == CPU_PII)) i686_pagezero(&prv_CPAGE3); else #endif *************** *** 2820,2826 **** } #if defined(I686_CPU) ! if (cpu == CPU_686) i686_pagezero(CADDR2); else #endif --- 2819,2825 ---- } #if defined(I686_CPU) ! if ((cpu == CPU_686) || (cpu == CPU_PII)) i686_pagezero(CADDR2); else #endif >Release-Note: >Audit-Trail: State-Changed-From-To: open->feedback State-Changed-By: phk State-Changed-When: Wed Jul 8 23:20:30 PDT 1998 State-Changed-Why: I think the test should be done on the cpu_class variable instead and please don't mix two issues in the same PR. From: smoergrd@oslo.geco-prakla.slb.com (Dag-Erling Coidan Smørgrav) To: gallatin@cs.duke.edu Cc: freebsd-gnats-submit@freebsd.org Subject: Re: i386/7201: (cpu == CPU_686) in pmap.c shoud also apply to CPU_PII, pmap_setdevram() disabled in wrong place Date: 09 Jul 1998 11:50:33 +0200 gallatin@cs.duke.edu writes: > o Many Pentium Pro class features in pmap.c are enabled if cpu == CPU_686. > With the recent addition of the CPU_PII, the CPU_PII cpu needs to be > added to those tests. Hmm, I should have checked this when I introduced the code to identify PII processors, so color me guilty. But as Poul-Henning remarked, these tests should check the CPU class and not the CPU model, so your fix is inadequate (or perhaps I should rather say that it is only a short-term fix - until the next 686-class CPU hits the market) I'll try to set aside some time to look into this. Thank you very much for pointing it out to us. DES (bad DES, baaaad DES) -- Dag-Erling Smørgrav - smoergrd@oslo.geco-prakla.slb.com From: Andrew Gallatin To: smoergrd@oslo.geco-prakla.slb.com (Dag-Erling Coidan Smørgrav) Cc: freebsd-gnats-submit@freebsd.org Subject: Re: i386/7201: (cpu == CPU_686) in pmap.c shoud also apply to CPU_PII, pmap_setdevram() disabled in wrong place Date: Thu, 9 Jul 1998 11:25:28 -0400 (EDT) Dag-Erling Coidan Smørgrav writes: > gallatin@cs.duke.edu writes: > > o Many Pentium Pro class features in pmap.c are enabled if cpu == CPU_686. > > With the recent addition of the CPU_PII, the CPU_PII cpu needs to be > > added to those tests. > > Hmm, I should have checked this when I introduced the code to identify > PII processors, so color me guilty. But as Poul-Henning remarked, > these tests should check the CPU class and not the CPU model, so your > fix is inadequate (or perhaps I should rather say that it is only a > short-term fix - until the next 686-class CPU hits the market) > > I'll try to set aside some time to look into this. Thank you very much > for pointing it out to us. > > DES (bad DES, baaaad DES) > -- > Dag-Erling Smørgrav - smoergrd@oslo.geco-prakla.slb.com I'd considered that myself, but I don't know anything about the Cyrix 6x86MX, which is identified as a CPUCLASS_686 cpu in identcpu.c. I was concerned that the P.Pro/PII optimizations in pmap.c might not apply to the Cyrix chip... Its a lot cleaner if they do. Drew State-Changed-From-To: feedback->closed State-Changed-By: nrahlstr State-Changed-When: Wed May 26 16:09:52 PDT 1999 State-Changed-Why: Fixed by msmith in revision 1.212. >Unformatted: