diff mbox

[v4,03/14] ARM: Factor out cpuid implementor and part number

Message ID 20121110154231.2836.24377.stgit@chazy-air (mailing list archive)
State New, archived
Headers show

Commit Message

Christoffer Dall Nov. 10, 2012, 3:42 p.m. UTC
Decoding the implementor and part number of the CPU id in the CPU ID
register is needed by KVM, so we factor it out to share the code.

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
---
 arch/arm/include/asm/cputype.h   |   26 ++++++++++++++++++++++++++
 arch/arm/kernel/perf_event_cpu.c |   30 +++++++++++++++---------------
 2 files changed, 41 insertions(+), 15 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Will Deacon Nov. 19, 2012, 2:21 p.m. UTC | #1
On Sat, Nov 10, 2012 at 03:42:31PM +0000, Christoffer Dall wrote:
> Decoding the implementor and part number of the CPU id in the CPU ID
> register is needed by KVM, so we factor it out to share the code.
> 
> Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>

[...]

> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
> index cb47d28..306fb2c 100644
> --- a/arch/arm/include/asm/cputype.h
> +++ b/arch/arm/include/asm/cputype.h
> @@ -51,6 +51,22 @@ extern unsigned int processor_id;
>  #define read_cpuid_ext(reg) 0
>  #endif
>  
> +#define IMPLEMENTOR_ARM		0x41
> +#define IMPLEMENTOR_INTEL	0x69
> +
> +#define PART_NUMBER_ARM1136	0xB360
> +#define PART_NUMBER_ARM1156	0xB560
> +#define PART_NUMBER_ARM1176	0xB760
> +#define PART_NUMBER_ARM11MPCORE	0xB020
> +#define PART_NUMBER_CORTEX_A8	0xC080
> +#define PART_NUMBER_CORTEX_A9 	0xC090
> +#define PART_NUMBER_CORTEX_A5 	0xC050
> +#define PART_NUMBER_CORTEX_A15	0xC0F0
> +#define PART_NUMBER_CORTEX_A7	0xC070
> +
> +#define PART_NUMBER_XSCALE1	0x1
> +#define PART_NUMBER_XSCALE2	0x2

We should probably prefix these with ARM_CPU_ and make the current names
shorter to compensate. e.g. ARM_CPU_PART_1136, ARM_CPU_IMP_ARM ?

