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.
They're pretending to be an AMD K10 processor.
On all real hardware I've tested this works fine. If you wish to be pedantic,
patches are welcome.
> >> Can you check what cpuid 80000007 returns by running 'x86info -r
> >> | grep 80000007' in a Linux guest with the same command line? if
> >> edx returns zero, then it's OpenBSD not checking cpuid
> >> correctly.
>
> EDX is zero in a Linux guest (i386 and x86_64).
The previous patch avoids touching the msr at all if ACPI indicates speed
scaling is unavailable, this should prevent your panic.
> Walter
Both i386/amd64(..fixed) patches attached below.
-Bryan.
Index: amd64/amd64/k1x-pstate.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/k1x-pstate.c,v
retrieving revision 1.2
diff -u -p -u -r1.2 k1x-pstate.c
--- amd64/amd64/k1x-pstate.c 29 May 2011 12:29:28 -0000 1.2
+++ amd64/amd64/k1x-pstate.c 8 Nov 2011 18:30:59 -0000
@@ -75,7 +75,7 @@ struct k1x_cpu_state *k1x_current_state;
void k1x_transition(struct k1x_cpu_state *, int);
#if NACPICPU > 0
-void k1x_acpi_init(struct k1x_cpu_state *, u_int64_t);
+void k1x_acpi_init(struct k1x_cpu_state *);
void k1x_acpi_states(struct k1x_cpu_state *, struct acpicpu_pss *, int,
u_int64_t);
#endif
@@ -154,14 +154,17 @@ k1x_acpi_states(struct k1x_cpu_state *cs
}
void
-k1x_acpi_init(struct k1x_cpu_state *cstate, u_int64_t msr)
+k1x_acpi_init(struct k1x_cpu_state *cstate)
{
struct acpicpu_pss *pss;
+ u_int64_t msr;
cstate->n_states = acpicpu_fetch_pss(&pss);
if (cstate->n_states == 0)
return;
+ msr = rdmsr(MSR_K1X_STATUS);
+
k1x_acpi_states(cstate, pss, cstate->n_states, msr);
return;
@@ -172,12 +175,9 @@ k1x_acpi_init(struct k1x_cpu_state *csta
void
k1x_init(struct cpu_info *ci)
{
-#if NACPICPU > 0
- u_int64_t msr;
-#endif
- u_int i;
struct k1x_cpu_state *cstate;
struct k1x_state *state;
+ u_int i;
if (setperf_prio > 1)
return;
@@ -189,8 +189,7 @@ k1x_init(struct cpu_info *ci)
cstate->n_states = 0;
#if NACPICPU > 0
- msr = rdmsr(MSR_K1X_STATUS);
- k1x_acpi_init(cstate, msr);
+ k1x_acpi_init(cstate);
#endif
if (cstate->n_states) {
printf("%s: %d MHz: speeds:",
Index: i386/i386/k1x-pstate.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/k1x-pstate.c,v
retrieving revision 1.2
diff -u -p -u -r1.2 k1x-pstate.c
--- i386/i386/k1x-pstate.c 29 May 2011 12:29:28 -0000 1.2
+++ i386/i386/k1x-pstate.c 8 Nov 2011 18:30:59 -0000
@@ -75,7 +75,7 @@ struct k1x_cpu_state *k1x_current_state;
void k1x_transition(struct k1x_cpu_state *, int);
#if NACPICPU > 0
-void k1x_acpi_init(struct k1x_cpu_state *, u_int64_t);
+void k1x_acpi_init(struct k1x_cpu_state *);
void k1x_acpi_states(struct k1x_cpu_state *, struct acpicpu_pss *, int,
u_int64_t);
#endif
@@ -154,14 +154,17 @@ k1x_acpi_states(struct k1x_cpu_state *cs
}
void
-k1x_acpi_init(struct k1x_cpu_state *cstate, u_int64_t msr)
+k1x_acpi_init(struct k1x_cpu_state *cstate)
{
struct acpicpu_pss *pss;
+ u_int64_t msr;
cstate->n_states = acpicpu_fetch_pss(&pss);
if (cstate->n_states == 0)
return;
+ msr = rdmsr(MSR_K1X_STATUS);
+
k1x_acpi_states(cstate, pss, cstate->n_states, msr);
return;
@@ -172,12 +175,9 @@ k1x_acpi_init(struct k1x_cpu_state *csta
void
k1x_init(struct cpu_info *ci)
{
-#if NACPICPU > 0
- u_int64_t msr;
-#endif
- u_int i;
struct k1x_cpu_state *cstate;
struct k1x_state *state;
+ u_int i;
if (setperf_prio > 1)
return;
@@ -189,8 +189,7 @@ k1x_init(struct cpu_info *ci)
cstate->n_states = 0;
#if NACPICPU > 0
- msr = rdmsr(MSR_K1X_STATUS);
- k1x_acpi_init(cstate, msr);
+ k1x_acpi_init(cstate);
#endif
if (cstate->n_states) {
printf("%s: %d MHz: speeds:",
