Skip to contentSkip to conversation

x86/amd: Fix xen_apic_write warnings in Dom0

PATCH In review Wei Wang 3 participants 4 messages Oct 9, 2012 – Oct 15, 2012

lore
Share & tools
lore

Wei Wang

author
#0 lore raw

PATCH 1/1 x86/amd: Fix xen_apic_write warnings in Dom0

Hi, this patch fixes a xen_apic_write warnings in Dom0

[    0.020294] ------------[ cut here ]------------
[    0.020311] WARNING: at arch/x86/xen/enlighten.c:730
xen_apic_write+0x15/0x17()
[    0.020318] Hardware name: empty
[    0.020323] Modules linked in:
[    0.020334] Pid: 1, comm: swapper/0 Not tainted 3.3.8 #7
[    0.020340] Call Trace:
[    0.020354]  [<ffffffff81050379>] warn_slowpath_common+0x80/0x98
[    0.020369]  [<ffffffff810503a6>] warn_slowpath_null+0x15/0x17
[    0.020378]  [<ffffffff810034df>] xen_apic_write+0x15/0x17
[    0.020392]  [<ffffffff8101cb2b>] perf_events_lapic_init+0x2e/0x30
[    0.020410]  [<ffffffff81ee4dd0>] init_hw_perf_events+0x250/0x407
[    0.020419]  [<ffffffff81ee4b80>] ? check_bugs+0x2d/0x2d
[    0.020430]  [<ffffffff81002181>] do_one_initcall+0x7a/0x131
[    0.020444]  [<ffffffff81edbbf9>] kernel_init+0x91/0x15d
[    0.020456]  [<ffffffff817caaa4>] kernel_thread_helper+0x4/0x10
[    0.020471]  [<ffffffff817c347c>] ? retint_restore_args+0x5/0x6
[    0.020481]  [<ffffffff817caaa0>] ? gs_change+0x13/0x13
[    0.020500] ---[ end trace a7919e7f17c0a725 ]---

Kernel function check_hw_exists() writes 0xabcd to msr 0xc0010201 (Performance Event Counter 0) and read it again to check if it is running as dom0. Early amd cpus does not reset perf counters during warm reboot. If the kernel is booted with bare metal and then as a dom0, the content of msr 0xc0010201 will stay and the checking will pass and PMU will be enabled unexpectedly.

Signed-off-by Wei Wang <[email protected]>

Thanks
Wei

Quoted 1 line

From 5cb3c3ed7db525ccb44660ed30890ffeb00b2ea8 Mon Sep 17 00:00:00 2001

From: Wei Wang <[email protected]>
Date: Mon, 8 Oct 2012 15:48:55 +0200
Subject: [PATCH] x86/amd: Fix xen_apic_write warnings in Dom0

[    0.020294] ------------[ cut here ]------------
[    0.020311] WARNING: at arch/x86/xen/enlighten.c:730
xen_apic_write+0x15/0x17()
[    0.020318] Hardware name: empty
[    0.020323] Modules linked in:
[    0.020334] Pid: 1, comm: swapper/0 Not tainted 3.3.8 #7
[    0.020340] Call Trace:
[    0.020354]  [<ffffffff81050379>] warn_slowpath_common+0x80/0x98
[    0.020369]  [<ffffffff810503a6>] warn_slowpath_null+0x15/0x17
[    0.020378]  [<ffffffff810034df>] xen_apic_write+0x15/0x17
[    0.020392]  [<ffffffff8101cb2b>] perf_events_lapic_init+0x2e/0x30
[    0.020410]  [<ffffffff81ee4dd0>] init_hw_perf_events+0x250/0x407
[    0.020419]  [<ffffffff81ee4b80>] ? check_bugs+0x2d/0x2d
[    0.020430]  [<ffffffff81002181>] do_one_initcall+0x7a/0x131
[    0.020444]  [<ffffffff81edbbf9>] kernel_init+0x91/0x15d
[    0.020456]  [<ffffffff817caaa4>] kernel_thread_helper+0x4/0x10
[    0.020471]  [<ffffffff817c347c>] ? retint_restore_args+0x5/0x6
[    0.020481]  [<ffffffff817caaa0>] ? gs_change+0x13/0x13
[    0.020500] ---[ end trace a7919e7f17c0a725 ]---

