Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 06 Sep 2014 16:04:49 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r270850 - in head/sys: i386/i386 i386/include i386/isa x86/acpica
Message-ID:  <1432043.pIBGVXe1sj@ralph.baldwin.cx>
In-Reply-To: <201409051044.05853.jhb@freebsd.org>
References:  <201408301748.s7UHmc6H059701@svn.freebsd.org> <20140905084305.GN2737@kib.kiev.ua> <201409051044.05853.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Friday, September 05, 2014 10:44:05 AM John Baldwin wrote:
> On Friday, September 05, 2014 4:43:05 am Konstantin Belousov wrote:
> > There is one weird detail, not touched by your patch.  Amd64 resume
> > path calls initializecpu(), while i386 does not.  I do not see any
> > use for the call, the reload of CRX registers by trampoline/resumectx
> > should already set everything to working state.  E.g., enabling XMM
> > in CR4 after fpu state is restored looks strange.
> 
> I can test that.

I looked at this, and I actually think calling initializecpu() is correct.
It is true that it will set bits in CR4 that are already set, but it also
does vendor-specific initialization (e.g. init_amd() and init_via() on amd64
set MSRs, and on i386 there is a lot more of those, though most of the
CPUs with extra settings probably do not support SMP or run in ACPI systems).
We could either save and restore those various vendor-specific MSRs and 
registers in suspend/resume, or just call initializecpu() to set their values.  
The latter approach seems simpler, so I chose that route.  In immediate terms, 
it should fix enabling PG_NX in MSR_EFER on i386 + PAE resume, but it also 
makes the AP startup case simpler and closer to the amd64 code.  I also moved 
some code that set MSRs out of printcpuinfo() in identcpu.c and into 
initializecpu() instead.  Finally, I split initializecpucache() out similar to 
how it is split on amd64.

Ah, I see one bug I will fix in my tree, pc98's machdep.c needs the change to
call initializecpucache().

--- //depot/vendor/freebsd/src/sys/i386/i386/initcpu.c
+++ //depot/user/jhb/acpipci/i386/i386/initcpu.c
@@ -59,6 +59,12 @@
 static void init_6x86(void);
 #endif /* I486_CPU */
 
+#if defined(I586_CPU) && defined(CPU_WT_ALLOC)
+static void	enable_K5_wt_alloc(void);
+static void	enable_K6_wt_alloc(void);
+static void	enable_K6_2_wt_alloc(void);
+#endif
+
 #ifdef I686_CPU
 static void	init_6x86MX(void);
 static void	init_ppro(void);
@@ -527,6 +533,8 @@
 	intr_restore(saveintr);
 }
 
+static int ppro_apic_used = -1;
+
 static void
 init_ppro(void)
 {
@@ -535,9 +543,29 @@
 	/*
 	 * Local APIC should be disabled if it is not going to be used.
 	 */
-	apicbase = rdmsr(MSR_APICBASE);
-	apicbase &= ~APICBASE_ENABLED;
-	wrmsr(MSR_APICBASE, apicbase);
+	if (ppro_apic_used != 1) {
+		apicbase = rdmsr(MSR_APICBASE);
+		apicbase &= ~APICBASE_ENABLED;
+		wrmsr(MSR_APICBASE, apicbase);
+		ppro_apic_used = 0;
+	}
+}
+
+/*
+ * If the local APIC is going to be used after being disabled above,
+ * re-enable it and don't disable it in the future.
+ */
+void
+ppro_reenable_apic(void)
+{
+	u_int64_t	apicbase;
+
+	if (ppro_apic_used == 0) {
+		apicbase = rdmsr(MSR_APICBASE);
+		apicbase |= APICBASE_ENABLED;
+		wrmsr(MSR_APICBASE, apicbase);
+		ppro_apic_used = 1;
+	}
 }
 
 /*
@@ -646,20 +674,6 @@
 }
 #endif
 
-/*
- * Initialize CR4 (Control register 4) to enable SSE instructions.
- */
-void
-enable_sse(void)
-{
-#if defined(CPU_ENABLE_SSE)
-	if ((cpu_feature & CPUID_XMM) && (cpu_feature & CPUID_FXSR)) {
-		load_cr4(rcr4() | CR4_FXSR | CR4_XMM);
-		cpu_fxsr = hw_instruction_sse = 1;
-	}
-#endif
-}
-
 extern int elf32_nxstack;
 
 void
