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>