[PATCH] Add Xeon 7500 series support to oprofile

[PATCH] Add Xeon 7500 series support to oprofile

[Andi Kleen]

Add Xeon 7500 series support to oprofile

Straight forward: it's the same as Core i7, so just detect
the model number. No user space changes needed.

Very simple patch, so it could be still merged for .33?

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 arch/x86/oprofile/nmi_int.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux/arch/x86/oprofile/nmi_int.c
===================================================================
--- linux.orig/arch/x86/oprofile/nmi_int.c
+++ linux/arch/x86/oprofile/nmi_int.c
@@ -598,6 +598,7 @@ static int __init ppro_init(char **cpu_t
 	case 15: case 23:
 		*cpu_type = "i386/core_2";
 		break;
+	case 0x2e:
 	case 26:
 		spec = &op_arch_perfmon_spec;
 		*cpu_type = "i386/core_i7";

Re: [PATCH] Add Xeon 7500 series support to oprofile

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

On Thu, 21 Jan 2010 23:26:27 +0100, Andi Kleen said:
> Add Xeon 7500 series support to oprofile
> 
> Straight forward: it's the same as Core i7, so just detect
> the model number. No user space changes needed.
> 
> Very simple patch, so it could be still merged for .33?
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
>  arch/x86/oprofile/nmi_int.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux/arch/x86/oprofile/nmi_int.c
> ===================================================================
> --- linux.orig/arch/x86/oprofile/nmi_int.c
> +++ linux/arch/x86/oprofile/nmi_int.c
> @@ -598,6 +598,7 @@ static int __init ppro_init(char **cpu_t
>  	case 15: case 23:
>  		*cpu_type = "i386/core_2";
>  		break;
> +	case 0x2e:
>  	case 26:
>  		spec = &op_arch_perfmon_spec;
>  		*cpu_type = "i386/core_i7";


I'll bite - why a hex constant rather than the decimal values nearby?

Also, should we do something about *cpu_type so it isn't confusing on a 7500
reporting itself as a i7?


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH] Add Xeon 7500 series support to oprofile

[Andi Kleen]

> 
> I'll bite - why a hex constant rather than the decimal values nearby?

Why not?

> 
> Also, should we do something about *cpu_type so it isn't confusing on a 7500
> reporting itself as a i7?

No, that wouldn't be compatible to oprofile userland.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] Add Xeon 7500 series support to oprofile

[John Villalovos]

On Thu, Jan 21, 2010 at 5:26 PM, Andi Kleen <andi@firstfloor.org> wrote:
> Add Xeon 7500 series support to oprofile
>
> Straight forward: it's the same as Core i7, so just detect
> the model number. No user space changes needed.
>
> Very simple patch, so it could be still merged for .33?
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> ---
>  arch/x86/oprofile/nmi_int.c |    1 +
>  1 file changed, 1 insertion(+)
>
> Index: linux/arch/x86/oprofile/nmi_int.c
> ===================================================================
> --- linux.orig/arch/x86/oprofile/nmi_int.c
> +++ linux/arch/x86/oprofile/nmi_int.c
> @@ -598,6 +598,7 @@ static int __init ppro_init(char **cpu_t
>        case 15: case 23:
>                *cpu_type = "i386/core_2";
>                break;
> +       case 0x2e:
>        case 26:
>                spec = &op_arch_perfmon_spec;
>                *cpu_type = "i386/core_i7";

How about: this instead?

Signed-off-by: John L. Villalovos <john.l.villalovos@intel.com>

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index cb88b1a..edc074c 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -598,7 +598,7 @@ static int __init ppro_init(char **cpu_type)
 	case 15: case 23:
 		*cpu_type = "i386/core_2";
 		break;
-	case 26:
+	case 26: case 46:
 		spec = &op_arch_perfmon_spec;
 		*cpu_type = "i386/core_i7";
 		break;

Re: [PATCH] Add Xeon 7500 series support to oprofile

[Robert Richter]

On 22.01.10 11:23:08, John Villalovos wrote:
> diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
> index cb88b1a..edc074c 100644
> --- a/arch/x86/oprofile/nmi_int.c
> +++ b/arch/x86/oprofile/nmi_int.c
> @@ -598,7 +598,7 @@ static int __init ppro_init(char **cpu_type)
>  	case 15: case 23:
>  		*cpu_type = "i386/core_2";
>  		break;
> -	case 26:
> +	case 26: case 46:

Hex values fit better here since the spec is using bit values
too. Instead, the code should be changed to only use hex values for
models with extended model bits set.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH] Add Xeon 7500 series support to oprofile

[Robert Richter]

On 21.01.10 23:26:27, Andi Kleen wrote:
> Add Xeon 7500 series support to oprofile
> 
> Straight forward: it's the same as Core i7, so just detect
> the model number. No user space changes needed.
> 
> Very simple patch, so it could be still merged for .33?

Patch applied to oprofile/urgent. I will try to merge it for .33.

I would like to see a follow on patch that changes all x86_model
values to hex that have the extended model bit set. This would make
the code more readable since the spec is also using bit values for
this.

Thanks.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH] Add Xeon 7500 series support to oprofile

[Andi Kleen]

> Patch applied to oprofile/urgent. I will try to merge it for .33.

Thanks.

> 
> I would like to see a follow on patch that changes all x86_model
> values to hex that have the extended model bit set. This would make
> the code more readable since the spec is also using bit values for
> this.

Ok, makes sense.  I'll send it later.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.