@@ -692,6 +706,27 @@
 #ifdef I586_CPU
 	case CPU_586:
 		switch (cpu_vendor_id) {
+		case CPU_VENDOR_AMD:
+#ifdef CPU_WT_ALLOC
+			if (((cpu_id & 0x0f0) > 0) &&
+			    ((cpu_id & 0x0f0) < 0x60) &&
+			    ((cpu_id & 0x00f) > 3))
+				enable_K5_wt_alloc();
+			else if (((cpu_id & 0x0f0) > 0x80) ||
+			    (((cpu_id & 0x0f0) == 0x80) &&
+				(cpu_id & 0x00f) > 0x07))
+				enable_K6_2_wt_alloc();
+			else if ((cpu_id & 0x0f0) > 0x50)
+				enable_K6_wt_alloc();
+#endif
+			if ((cpu_id & 0xf0) == 0xa0)
+				/*
+				 * Make sure the TSC runs through
+				 * suspension, otherwise we can't use
+				 * it as timecounter
+				 */
+				wrmsr(0x1900, rdmsr(0x1900) | 0x20ULL);
+			break;
 		case CPU_VENDOR_CENTAUR:
 			init_winchip();
 			break;
@@ -762,7 +797,17 @@
 	default:
 		break;
 	}
-	enable_sse();
+#if defined(CPU_ENABLE_SSE)
+	if ((cpu_feature & CPUID_XMM) && (cpu_feature & CPUID_FXSR)) {
+		load_cr4(rcr4() | CR4_FXSR | CR4_XMM);
+		cpu_fxsr = hw_instruction_sse = 1;
+	}
+#endif
+}
+
+void
+initializecpucache(void)
+{
 
 	/*
 	 * CPUID with %eax = 1, %ebx returns
@@ -839,7 +884,7 @@
  * Enable write allocate feature of AMD processors.
  * Following two functions require the Maxmem variable being set.
  */
-void
+static void
 enable_K5_wt_alloc(void)
 {
 	u_int64_t	msr;
@@ -885,7 +930,7 @@
 	}
 }
 
-void
+static void
 enable_K6_wt_alloc(void)
 {
 	quad_t	size;
@@ -945,7 +990,7 @@
 	intr_restore(saveintr);
 }
 
