Date: Fri, 17 Aug 2001 00:21:49 -0700 From: Peter Wemm <peter@wemm.org> To: Mitsuru IWASAKI <iwasaki@jp.FreeBSD.org> Cc: arch@FreeBSD.ORG, audit@FreeBSD.ORG, kumabu@t3.rim.or.jp Subject: Re: CFR: Timing to enable CR4.PGE bit Message-ID: <20010817072149.0BCD63811@overcee.netplex.com.au> In-Reply-To: <20010809035801V.iwasaki@jp.FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Mitsuru IWASAKI wrote: > Hi, I've found a report in Japanese mailing list that CR4.PGE seems to > be enabled before CR0.PG in locore.s. This was originally reported by > Kumabuchi-san (Thanks!). > > According to developer's manual from Intel site, > ftp://download.intel.com/design/PentiumII/manuals/24319202.pdf > ---- > 2.5. CONTROL REGISTERS > [snip] > PGE > (2-17) > Page Global Enable (bit 7 of CR4). (Introduced in the P6 family > processors.) Enables the global page feature when set; disables the > global page feature when clear. [snip] In addition, the bit must not > ^^^^^^^^^^^^^^^^ > be enabled before paging is enabled via CR0.PG. Program correctness > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > may be affected by reversing this sequence, and processor performance > will be impacted. > ---- > > Currently, we enable CR4.PGE bit in create_pagetables, then enable > CR0.PG in locore.s. This seems to violate Intel's note. > > I've made patches for this, moving CR4.PGE enabling code to just > before calling init386(). > > Index: locore.s > =================================================================== > RCS file: /home/ncvs/src/sys/i386/i386/locore.s,v > retrieving revision 1.144 > diff -u -r1.144 locore.s > --- locore.s 2001/07/12 06:32:50 1.144 > +++ locore.s 2001/08/08 17:49:28 > @@ -374,6 +374,12 @@ > movl IdlePTD,%esi > movl %esi,PCB_CR3(%eax) > > + testl $CPUID_PGE, R(cpu_feature) > + jz 1f > + movl %cr4, %eax > + orl $CR4_PGE, %eax > + movl %eax, %cr4 > +1: > pushl physfree /* value of first for init386(f irst) */ > call init386 /* wire 386 chip for unix opera tion */ > > @@ -718,13 +724,6 @@ > */ > > create_pagetables: > - > - testl $CPUID_PGE, R(cpu_feature) > - jz 1f > - movl %cr4, %eax > - orl $CR4_PGE, %eax > - movl %eax, %cr4 > -1: > > /* Find end of kernel image (rounded up to a page boundary). */ > movl $R(_end),%esi This part is fine. However: > Also I have another thing to be confirmed. Should we utilize TLB by > enabling PGE bit at very later stage? I think it would be more > efficient to cache page entries with G flag in multi-user environment, > not in kernel bootstrap. If we enable PGE bit in locore.s, TLB could > be occupied by entries which is referenced by initialization code > (yes, most of them are executed only once). > # but I could be wrong... The G bit does not "lock" the TLB entries in. All it does is stop unnecessary flushes when %cr3 is changed. If entries are not used for a short while, they will be recycled when the TLB slot is needed for something else soon enough. ie: this should not be a problem. > Anyway, patch for this is attached here. Regardless of my doubts above, I do have a problem with the patch... It only works for the PPro/p2/p3 and not the p4. Is this intentional? All have the CPUID_PGE bit. I think the test for cpu_id & 0x600 is bogus and should be removed. > Thanks. > > Index: initcpu.c > =================================================================== > RCS file: /home/ncvs/src/sys/i386/i386/initcpu.c,v > retrieving revision 1.29 > diff -u -r1.29 initcpu.c > --- initcpu.c 2001/07/13 11:23:06 1.29 > +++ initcpu.c 2001/08/08 15:35:51 > @@ -847,3 +847,23 @@ > printf("CR0=%x\n", cr0); > } > #endif /* DDB */ > + > +/* > + * Enable CR4.PGE after kernel bootstrap. > + */ > + > +static void > +enable_i686_pge(void *unused) > +{ > + > + if ((cpu_feature & CPUID_PGE) && > + (cpu_id & 0xf00) == 0x600) { > + load_cr4(rcr4() | CR4_PGE); > + if (bootverbose) { > + printf("P6 family processor PGE on\n"); > + } > + } > +} > + > +SYSINIT(initcpu, SI_SUB_RUN_SCHEDULER, SI_ORDER_FIRST, enable_i686_pge, NULL ) > + > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message > > Cheers, -Peter -- Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com; peter@netplex.com.au "All of this is for nothing if we don't go to the stars" - JMS/B5 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010817072149.0BCD63811>