>  /*
>   * The CPU ID never changes at run time, so we might as well tell the
>   * compiler that it's constant.  Use this function to read the CPU ID
> @@ -61,6 +77,16 @@ static inline unsigned int __attribute_const__ read_cpuid_id(void)
>  	return read_cpuid(CPUID_ID);
>  }
>  
> +static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
> +{
> +	return (read_cpuid_id() & 0xFF000000) >> 24;
> +}
> +
> +static inline unsigned int __attribute_const__ read_cpuid_part_number(void)
> +{
> +	return (read_cpuid_id() & 0xFFF0);
> +}

Perhaps this should take the implementor as an argument, given that the
part number is described differently between implementors. The xscale
stuff can then move in here (we'll need to check the xscale docs in case
perf is using a subfield -- I can't remember off-hand).

>  static inline unsigned int __attribute_const__ read_cpuid_cachetype(void)
>  {
>  	return read_cpuid(CPUID_CACHETYPE);
> diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> index 8d7d8d4..ff18566 100644
> --- a/arch/arm/kernel/perf_event_cpu.c
> +++ b/arch/arm/kernel/perf_event_cpu.c
> @@ -200,46 +200,46 @@ static struct arm_pmu *__devinit probe_current_pmu(void)
>  	struct arm_pmu *pmu = NULL;
>  	int cpu = get_cpu();
>  	unsigned long cpuid = read_cpuid_id();
> -	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
> -	unsigned long part_number = (cpuid & 0xFFF0);
> +	unsigned long implementor = read_cpuid_implementor();
> +	unsigned long part_number = read_cpuid_part_number();
>  
>  	pr_info("probing PMU on CPU %d\n", cpu);
>  
>  	/* ARM Ltd CPUs. */
> -	if (0x41 == implementor) {
> +	if (implementor == IMPLEMENTOR_ARM) {
>  		switch (part_number) {
> -		case 0xB360:	/* ARM1136 */
> -		case 0xB560:	/* ARM1156 */
> -		case 0xB760:	/* ARM1176 */
> +		case PART_NUMBER_ARM1136:
> +		case PART_NUMBER_ARM1156:
> +		case PART_NUMBER_ARM1176:
>  			pmu = armv6pmu_init();
>  			break;
> -		case 0xB020:	/* ARM11mpcore */
> +		case PART_NUMBER_ARM11MPCORE:
>  			pmu = armv6mpcore_pmu_init();
>  			break;
> -		case 0xC080:	/* Cortex-A8 */
> +		case PART_NUMBER_CORTEX_A8:
>  			pmu = armv7_a8_pmu_init();
>  			break;
> -		case 0xC090:	/* Cortex-A9 */
> +		case PART_NUMBER_CORTEX_A9:
>  			pmu = armv7_a9_pmu_init();
>  			break;
> -		case 0xC050:	/* Cortex-A5 */
> +		case PART_NUMBER_CORTEX_A5:
>  			pmu = armv7_a5_pmu_init();
>  			break;
> -		case 0xC0F0:	/* Cortex-A15 */
> +		case PART_NUMBER_CORTEX_A15:
>  			pmu = armv7_a15_pmu_init();
>  			break;
> -		case 0xC070:	/* Cortex-A7 */
> +		case PART_NUMBER_CORTEX_A7:
>  			pmu = armv7_a7_pmu_init();
>  			break;
>  		}
>  	/* Intel CPUs [xscale]. */
> -	} else if (0x69 == implementor) {
> +	} else if (implementor == IMPLEMENTOR_INTEL) {
>  		part_number = (cpuid >> 13) & 0x7;
>  		switch (part_number) {
> -		case 1:
> +		case PART_NUMBER_XSCALE1:
>  			pmu = xscale1pmu_init();
>  			break;
> -		case 2:
> +		case PART_NUMBER_XSCALE2:
>  			pmu = xscale2pmu_init();
>  			break;
>  		}

If you stick this one in a separate patch, I can take it via the perf
tree (along with the CPUID rework above).

Will
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoffer Dall Nov. 29, 2012, 9:38 p.m. UTC | #2
On Mon, Nov 19, 2012 at 9:21 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Sat, Nov 10, 2012 at 03:42:31PM +0000, Christoffer Dall wrote:
>> Decoding the implementor and part number of the CPU id in the CPU ID
>> register is needed by KVM, so we factor it out to share the code.
>>
>> Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
>> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>
> [...]
>
>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>> index cb47d28..306fb2c 100644
>> --- a/arch/arm/include/asm/cputype.h
>> +++ b/arch/arm/include/asm/cputype.h
>> @@ -51,6 +51,22 @@ extern unsigned int processor_id;
>>  #define read_cpuid_ext(reg) 0
>>  #endif
>>
>> +#define IMPLEMENTOR_ARM              0x41
>> +#define IMPLEMENTOR_INTEL    0x69
>> +
>> +#define PART_NUMBER_ARM1136  0xB360
>> +#define PART_NUMBER_ARM1156  0xB560
>> +#define PART_NUMBER_ARM1176  0xB760
>> +#define PART_NUMBER_ARM11MPCORE      0xB020
>> +#define PART_NUMBER_CORTEX_A8        0xC080
>> +#define PART_NUMBER_CORTEX_A9        0xC090
>> +#define PART_NUMBER_CORTEX_A5        0xC050
>> +#define PART_NUMBER_CORTEX_A15       0xC0F0
>> +#define PART_NUMBER_CORTEX_A7        0xC070
>> +
>> +#define PART_NUMBER_XSCALE1  0x1
>> +#define PART_NUMBER_XSCALE2  0x2
>
> We should probably prefix these with ARM_CPU_ and make the current names
> shorter to compensate. e.g. ARM_CPU_PART_1136, ARM_CPU_IMP_ARM ?
>
>>  /*
>>   * The CPU ID never changes at run time, so we might as well tell the
>>   * compiler that it's constant.  Use this function to read the CPU ID
>> @@ -61,6 +77,16 @@ static inline unsigned int __attribute_const__ read_cpuid_id(void)
>>       return read_cpuid(CPUID_ID);
>>  }
>>
>> +static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
>> +{
>> +     return (read_cpuid_id() & 0xFF000000) >> 24;
>> +}
>> +
>> +static inline unsigned int __attribute_const__ read_cpuid_part_number(void)
>> +{
>> +     return (read_cpuid_id() & 0xFFF0);
>> +}
>
> Perhaps this should take the implementor as an argument, given that the
> part number is described differently between implementors. The xscale
> stuff can then move in here (we'll need to check the xscale docs in case
> perf is using a subfield -- I can't remember off-hand).
>
>>  static inline unsigned int __attribute_const__ read_cpuid_cachetype(void)
>>  {
>>       return read_cpuid(CPUID_CACHETYPE);
>> diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
>> index 8d7d8d4..ff18566 100644
>> --- a/arch/arm/kernel/perf_event_cpu.c
>> +++ b/arch/arm/kernel/perf_event_cpu.c
>> @@ -200,46 +200,46 @@ static struct arm_pmu *__devinit probe_current_pmu(void)
>>       struct arm_pmu *pmu = NULL;
>>       int cpu = get_cpu();
>>       unsigned long cpuid = read_cpuid_id();
>> -     unsigned long implementor = (cpuid & 0xFF000000) >> 24;
>> -     unsigned long part_number = (cpuid & 0xFFF0);
>> +     unsigned long implementor = read_cpuid_implementor();
>> +     unsigned long part_number = read_cpuid_part_number();
>>
>>       pr_info("probing PMU on CPU %d\n", cpu);
>>
>>       /* ARM Ltd CPUs. */
>> -     if (0x41 == implementor) {
>> +     if (implementor == IMPLEMENTOR_ARM) {
>>               switch (part_number) {
>> -             case 0xB360:    /* ARM1136 */
>> -             case 0xB560:    /* ARM1156 */
>> -             case 0xB760:    /* ARM1176 */
>> +             case PART_NUMBER_ARM1136:
>> +             case PART_NUMBER_ARM1156:
>> +             case PART_NUMBER_ARM1176:
>>                       pmu = armv6pmu_init();
>>                       break;
>> -             case 0xB020:    /* ARM11mpcore */
>> +             case PART_NUMBER_ARM11MPCORE:
>>                       pmu = armv6mpcore_pmu_init();
>>                       break;
>> -             case 0xC080:    /* Cortex-A8 */
>> +             case PART_NUMBER_CORTEX_A8:
>>                       pmu = armv7_a8_pmu_init();
>>                       break;
>> -             case 0xC090:    /* Cortex-A9 */
>> +             case PART_NUMBER_CORTEX_A9:
>>                       pmu = armv7_a9_pmu_init();
>>                       break;
>> -             case 0xC050:    /* Cortex-A5 */
>> +             case PART_NUMBER_CORTEX_A5:
>>                       pmu = armv7_a5_pmu_init();
>>                       break;
>> -             case 0xC0F0:    /* Cortex-A15 */
>> +             case PART_NUMBER_CORTEX_A15:
>>                       pmu = armv7_a15_pmu_init();
>>                       break;
>> -             case 0xC070:    /* Cortex-A7 */
>> +             case PART_NUMBER_CORTEX_A7:
>>                       pmu = armv7_a7_pmu_init();
>>                       break;
>>               }
>>       /* Intel CPUs [xscale]. */
>> -     } else if (0x69 == implementor) {
>> +     } else if (implementor == IMPLEMENTOR_INTEL) {
>>               part_number = (cpuid >> 13) & 0x7;
>>               switch (part_number) {
>> -             case 1:
>> +             case PART_NUMBER_XSCALE1:
>>                       pmu = xscale1pmu_init();
>>                       break;
>> -             case 2:
>> +             case PART_NUMBER_XSCALE2:
>>                       pmu = xscale2pmu_init();
>>                       break;
>>               }
>
> If you stick this one in a separate patch, I can take it via the perf
> tree (along with the CPUID rework above).
>
thanks,
I sent a separate patch.

-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Will Deacon Nov. 30, 2012, 10:21 a.m. UTC | #3
On Thu, Nov 29, 2012 at 09:38:46PM +0000, Christoffer Dall wrote:
> On Mon, Nov 19, 2012 at 9:21 AM, Will Deacon <will.deacon@arm.com> wrote:
> > On Sat, Nov 10, 2012 at 03:42:31PM +0000, Christoffer Dall wrote:
> >> Decoding the implementor and part number of the CPU id in the CPU ID
> >> register is needed by KVM, so we factor it out to share the code.
> >>
> >> Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
> >> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>

[...]

> >> +static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
> >> +{
> >> +     return (read_cpuid_id() & 0xFF000000) >> 24;
> >> +}
> >> +
> >> +static inline unsigned int __attribute_const__ read_cpuid_part_number(void)
> >> +{
> >> +     return (read_cpuid_id() & 0xFFF0);
> >> +}
> >
> > Perhaps this should take the implementor as an argument, given that the
> > part number is described differently between implementors. The xscale
> > stuff can then move in here (we'll need to check the xscale docs in case
> > perf is using a subfield -- I can't remember off-hand).

[...]

> > If you stick this one in a separate patch, I can take it via the perf
> > tree (along with the CPUID rework above).
> >
> thanks,
> I sent a separate patch.

Looks like we still have the ugly xscale cpuid parsing inline. Could you
move it as I suggested, please?

Will
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoffer Dall Nov. 30, 2012, 3:42 p.m. UTC | #4
On Fri, Nov 30, 2012 at 5:21 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Nov 29, 2012 at 09:38:46PM +0000, Christoffer Dall wrote:
>> On Mon, Nov 19, 2012 at 9:21 AM, Will Deacon <will.deacon@arm.com> wrote:
>> > On Sat, Nov 10, 2012 at 03:42:31PM +0000, Christoffer Dall wrote:
>> >> Decoding the implementor and part number of the CPU id in the CPU ID
>> >> register is needed by KVM, so we factor it out to share the code.
>> >>
>> >> Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
>> >> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>
> [...]
>
>> >> +static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
>> >> +{
>> >> +     return (read_cpuid_id() & 0xFF000000) >> 24;
>> >> +}
>> >> +
>> >> +static inline unsigned int __attribute_const__ read_cpuid_part_number(void)
>> >> +{
>> >> +     return (read_cpuid_id() & 0xFFF0);
>> >> +}
>> >
>> > Perhaps this should take the implementor as an argument, given that the
>> > part number is described differently between implementors. The xscale
>> > stuff can then move in here (we'll need to check the xscale docs in case
>> > perf is using a subfield -- I can't remember off-hand).
>
> [...]
>
>> > If you stick this one in a separate patch, I can take it via the perf
>> > tree (along with the CPUID rework above).
>> >
>> thanks,
>> I sent a separate patch.
>
> Looks like we still have the ugly xscale cpuid parsing inline. Could you
> move it as I suggested, please?
>
yes, sorry I missed that one. I went hunting through some old Xscale
docs, but cannot find anything that specifies more details about the
part number (or "product number" as intel calls it), so I preserved
the existing bit parsing.

-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index cb47d28..306fb2c 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -51,6 +51,22 @@  extern unsigned int processor_id;
 #define read_cpuid_ext(reg) 0
 #endif
 
+#define IMPLEMENTOR_ARM		0x41
+#define IMPLEMENTOR_INTEL	0x69
+
+#define PART_NUMBER_ARM1136	0xB360
+#define PART_NUMBER_ARM1156	0xB560
+#define PART_NUMBER_ARM1176	0xB760
+#define PART_NUMBER_ARM11MPCORE	0xB020
+#define PART_NUMBER_CORTEX_A8	0xC080
+#define PART_NUMBER_CORTEX_A9 	0xC090
+#define PART_NUMBER_CORTEX_A5 	0xC050
+#define PART_NUMBER_CORTEX_A15	0xC0F0
+#define PART_NUMBER_CORTEX_A7	0xC070
+
+#define PART_NUMBER_XSCALE1	0x1
+#define PART_NUMBER_XSCALE2	0x2
+
 /*
  * The CPU ID never changes at run time, so we might as well tell the
  * compiler that it's constant.  Use this function to read the CPU ID
@@ -61,6 +77,16 @@  static inline unsigned int __attribute_const__ read_cpuid_id(void)
 	return read_cpuid(CPUID_ID);
 }
 
+static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
+{
+	return (read_cpuid_id() & 0xFF000000) >> 24;
+}
+
+static inline unsigned int __attribute_const__ read_cpuid_part_number(void)
+{
+	return (read_cpuid_id() & 0xFFF0);
+}
+
 static inline unsigned int __attribute_const__ read_cpuid_cachetype(void)
 {
 	return read_cpuid(CPUID_CACHETYPE);
diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
index 8d7d8d4..ff18566 100644
--- a/arch/arm/kernel/perf_event_cpu.c
+++ b/arch/arm/kernel/perf_event_cpu.c
@@ -200,46 +200,46 @@  static struct arm_pmu *__devinit probe_current_pmu(void)
 	struct arm_pmu *pmu = NULL;
 	int cpu = get_cpu();
 	unsigned long cpuid = read_cpuid_id();
-	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
-	unsigned long part_number = (cpuid & 0xFFF0);
+	unsigned long implementor = read_cpuid_implementor();
+	unsigned long part_number = read_cpuid_part_number();
 
 	pr_info("probing PMU on CPU %d\n", cpu);
 
 	/* ARM Ltd CPUs. */
-	if (0x41 == implementor) {
+	if (implementor == IMPLEMENTOR_ARM) {
 		switch (part_number) {
-		case 0xB360:	/* ARM1136 */
-		case 0xB560:	/* ARM1156 */
-		case 0xB760:	/* ARM1176 */
+		case PART_NUMBER_ARM1136:
+		case PART_NUMBER_ARM1156:
+		case PART_NUMBER_ARM1176:
 			pmu = armv6pmu_init();
 			break;
-		case 0xB020:	/* ARM11mpcore */
+		case PART_NUMBER_ARM11MPCORE:
 			pmu = armv6mpcore_pmu_init();
 			break;
-		case 0xC080:	/* Cortex-A8 */
+		case PART_NUMBER_CORTEX_A8:
 			pmu = armv7_a8_pmu_init();
 			break;
-		case 0xC090:	/* Cortex-A9 */
+		case PART_NUMBER_CORTEX_A9:
 			pmu = armv7_a9_pmu_init();
 			break;
-		case 0xC050:	/* Cortex-A5 */
+		case PART_NUMBER_CORTEX_A5:
 			pmu = armv7_a5_pmu_init();
 			break;
-		case 0xC0F0:	/* Cortex-A15 */
+		case PART_NUMBER_CORTEX_A15:
 			pmu = armv7_a15_pmu_init();
 			break;
-		case 0xC070:	/* Cortex-A7 */
+		case PART_NUMBER_CORTEX_A7:
 			pmu = armv7_a7_pmu_init();
 			break;
 		}
 	/* Intel CPUs [xscale]. */
-	} else if (0x69 == implementor) {
+	} else if (implementor == IMPLEMENTOR_INTEL) {
 		part_number = (cpuid >> 13) & 0x7;
 		switch (part_number) {
-		case 1:
+		case PART_NUMBER_XSCALE1:
 			pmu = xscale1pmu_init();
 			break;
-		case 2:
+		case PART_NUMBER_XSCALE2:
 			pmu = xscale2pmu_init();
 			break;
 		}