Kernel function check_hw_exists() writes 0xabcd to msr 0xc0010201 (Performance Event Counter 0) and read it again to check if it is running as dom0. Early amd cpus does not reset perf counters during warm reboot. If the kernel is booted with bare metal and then as a dom0, the content of msr 0xc0010201 will stay and the checking will pass and PMU will be enabled unexpectedly.

Signed-off-by Wei Wang <[email protected]>
---
 xen/arch/x86/cpu/amd.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
xen/arch/x86/cpu/amd.c +11 -0
index c95349f..5909f8c 100644
@@ -485,6 +485,17 @@ static void __devinit init_amd(struct cpuinfo_x86 *c)
 	if (c->x86 > 0x11)
 		set_bit(X86_FEATURE_ARAT, c->x86_capability);
 
+	/* 
+	 * Prior to Family 0x14, perf counters are not reset during warm reboot.  
+	 * We have to reset them manually.
+	 */
+	if (c->x86 < 0x14) {
+		wrmsrl(MSR_K7_PERFCTR0, 0);
+		wrmsrl(MSR_K7_PERFCTR1, 0);
+		wrmsrl(MSR_K7_PERFCTR2, 0);
+		wrmsrl(MSR_K7_PERFCTR3, 0);
+	}
+
 	if (cpuid_edx(0x80000007) & (1 << 10)) {
 		rdmsr(MSR_K7_HWCR, l, h);
 		l |= (1 << 27); /* Enable read-only APERF/MPERF bit */
Signature · 1 line
1.7.4

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#1 lore raw
Quoted 7 lines
Quoted 1 line
Quoted 1 line · Wei Wang
xen/arch/x86/cpu/amd.c +0 -0
@@ -485,6 +485,17 @@ static void __devinit init_amd(struct cpuinfo_x86 *c)
 	if (c->x86 > 0x11)
 		set_bit(X86_FEATURE_ARAT, c->x86_capability);

xen/arch/x86/cpu/amd.c static void __devinit init_amd(struct cpuinfo_x86 *c) · line 497

Quoted 10 lines
xen/arch/x86/cpu/amd.c +10 -0
+	/* 
+	 * Prior to Family 0x14, perf counters are not reset during warm reboot.  
+	 * We have to reset them manually.
+	 */
+	if (c->x86 < 0x14) {
+		wrmsrl(MSR_K7_PERFCTR0, 0);
+		wrmsrl(MSR_K7_PERFCTR1, 0);
+		wrmsrl(MSR_K7_PERFCTR2, 0);
+		wrmsrl(MSR_K7_PERFCTR3, 0);
+	}

This collides with the NMI watchdog setup: smp_callin() calls setup_local_APIC() _before_ calling smp_store_cpu_info(), and hence you write zero again to an MSR possibly already in use. Since setup_k7_watchdog() itself does the clearing of the MSRs in question already, I would think that the most simple adjustment to your patch would be to make the condition

	if (nmi_watchdog != NMI_LOCAL_APIC && c->x86 < 0x14) {

If you agree (and Keir doesn't object), I would commit it that way.

Jan

Quoted 4 lines
xen/arch/x86/cpu/amd.c +1 -0
+
 	if (cpuid_edx(0x80000007) & (1 << 10)) {
 		rdmsr(MSR_K7_HWCR, l, h);
 		l |= (1 << 27); /* Enable read-only APERF/MPERF bit */
Acked-by
#2 lore raw
Quoted 25 lines · Jan Beulich
Quoted 7 lines
Quoted 1 line
Quoted 1 line · Wei Wang
xen/arch/x86/cpu/amd.c +0 -0
@@ -485,6 +485,17 @@ static void __devinit init_amd(struct cpuinfo_x86 *c)
 if (c->x86 > 0x11)
 set_bit(X86_FEATURE_ARAT, c->x86_capability);

xen/arch/x86/cpu/amd.c static void __devinit init_amd(struct cpuinfo_x86 *c) · line 497

Quoted 10 lines
xen/arch/x86/cpu/amd.c +10 -0
+ /* 
+  * Prior to Family 0x14, perf counters are not reset during warm reboot.
+  * We have to reset them manually.
+  */
+ if (c->x86 < 0x14) {
+  wrmsrl(MSR_K7_PERFCTR0, 0);
+  wrmsrl(MSR_K7_PERFCTR1, 0);
+  wrmsrl(MSR_K7_PERFCTR2, 0);
+  wrmsrl(MSR_K7_PERFCTR3, 0);
+ }

This collides with the NMI watchdog setup: smp_callin() calls setup_local_APIC() _before_ calling smp_store_cpu_info(), and hence you write zero again to an MSR possibly already in use. Since setup_k7_watchdog() itself does the clearing of the MSRs in question already, I would think that the most simple adjustment to your patch would be to make the condition

Quoted 3 lines

if (nmi_watchdog != NMI_LOCAL_APIC && c->x86 < 0x14) {

If you agree (and Keir doesn't object), I would commit it that way.

Sounds good.

Acked-by Keir Fraser <[email protected]>
Quoted 8 lines

Jan

Quoted 4 lines
xen/arch/x86/cpu/amd.c +1 -0
+
 if (cpuid_edx(0x80000007) & (1 << 10)) {
 rdmsr(MSR_K7_HWCR, l, h);

l |= (1 << 27); /* Enable read-only APERF/MPERF bit */

author
#3 lore raw
Quoted 27 lines · Keir Fraser
Quoted 25 lines · Jan Beulich
Quoted 7 lines
Quoted 1 line
Quoted 1 line · Wei Wang
xen/arch/x86/cpu/amd.c +0 -0
@@ -485,6 +485,17 @@ static void __devinit init_amd(struct cpuinfo_x86 *c)
 if (c->x86>  0x11)
 set_bit(X86_FEATURE_ARAT, c->x86_capability);

xen/arch/x86/cpu/amd.c static void __devinit init_amd(struct cpuinfo_x86 *c) · line 497

Quoted 10 lines
xen/arch/x86/cpu/amd.c +10 -0
+ /*
+  * Prior to Family 0x14, perf counters are not reset during warm reboot.
+  * We have to reset them manually.
+  */
+ if (c->x86<  0x14) {
+  wrmsrl(MSR_K7_PERFCTR0, 0);
+  wrmsrl(MSR_K7_PERFCTR1, 0);
+  wrmsrl(MSR_K7_PERFCTR2, 0);
+  wrmsrl(MSR_K7_PERFCTR3, 0);
+ }

This collides with the NMI watchdog setup: smp_callin() calls setup_local_APIC() _before_ calling smp_store_cpu_info(), and hence you write zero again to an MSR possibly already in use. Since setup_k7_watchdog() itself does the clearing of the MSRs in question already, I would think that the most simple adjustment to your patch would be to make the condition

Quoted 7 lines
Quoted 3 lines

if (nmi_watchdog != NMI_LOCAL_APIC&& c->x86< 0x14) {

If you agree (and Keir doesn't object), I would commit it that way.

Sounds good.

Acked-by Keir Fraser<[email protected]>

Sounds good to me too. Acked
Thanks,
Wei

Quoted 11 lines
Quoted 8 lines

Jan

Quoted 4 lines
xen/arch/x86/cpu/amd.c +1 -0
+
 if (cpuid_edx(0x80000007)&  (1<<  10)) {
 rdmsr(MSR_K7_HWCR, l, h);

l |= (1<< 27); /* Enable read-only APERF/MPERF bit */