oprofile: Add X7542 and E7-8837 to the list of supported cpus

PATCH In review Dietmar Hahn 2 participants 9 messages Nov 26, 2012 – Dec 4, 2012

lore

Share & tools

lore

Dietmar Hahn

author

2012-11-26 12:52 UTC #0 lore raw

PATCH 1/1 oprofile: Add X7542 and E7-8837 to the list of supported cpus

Add intel cpus X7542 and E7-8837 to the list of supported cpus.

Thanks.
Dietmar.

Signed-off-by Dietmar Hahn <[email protected]>

xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0

diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c
@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;

Signature · 1 line

Company details: http://ts.fujitsu.com/imprint.html

Jan Beulich

2012-11-26 13:39 UTC #1 lore raw

Quoted 2 lines

Quoted 1 line
Quoted 1 line · Dietmar Hahn
Add intel cpus X7542 and E7-8837 to the list of supported cpus.

I suppose these two random additions originate from whatever you happen to run your code on? To be honest, I'd prefer the list here to actually be put into proper shape...

Jan

Quoted 23 lines

Signed-off-by Dietmar Hahn <[email protected]>

diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c

xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0

@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;

Signature · 7 lines

-- 
Company details: http://ts.fujitsu.com/imprint.html 

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

Dietmar Hahn

author

2012-11-26 14:21 UTC #2 lore raw

Am Montag 26 November 2012, 13:39:20 schrieb Jan Beulich:

Quoted 5 lines

Quoted 2 lines
Quoted 1 line
Quoted 1 line · Dietmar Hahn
Add intel cpus X7542 and E7-8837 to the list of supported cpus.
I suppose these two random additions originate from whatever you happen to run your code on?

Yes. The machines I can test this.

Quoted 2 lines

To be honest, I'd prefer the list
here to actually be put into proper shape...

Here I must look at the specs or see in vpmu parts. Dietmar.

Quoted 28 lines

