Dietmar Hahn
2012-Nov-26 12:52 UTC
[PATCH] 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 <dietmar.hahn@ts.fujitsu.com>
diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c
--- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000
+++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100
@@ -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;
--
Company details: http://ts.fujitsu.com/imprint.html
Jan Beulich
2012-Nov-26 13:39 UTC
Re: [PATCH] oprofile: Add X7542 and E7-8837 to the list of supported cpus
>>> On 26.11.12 at 13:52, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: > 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> Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> > > diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c > --- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 > +++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 > @@ -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; > > > -- > Company details: http://ts.fujitsu.com/imprint.html > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Dietmar Hahn
2012-Nov-26 14:21 UTC
Re: [PATCH] oprofile: Add X7542 and E7-8837 to the list of supported cpus
Am Montag 26 November 2012, 13:39:20 schrieb Jan Beulich:> >>> On 26.11.12 at 13:52, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: > > 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.> 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.> > Jan > > > Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> > > > > diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c > > --- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 > > +++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 > > @@ -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 > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >-- Company details: http://ts.fujitsu.com/imprint.html
Jan Beulich
2012-Nov-27 13:04 UTC
Re: [PATCH] oprofile: Add X7542 and E7-8837 to the list of supported cpus
>>> On 26.11.12 at 13:52, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: > Add intel cpus X7542 and E7-8837 to the list of supported cpus. > > Thanks. > Dietmar. > > Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> > > diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c > --- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 > +++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 > @@ -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-Nov-27 13:19 UTC
Re: [PATCH] oprofile: Add X7542 and E7-8837 to the list of supported cpus
>>> On 27.11.12 at 14:04, "Jan Beulich" <JBeulich@suse.com> wrote: >>>> On 26.11.12 at 13:52, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: >> Add intel cpus X7542 and E7-8837 to the list of supported cpus. >> >> Thanks. >> Dietmar. >> >> Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> >> >> diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c >> --- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 >> +++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 >> @@ -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()?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 --- a/xen/arch/x86/oprofile/nmi_int.c +++ b/xen/arch/x86/oprofile/nmi_int.c @@ -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
2012-Nov-27 14:13 UTC
Re: [PATCH] oprofile: Add X7542 and E7-8837 to the list of supported cpus
Am Dienstag 27 November 2012, 13:04:36 schrieb Jan Beulich:> >>> On 26.11.12 at 13:52, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: > > Add intel cpus X7542 and E7-8837 to the list of supported cpus. > > > > Thanks. > > Dietmar. > > > > Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> > > > > diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c > > --- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 > > +++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 > > @@ -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()?Yes you are right. I wasn''t aware of this! Dietmar.> > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >-- Company details: http://ts.fujitsu.com/imprint.html
Dietmar Hahn
2012-Nov-27 14:21 UTC
Re: [PATCH] oprofile: Add X7542 and E7-8837 to the list of supported cpus
Am Dienstag 27 November 2012, 13:19:34 schrieb Jan Beulich:> >>> On 27.11.12 at 14:04, "Jan Beulich" <JBeulich@suse.com> wrote: > >>>> On 26.11.12 at 13:52, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: > >> Add intel cpus X7542 and E7-8837 to the list of supported cpus. > >> > >> Thanks. > >> Dietmar. > >> > >> Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> > >> > >> diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c > >> --- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 > >> +++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 > >> @@ -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()? > > 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.> > Jan > > --- a/xen/arch/x86/oprofile/nmi_int.c > +++ b/xen/arch/x86/oprofile/nmi_int.c > @@ -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-Nov-27 15:46 UTC
Re: [PATCH] oprofile: Add X7542 and E7-8837 to the list of supported cpus
>>> On 27.11.12 at 15:21, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: > Am Dienstag 27 November 2012, 13:19:34 schrieb Jan Beulich: >> >>> On 27.11.12 at 14:04, "Jan Beulich" <JBeulich@suse.com> wrote: >> >>>> On 26.11.12 at 13:52, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: >> >> Add intel cpus X7542 and E7-8837 to the list of supported cpus. >> >> >> >> Thanks. >> >> Dietmar. >> >> >> >> Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> >> >> >> >> diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c >> >> --- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 >> >> +++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 >> >> @@ -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()? >> >> 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
2012-Dec-04 14:36 UTC
Re: [PATCH] oprofile: Add X7542 and E7-8837 to the list of supported cpus
Am Dienstag 27 November 2012, 15:46:41 schrieb Jan Beulich:> >>> On 27.11.12 at 15:21, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: > > Am Dienstag 27 November 2012, 13:19:34 schrieb Jan Beulich: > >> >>> On 27.11.12 at 14:04, "Jan Beulich" <JBeulich@suse.com> wrote: > >> >>>> On 26.11.12 at 13:52, Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> wrote: > >> >> Add intel cpus X7542 and E7-8837 to the list of supported cpus. > >> >> > >> >> Thanks. > >> >> Dietmar. > >> >> > >> >> Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com> > >> >> > >> >> diff -r 0049de3827bc -r 6fb0129600cd xen/arch/x86/oprofile/nmi_int.c > >> >> --- a/xen/arch/x86/oprofile/nmi_int.c Fri Nov 23 11:06:15 2012 +0000 > >> >> +++ b/xen/arch/x86/oprofile/nmi_int.c Mon Nov 26 13:36:00 2012 +0100 > >> >> @@ -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()? > >> > >> 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.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.> Jan > >-- Company details: http://ts.fujitsu.com/imprint.html