-void
+static void
 enable_K6_2_wt_alloc(void)
 {
 	quad_t	size;
--- //depot/vendor/freebsd/src/sys/i386/i386/machdep.c
+++ //depot/user/jhb/acpipci/i386/i386/machdep.c
@@ -2753,6 +2753,7 @@
 	setidt(IDT_GP, &IDTVEC(prot),  SDT_SYS386TGT, SEL_KPL,
 	    GSEL(GCODE_SEL, SEL_KPL));
 	initializecpu();	/* Initialize CPU registers */
+	initializecpucache();
 
 	/* make an initial tss so cpu can get interrupt stack on syscall! */
 	/* Note: -16 is so we can grow the trapframe if we came from vm86 */
--- //depot/vendor/freebsd/src/sys/i386/i386/mp_machdep.c
+++ //depot/user/jhb/acpipci/i386/i386/mp_machdep.c
@@ -745,25 +745,15 @@
 	/* set up CPU registers and state */
 	cpu_setregs();
 
+	/* set up SSE/NX registers */
+	initializecpu();
+
 	/* set up FPU state on the AP */
 	npxinit();
 
-	/* set up SSE registers */
-	enable_sse();
-
 	if (cpu_ops.cpu_init)
 		cpu_ops.cpu_init();
 
-#ifdef PAE
-	/* Enable the PTE no-execute bit. */
-	if ((amd_feature & AMDID_NX) != 0) {
-		uint64_t msr;
-
-		msr = rdmsr(MSR_EFER) | EFER_NXE;
-		wrmsr(MSR_EFER, msr);
-	}
-#endif
-
 	/* A quick check from sanity claus */
 	cpuid = PCPU_GET(cpuid);
 	if (PCPU_GET(apic_id) != lapic_id()) {
@@ -1528,6 +1518,7 @@
 	} else {
 		npxresume(&susppcbs[cpu]->sp_fpususpend);
 		pmap_init_pat();
+		initializecpu();
 		PCPU_SET(switchtime, 0);
 		PCPU_SET(switchticks, ticks);
 
--- //depot/vendor/freebsd/src/sys/i386/include/md_var.h
+++ //depot/user/jhb/acpipci/i386/include/md_var.h
@@ -99,14 +99,9 @@
 void	dump_add_page(vm_paddr_t);
 void	dump_drop_page(vm_paddr_t);
 void	finishidentcpu(void);
-#if defined(I586_CPU) && defined(CPU_WT_ALLOC)
-void	enable_K5_wt_alloc(void);
-void	enable_K6_wt_alloc(void);
-void	enable_K6_2_wt_alloc(void);
-#endif
-void	enable_sse(void);
 void	fillw(int /*u_short*/ pat, void *base, size_t cnt);
 void	initializecpu(void);
+void	initializecpucache(void);
 void	i686_pagezero(void *addr);
 void	sse2_pagezero(void *addr);
 void	init_AMD_Elan_sc520(void);
@@ -114,6 +109,7 @@
 int	isa_nmi(int cd);
 vm_paddr_t kvtop(void *addr);
 void	panicifcpuunsupported(void);
+void	ppro_reenable_apic(void);
 void	printcpuinfo(void);
 void	setidt(int idx, alias_for_inthand_t *func, int typ, int dpl, int 
selec);
 int     user_dbreg_trap(void);
--- //depot/vendor/freebsd/src/sys/i386/xen/mp_machdep.c
+++ //depot/user/jhb/acpipci/i386/xen/mp_machdep.c
@@ -602,17 +602,8 @@
 	npxinit();
 #if 0
 	
-	/* set up SSE registers */
-	enable_sse();
-#endif
-#if 0 && defined(PAE)
-	/* Enable the PTE no-execute bit. */
-	if ((amd_feature & AMDID_NX) != 0) {
-		uint64_t msr;
-
-		msr = rdmsr(MSR_EFER) | EFER_NXE;
-		wrmsr(MSR_EFER, msr);
-	}
+	/* set up SSE/NX registers */
+	initializecpu();
 #endif
 #if 0
 	/* A quick check from sanity claus */
--- //depot/vendor/freebsd/src/sys/x86/x86/identcpu.c
+++ //depot/user/jhb/acpipci/x86/x86/identcpu.c
@@ -405,30 +405,11 @@
 			break;
 		case 0x5a0:
 			strcat(cpu_model, "Geode LX");
-			/*
-			 * Make sure the TSC runs through suspension,
-			 * otherwise we can't use it as timecounter
-			 */
-			wrmsr(0x1900, rdmsr(0x1900) | 0x20ULL);
 			break;
 		default:
 			strcat(cpu_model, "Unknown");
 			break;
 		}
-#if defined(I586_CPU) && defined(CPU_WT_ALLOC)
-		if ((cpu_id & 0xf00) == 0x500) {
-			if (((cpu_id & 0x0f0) > 0)
-			    && ((cpu_id & 0x0f0) < 0x60)
-			    && ((cpu_id & 0x00f) > 3))
-				enable_K5_wt_alloc();
-			else if (((cpu_id & 0x0f0) > 0x80)
-				 || (((cpu_id & 0x0f0) == 0x80)
-				     && (cpu_id & 0x00f) > 0x07))
-				enable_K6_2_wt_alloc();
-			else if ((cpu_id & 0x0f0) > 0x50)
-				enable_K6_wt_alloc();
-		}
-#endif
 #else
 		if ((cpu_id & 0xf00) == 0xf00)
 			strcat(cpu_model, "AMD64 Processor");
--- //depot/vendor/freebsd/src/sys/x86/x86/local_apic.c
+++ //depot/user/jhb/acpipci/x86/x86/local_apic.c
@@ -55,7 +55,6 @@
 #include <vm/pmap.h>
 
 #include <x86/apicreg.h>
-#include <machine/cpu.h>
 #include <machine/cputypes.h>
 #include <machine/frame.h>
 #include <machine/intr_machdep.h>
@@ -1331,9 +1330,6 @@
 apic_init(void *dummy __unused)
 {
 	struct apic_enumerator *enumerator;
-#ifndef __amd64__
-	uint64_t apic_base;
-#endif
 	int retval, best;
 
 	/* We only support built in local APICs. */
@@ -1375,12 +1371,7 @@
 	 * CPUs during early startup.  We need to turn the local APIC back
 	 * on on such CPUs now.
 	 */
-	if (cpu == CPU_686 && cpu_vendor_id == CPU_VENDOR_INTEL &&
-	    (cpu_id & 0xff0) == 0x610) {
-		apic_base = rdmsr(MSR_APICBASE);
-		apic_base |= APICBASE_ENABLED;
-		wrmsr(MSR_APICBASE, apic_base);
-	}
+	ppro_reenable_apic();
 #endif
 
 	/* Probe the CPU's in the system. */

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1432043.pIBGVXe1sj>