On Wed, Nov 09, 2011 at 12:05:04PM -0500, Brynet wrote:
> On Wed, Nov 09, 2011 at 03:35:01PM +0100, Walter Haidinger wrote:
> > Does the patch fix the following?
> >
> > I've forwarding the bug report to the Linux KVM developers.
> > The response:
>
> The patch in my first email should be enough to avoid the issue, the i386
> patch was fine, only the amd64 patch was incorrect.
>
> > On 09.11.2011 14:40, Avi Kivity wrote:
> > > Actually, it looks like an OpenBSD bug. According to the AMD
> > > documentation:
> > >> "The current P-state value can be read using the P-State Status
> > >> Register. The P-State Current Limit Register and the P-State
> > >> Status Register are read-only registers. Writes to these
> > >> registers cause a #GP exception. Support for hardware P-state
> > >> control is indicated by EDX bit 7 as returned by CPUID function
> > >> 8000_0007h. Figure 18-1 shows the format of the P-State Current
> > >> Limit register."
>
> The panic you hit is for an msr read, not a write. I'm aware those registers
> are read-only.
>
> The CPUID check isn't done, it matches on all family 10 and/or higher AMD
> processors.
As pointed out earlier the AMD documentation says it should be,
the MSRs shouldn't be touched if the cpuid flag isn't set.
Access to unimplemented MSRs often leads to things like faults/reboots
on real hardware. And while a K1x machine that doesn't have hardware
pstate control might not exist now, code that follows AMD's documentation
would work on such a machine.
The k1x_init() function should probably be renamed to something
like k1x_pstate_init() for clarity as well.
Index: i386/locore.s
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/locore.s,v
retrieving revision 1.141
diff -u -p -r1.141 locore.s
--- i386/locore.s 2 Nov 2011 23:53:44 -0000 1.141
+++ i386/locore.s 10 Nov 2011 01:15:46 -0000
@@ -157,6 +157,7 @@
.globl _C_LABEL(cpu_miscinfo)
.globl _C_LABEL(cpu_feature), _C_LABEL(cpu_ecxfeature)
.globl _C_LABEL(ecpu_feature), _C_LABEL(ecpu_ecxfeature)
+ .globl _C_LABEL(cpu_apmfeature)
.globl _C_LABEL(cpu_cache_eax), _C_LABEL(cpu_cache_ebx)
.globl _C_LABEL(cpu_cache_ecx), _C_LABEL(cpu_cache_edx)
.globl _C_LABEL(cold), _C_LABEL(cnvmem), _C_LABEL(extmem)
@@ -197,6 +198,7 @@ _C_LABEL(cpu_feature): .long 0 # feature
_C_LABEL(ecpu_feature): .long 0 # extended feature flags from 'cpuid'
_C_LABEL(cpu_ecxfeature):.long 0 # ecx feature flags from 'cpuid'
_C_LABEL(ecpu_ecxfeature): .long 0 # extended ecx feature flags
+_C_LABEL(cpu_apmfeature): .long 0 # Advanced Power Management Information
_C_LABEL(cpuid_level): .long -1 # max. lvl accepted by 'cpuid' insn
_C_LABEL(cpu_cache_eax):.long 0
_C_LABEL(cpu_cache_ebx):.long 0
@@ -414,6 +416,9 @@ try586: /* Use the `cpuid' instruction.
cpuid
movl %edx,RELOC(_C_LABEL(ecpu_feature))
movl %ecx,RELOC(_C_LABEL(ecpu_ecxfeature))
+ movl $0x80000007,%eax
+ cpuid
+ movl %edx,RELOC(_C_LABEL(cpu_apmfeature))
movl $0x80000002,%eax
cpuid
movl %eax,RELOC(_C_LABEL(cpu_brandstr))
Index: i386/machdep.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.506
diff -u -p -r1.506 machdep.c
--- i386/machdep.c 2 Nov 2011 23:53:44 -0000 1.506
+++ i386/machdep.c 10 Nov 2011 01:15:48 -0000
@@ -1347,7 +1347,7 @@ amd_family6_setperf_setup(struct cpu_inf
k8_powernow_init();
break;
}
- if (ci->ci_family >= 0x10)
+ if (ci->ci_family >= 0x10 && cpu_apmfeature & CPUIDAPM_PSTATE)
k1x_init(ci);
}
#endif
Index: include/cpu.h
===================================================================
RCS file: /cvs/src/sys/arch/i386/include/cpu.h,v
retrieving revision 1.121
diff -u -p -r1.121 cpu.h
--- include/cpu.h 2 Nov 2011 23:53:44 -0000 1.121
+++ include/cpu.h 10 Nov 2011 01:15:48 -0000
@@ -314,6 +314,7 @@ extern int cpu_feature;
extern int ecpu_feature;
extern int cpu_ecxfeature;
extern int ecpu_ecxfeature;
+extern int cpu_apmfeature;
extern int cpu_cache_eax;
extern int cpu_cache_ebx;
extern int cpu_cache_ecx;
Index: include/specialreg.h
===================================================================
RCS file: /cvs/src/sys/arch/i386/include/specialreg.h,v
retrieving revision 1.40
diff -u -p -r1.40 specialreg.h
--- include/specialreg.h 2 Nov 2011 23:53:44 -0000 1.40
+++ include/specialreg.h 10 Nov 2011 01:15:50 -0000
@@ -167,6 +167,8 @@
#define CPUIDECX_WDT 0x00002000 /* watchdog timer */
#define CPUIDECX_FMA4 0x00010000 /* 4-operand FMA instructions */
+#define CPUIDAPM_PSTATE 0x00000080 /* hardware P-state control */
+
/*
* Model-specific registers for the i386 family
*/