Jan
Quoted 8 lines
Signed-off-by Dietmar Hahn <[email protected]>
diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 static int __init ppro_init(char ** cpu_ · line 373
Quoted 8 lines
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0
@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Signature · 1 line

Company details: http://ts.fujitsu.com/imprint.html

Jan Beulich

2012-11-27 13:04 UTC #3 lore raw

In reply to Dietmar Hahn

Quoted 12 lines

Quoted 1 line
Quoted 1 line · Dietmar Hahn
Add intel cpus X7542 and E7-8837 to the list of supported cpus.
Thanks.
Dietmar.
Signed-off-by Dietmar Hahn <[email protected]>
diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c

xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 static int __init ppro_init(char ** cpu_ · line 373

Quoted 8 lines

xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0

@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;

Actually, and apart from the patch being white space damaged, after a closer look I think this is wrong - these newer CPUs shouldn't get be handled here, but instead should be covered by arch_perfmon_init(). Are you observing X86_FEATURE_ARCH_PERFMON not getting set on these CPUs by init_intel()?

Jan

Jan Beulich

2012-11-27 13:19 UTC #4 lore raw

Quoted 19 lines

Quoted 19 lines
Quoted 2 lines · Jan Beulich
Quoted 1 line · Dietmar Hahn
Add intel cpus X7542 and E7-8837 to the list of supported cpus.
Thanks.
Dietmar.
Signed-off-by Dietmar Hahn <[email protected]>
diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0
@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();

Quoted 8 lines

xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 static int __init ppro_init(char ** cpu_ · line 373
Quoted 2 lines
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +0 -0
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;
Actually, and apart from the patch being white space damaged, after a closer look I think this is wrong - these newer CPUs shouldn't get be handled here, but instead should be covered by arch_perfmon_init(). Are you observing X86_FEATURE_ARCH_PERFMON not getting set on these CPUs by init_intel()?

I.e. the below would be the patch I'd expect when merely taking the SDM as reference (with the "todo remove?" ones also fully removed of course).

Jan

xen/arch/x86/oprofile/nmi_int.c +9 ~~-21~~

@@ -342,30 +342,22 @@ static int __init ppro_init(char ** cpu_
 		return 0;
 
 	switch (cpu_model) {
-	case 0 ... 2:
-		*cpu_type = "i386/ppro";
-		break;
-	case 3 ... 5:
-		*cpu_type = "i386/pii";
-		break;
-	case 6 ... 8:
-	case 10 ... 11:
-		*cpu_type = "i386/piii";
-		break;
-	case 9:
-	case 13:
-		*cpu_type = "i386/p6_mobile";
-		break;
 	case 14:
 		*cpu_type = "i386/core";
 		break;
 	case 15:
+#if 0//todo remove?
 	case 23:
 	case 29:
+#endif//todo
 		*cpu_type = "i386/core_2";
 		ppro_has_global_ctrl = 1;
 		break;
-	case 26:
+#if 0//todo remove?
+	/* Nehalem */
+	case 26: case 30: case 31: case 46:
+	/ Westmere */
+	case 37: case 44: case 47:
 		arch_perfmon_setup_counters();
 		*cpu_type = "i386/core_i7";
 		ppro_has_global_ctrl = 1;

@@ -373,6 +365,7 @@ static int __init ppro_init(char ** cpu_
 	case 28:
 		*cpu_type = "i386/atom";
 		break;
+#endif//todo
 	default:
 		/* Unknown */
 		return 0;

@@ -389,6 +382,7 @@ static int __init arch_perfmon_init(char
 	*cpu_type = "i386/arch_perfmon";
 	model = &op_arch_perfmon_spec;
 	arch_perfmon_setup_counters();
+	ppro_has_global_ctrl = 1;
 	return 1;
 }

@@ -413,14 +407,8 @@ static int __init nmi_init(void)
 				       "AMD processor family %d is not "
 				       "supported\n", family);
 				return -ENODEV;
-			case 6:
-				model = &op_athlon_spec;
-				cpu_type = "i386/athlon";
-				break;
 			case 0xf:
 				model = &op_athlon_spec;
-				/* Actually it could be i386/hammer too, but
-				   give user space an consistent name. */
 				cpu_type = "x86-64/hammer";
 				break;
 			case 0x10:

Dietmar Hahn

author

2012-11-27 14:21 UTC #5 lore raw

Am Dienstag 27 November 2012, 13:19:34 schrieb Jan Beulich:

Quoted 28 lines

Quoted 19 lines
Quoted 19 lines
Quoted 2 lines · Jan Beulich
Quoted 1 line · Dietmar Hahn
Add intel cpus X7542 and E7-8837 to the list of supported cpus.
Thanks.
Dietmar.
Signed-off-by Dietmar Hahn <[email protected]>
diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0
@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();
Quoted 8 lines
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 static int __init ppro_init(char ** cpu_ · line 373
Quoted 2 lines
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +0 -0
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;
Actually, and apart from the patch being white space damaged, after a closer look I think this is wrong - these newer CPUs shouldn't get be handled here, but instead should be covered by arch_perfmon_init(). Are you observing X86_FEATURE_ARCH_PERFMON not getting set on these CPUs by init_intel()?

Quoted 3 lines

I.e. the below would be the patch I'd expect when merely taking the SDM as reference (with the "todo remove?" ones also fully removed of course).

Yes, looks much cleaner.
Thanks.
Dietmar.

Quoted 75 lines

Jan

xen/arch/x86/oprofile/nmi_int.c +9 ~~-21~~

@@ -342,30 +342,22 @@ static int __init ppro_init(char ** cpu_
 		return 0;
 
 	switch (cpu_model) {
-	case 0 ... 2:
-		*cpu_type = "i386/ppro";
-		break;
-	case 3 ... 5:
-		*cpu_type = "i386/pii";
-		break;
-	case 6 ... 8:
-	case 10 ... 11:
-		*cpu_type = "i386/piii";
-		break;
-	case 9:
-	case 13:
-		*cpu_type = "i386/p6_mobile";
-		break;
 	case 14:
 		*cpu_type = "i386/core";
 		break;
 	case 15:
+#if 0//todo remove?
 	case 23:
 	case 29:
+#endif//todo
 		*cpu_type = "i386/core_2";
 		ppro_has_global_ctrl = 1;
 		break;
-	case 26:
+#if 0//todo remove?
+	/* Nehalem */
+	case 26: case 30: case 31: case 46:
+	/ Westmere */
+	case 37: case 44: case 47:
 		arch_perfmon_setup_counters();
 		*cpu_type = "i386/core_i7";
 		ppro_has_global_ctrl = 1;

@@ -373,6 +365,7 @@ static int __init ppro_init(char ** cpu_
 	case 28:
 		*cpu_type = "i386/atom";
 		break;
+#endif//todo
 	default:
 		/* Unknown */
 		return 0;

@@ -389,6 +382,7 @@ static int __init arch_perfmon_init(char
 	*cpu_type = "i386/arch_perfmon";
 	model = &op_arch_perfmon_spec;
 	arch_perfmon_setup_counters();
+	ppro_has_global_ctrl = 1;
 	return 1;
 }

@@ -413,14 +407,8 @@ static int __init nmi_init(void)
 				       "AMD processor family %d is not "
 				       "supported\n", family);
 				return -ENODEV;
-			case 6:
-				model = &op_athlon_spec;
-				cpu_type = "i386/athlon";
-				break;
 			case 0xf:
 				model = &op_athlon_spec;
-				/* Actually it could be i386/hammer too, but
-				   give user space an consistent name. */
 				cpu_type = "x86-64/hammer";
 				break;
 			case 0x10:

Jan Beulich

2012-11-27 15:46 UTC #6 lore raw

Quoted 30 lines

Quoted 1 line
Quoted 1 line · Dietmar Hahn
Am Dienstag 27 November 2012, 13:19:34 schrieb Jan Beulich:
Quoted 28 lines
Quoted 27 lines
Quoted 13 lines
Quoted 2 lines · Jan Beulich
Quoted 1 line · Dietmar Hahn
Add intel cpus X7542 and E7-8837 to the list of supported cpus.
Thanks.
Dietmar.
Signed-off-by Dietmar Hahn <[email protected]>
diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 static int __init ppro_init(char ** cpu_ · line 373
Quoted 8 lines
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0
@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;
Actually, and apart from the patch being white space damaged, after a closer look I think this is wrong - these newer CPUs shouldn't get be handled here, but instead should be covered by arch_perfmon_init(). Are you observing X86_FEATURE_ARCH_PERFMON not getting set on these CPUs by init_intel()?

Quoted 5 lines

Quoted 3 lines
I.e. the below would be the patch I'd expect when merely taking the SDM as reference (with the "todo remove?" ones also fully removed of course).
Yes, looks much cleaner.

Question is - does it work for you? And if it does, why would it not have worked without any change? After all, the patch in this form, apart from the setting of ppro_has_global_ctrl in arch_perfmon_init(), only removes code.

Jan

Dietmar Hahn

author

2012-12-04 14:36 UTC #7 lore raw

Am Dienstag 27 November 2012, 15:46:41 schrieb Jan Beulich:

Quoted 34 lines

Quoted 34 lines
Quoted 1 line
Quoted 1 line · Dietmar Hahn
Am Dienstag 27 November 2012, 13:19:34 schrieb Jan Beulich:
Quoted 31 lines
Quoted 19 lines
Quoted 19 lines
Quoted 2 lines · Jan Beulich
Quoted 1 line · Dietmar Hahn
Add intel cpus X7542 and E7-8837 to the list of supported cpus.
Thanks.
Dietmar.
Signed-off-by Dietmar Hahn <[email protected]>
diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0
@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();
Quoted 8 lines
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 static int __init ppro_init(char ** cpu_ · line 373
Quoted 2 lines
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +0 -0
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;
Actually, and apart from the patch being white space damaged, after a closer look I think this is wrong - these newer CPUs shouldn't get be handled here, but instead should be covered by arch_perfmon_init(). Are you observing X86_FEATURE_ARCH_PERFMON not getting set on these CPUs by init_intel()?
I.e. the below would be the patch I'd expect when merely taking the SDM as reference (with the "todo remove?" ones also fully removed of course).

Quoted 6 lines

Quoted 1 line
Yes, looks much cleaner.
Question is - does it work for you? And if it does, why would it not have worked without any change? After all, the patch in this form, apart from the setting of ppro_has_global_ctrl in arch_perfmon_init(), only removes code.

After one week of vacation I can't reproduce the thing.
I only remember that trying oprofile didn't work on my machine and the problem went away after the fix. But now I looked deeper and see that something else must have been wrong I cant reproduce. Sorry I didn't think enough! Please forget my patch and thank you for better looking than me!

Dietmar.

Quoted 3 lines

Jan

Signature · 1 line

Company details: http://ts.fujitsu.com/imprint.html

Dietmar Hahn

author

2012-11-27 14:13 UTC #8 lore raw

In reply to Jan Beulich

Am Dienstag 27 November 2012, 13:04:36 schrieb Jan Beulich:

Quoted 18 lines

Quoted 18 lines
Quoted 1 line
Quoted 1 line · Dietmar Hahn
Add intel cpus X7542 and E7-8837 to the list of supported cpus.
Thanks.
Dietmar.
Signed-off-by Dietmar Hahn <[email protected]>
diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +2 -0
@@ -366,6 +366,8 @@ static int __init ppro_init(char ** cpu_
                ppro_has_global_ctrl = 1;
                break;
        case 26:
+       case 46:
+       case 47:
                arch_perfmon_setup_counters();

Quoted 8 lines

xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 static int __init ppro_init(char ** cpu_ · line 373
Quoted 2 lines
xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 renamed from xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 +0 -0
                *cpu_type = "i386/core_i7";
                ppro_has_global_ctrl = 1;
Actually, and apart from the patch being white space damaged, after a closer look I think this is wrong - these newer CPUs shouldn't get be handled here, but instead should be covered by arch_perfmon_init(). Are you observing X86_FEATURE_ARCH_PERFMON not getting set on these CPUs by init_intel()?

Yes you are right. I wasn't aware of this!
Dietmar.

Quoted 9 lines

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

Signature · 1 line

Company details: http://ts.fujitsu.com/imprint.html