[PATCH 1/3] sparc64: correctly recognise M7 cpu type

[PATCH 1/3] sparc64: correctly recognise M7 cpu type

[Allen Pais]

 The following patch adds support for correctly
recognising M7 cpu type.

Signed-off-by: Allen Pais <allen.pais@oracle.com>
---
 arch/sparc/include/asm/spitfire.h |    1 +
 arch/sparc/kernel/cpu.c           |    6 ++++++
 arch/sparc/kernel/head_64.S       |   13 +++++++++++++
 3 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/arch/sparc/include/asm/spitfire.h b/arch/sparc/include/asm/spitfire.h
index 3fc5869..9aec17b 100644
--- a/arch/sparc/include/asm/spitfire.h
+++ b/arch/sparc/include/asm/spitfire.h
@@ -45,6 +45,7 @@
 #define SUN4V_CHIP_NIAGARA3	0x03
 #define SUN4V_CHIP_NIAGARA4	0x04
 #define SUN4V_CHIP_NIAGARA5	0x05
+#define SUN4V_CHIP_SPARC_M7	0x08
 #define SUN4V_CHIP_SPARC64X	0x8a
 #define SUN4V_CHIP_UNKNOWN	0xff
 
diff --git a/arch/sparc/kernel/cpu.c b/arch/sparc/kernel/cpu.c
index 82a3a71..55dfb62 100644
--- a/arch/sparc/kernel/cpu.c
+++ b/arch/sparc/kernel/cpu.c
@@ -494,6 +494,12 @@ static void __init sun4v_cpu_probe(void)
 		sparc_pmu_type = "niagara5";
 		break;
 
+	case SUN4V_CHIP_SPARC_M7:
+		sparc_cpu_type = "SPARC-M7";
+		sparc_fpu_type = "SPARC-M7 integrated FPU";
+		sparc_pmu_type = "sparc-m7";
+		break;
+
 	case SUN4V_CHIP_SPARC64X:
 		sparc_cpu_type = "SPARC64-X";
 		sparc_fpu_type = "SPARC64-X integrated FPU";
diff --git a/arch/sparc/kernel/head_64.S b/arch/sparc/kernel/head_64.S
index 452f04f..508a542 100644
--- a/arch/sparc/kernel/head_64.S
+++ b/arch/sparc/kernel/head_64.S
@@ -414,6 +414,7 @@ sun4v_chip_type:
 	cmp	%g2, 'T'
 	be,pt	%xcc, 70f
 	 cmp	%g2, 'M'
+	be,pt	%xcc, 71f
 	bne,pn	%xcc, 49f
 	 nop
 
@@ -430,6 +431,14 @@ sun4v_chip_type:
 	ba,pt	%xcc, 49f
 	 nop
 
+71:
+	ldub	[%g1 + 7], %g2
+	cmp	%g2, '7'
+	be,pt	%xcc, 5f
+	 mov	SUN4V_CHIP_SPARC_M7, %g4
+	ba,pt	%xcc, 49f
+	 nop
+
 91:	sethi	%hi(prom_cpu_compatible), %g1
 	or	%g1, %lo(prom_cpu_compatible), %g1
 	ldub	[%g1 + 17], %g2
@@ -586,6 +595,10 @@ niagara_tlb_fixup:
 	be,pt	%xcc, niagara4_patch
 	 nop
 
+	cmp	%g1, SUN4V_CHIP_SPARC_M7
+	be,pt	%xcc, niagara4_patch
+	 nop
+
 	call	generic_patch_copyops
 	 nop
 	call	generic_patch_bzero
-- 
1.7.1

Re: [PATCH 1/3] sparc64: correctly recognise M7 cpu type

[Sam Ravnborg]

On Tue, Aug 19, 2014 at 10:23:32AM +0530, Allen Pais wrote:
>  The following patch adds support for correctly
> recognising M7 cpu type.
> 
> Signed-off-by: Allen Pais <allen.pais@oracle.com>
> ---
>  arch/sparc/include/asm/spitfire.h |    1 +
>  arch/sparc/kernel/cpu.c           |    6 ++++++
>  arch/sparc/kernel/head_64.S       |   13 +++++++++++++
>  3 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/sparc/include/asm/spitfire.h b/arch/sparc/include/asm/spitfire.h
> index 3fc5869..9aec17b 100644
> --- a/arch/sparc/include/asm/spitfire.h
> +++ b/arch/sparc/include/asm/spitfire.h
> @@ -45,6 +45,7 @@
>  #define SUN4V_CHIP_NIAGARA3	0x03
>  #define SUN4V_CHIP_NIAGARA4	0x04
>  #define SUN4V_CHIP_NIAGARA5	0x05
> +#define SUN4V_CHIP_SPARC_M7	0x08
>  #define SUN4V_CHIP_SPARC64X	0x8a
>  #define SUN4V_CHIP_UNKNOWN	0xff
>  
> diff --git a/arch/sparc/kernel/cpu.c b/arch/sparc/kernel/cpu.c
> index 82a3a71..55dfb62 100644
> --- a/arch/sparc/kernel/cpu.c
> +++ b/arch/sparc/kernel/cpu.c
> @@ -494,6 +494,12 @@ static void __init sun4v_cpu_probe(void)
>  		sparc_pmu_type = "niagara5";
>  		break;
>  
> +	case SUN4V_CHIP_SPARC_M7:
> +		sparc_cpu_type = "SPARC-M7";
> +		sparc_fpu_type = "SPARC-M7 integrated FPU";
> +		sparc_pmu_type = "sparc-m7";
> +		break;
> +
>  	case SUN4V_CHIP_SPARC64X:
>  		sparc_cpu_type = "SPARC64-X";
>  		sparc_fpu_type = "SPARC64-X integrated FPU";
> diff --git a/arch/sparc/kernel/head_64.S b/arch/sparc/kernel/head_64.S
> index 452f04f..508a542 100644
> --- a/arch/sparc/kernel/head_64.S
> +++ b/arch/sparc/kernel/head_64.S
> @@ -414,6 +414,7 @@ sun4v_chip_type:
>  	cmp	%g2, 'T'
>  	be,pt	%xcc, 70f
>  	 cmp	%g2, 'M'
> +	be,pt	%xcc, 71f
>  	bne,pn	%xcc, 49f
Looks like you are missing a nop in the delay slot?

	Sam

Re: [PATCH 1/3] sparc64: correctly recognise M7 cpu type

[Allen Pais]

>> diff --git a/arch/sparc/kernel/head_64.S b/arch/sparc/kernel/head_64.S
>> index 452f04f..508a542 100644
>> --- a/arch/sparc/kernel/head_64.S
>> +++ b/arch/sparc/kernel/head_64.S
>> @@ -414,6 +414,7 @@ sun4v_chip_type:
>>  	cmp	%g2, 'T'
>>  	be,pt	%xcc, 70f
>>  	 cmp	%g2, 'M'
>> +	be,pt	%xcc, 71f
>>  	bne,pn	%xcc, 49f
> Looks like you are missing a nop in the delay slot?

  I don't think so. I have tested the patch on M7 and I have had no issues with it.

- Allen

Re: [PATCH 1/3] sparc64: correctly recognise M7 cpu type

[Sam Ravnborg]

On Thu, Aug 21, 2014 at 03:33:27PM +0530, Allen Pais wrote:
> 
> >> diff --git a/arch/sparc/kernel/head_64.S b/arch/sparc/kernel/head_64.S
> >> index 452f04f..508a542 100644
> >> --- a/arch/sparc/kernel/head_64.S
> >> +++ b/arch/sparc/kernel/head_64.S
> >> @@ -414,6 +414,7 @@ sun4v_chip_type:
> >>  	cmp	%g2, 'T'
> >>  	be,pt	%xcc, 70f
> >>  	 cmp	%g2, 'M'
> >> +	be,pt	%xcc, 71f
> >>  	bne,pn	%xcc, 49f
> > Looks like you are missing a nop in the delay slot?
> 
>   I don't think so. I have tested the patch on M7 and I have had no issues with it.
Even if it works as expected then it is confusing.
I read the code like this:

if xcc equals pt then jump to 71 but if pn is not equal to xcc then jump to 49

Maybe the second opcode is ignored because it is a conditional jump or maybe
they are always equal.
But whatever it is confusing.

Adding an extra nop would help the readability.

	Sam

Re: [PATCH 1/3] sparc64: correctly recognise M7 cpu type

[Richard Mortimer]

On 21/08/2014 13:14, Sam Ravnborg wrote:
> On Thu, Aug 21, 2014 at 03:33:27PM +0530, Allen Pais wrote:
>>
>>>> diff --git a/arch/sparc/kernel/head_64.S b/arch/sparc/kernel/head_64.S
>>>> index 452f04f..508a542 100644
>>>> --- a/arch/sparc/kernel/head_64.S
>>>> +++ b/arch/sparc/kernel/head_64.S
>>>> @@ -414,6 +414,7 @@ sun4v_chip_type:
>>>>   	cmp	%g2, 'T'
>>>>   	be,pt	%xcc, 70f
>>>>   	 cmp	%g2, 'M'
>>>> +	be,pt	%xcc, 71f
>>>>   	bne,pn	%xcc, 49f
>>> Looks like you are missing a nop in the delay slot?
>>
>>    I don't think so. I have tested the patch on M7 and I have had no issues with it.
> Even if it works as expected then it is confusing.
> I read the code like this:
>
> if xcc equals pt then jump to 71 but if pn is not equal to xcc then jump to 49
>
> Maybe the second opcode is ignored because it is a conditional jump or maybe
> they are always equal.
The two branches are the logical opposite of each other so only one is 
ever taken and that is why the code "works".

> But whatever it is confusing.
>
> Adding an extra nop would help the readability.
>
Agreed. This isn't performance sensitive code.

Regards

Richard

P.S. Historical note - SPARC V8 didn't define what happens in the above 
case as noted on page 77 of the SPARC v9 manual)

"V8 Compatibility Note:
SPARC-V8 left as undefined the result of executing a delayed conditional 
branch that had a delayed control transfer in its delay slot. For this 
reason, programmers should avoid such constructs when
backwards compatibility is an issue."

Re: [PATCH 1/3] sparc64: correctly recognise M7 cpu type

[Allen Pais]

Sam, Richard,
>>>>> @@ -414,6 +414,7 @@ sun4v_chip_type:
>>>>>       cmp    %g2, 'T'
>>>>>       be,pt    %xcc, 70f
>>>>>        cmp    %g2, 'M'
>>>>> +    be,pt    %xcc, 71f
>>>>>       bne,pn    %xcc, 49f
>>>> Looks like you are missing a nop in the delay slot?
>>>
>>>    I don't think so. I have tested the patch on M7 and I have had no issues with it.
>> Even if it works as expected then it is confusing.
>> I read the code like this:
>>
>> if xcc equals pt then jump to 71 but if pn is not equal to xcc then jump to 49
>>
>> Maybe the second opcode is ignored because it is a conditional jump or maybe
>> they are always equal.
> The two branches are the logical opposite of each other so only one is ever taken and that is why the code "works".

  I'll go ahead and add the delay slot for better readability. It'll look something like this,

	 cmp	%g2, 'T'
 	be,pt	%xcc, 70f
 	 cmp	%g2, 'M'
+	be,pt	%xcc, 71f
+ 	 nop
 	bne,pn	%xcc, 49f
 	 nop

- ALlen

Re: [PATCH 1/3] sparc64: correctly recognise M7 cpu type

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Thu, 21 Aug 2014 14:14:50 +0200

> Maybe the second opcode is ignored because it is a conditional jump
> or maybe they are always equal.  But whatever it is confusing.

I totally agree that the patch should use a nop in the delay slot so
that it's clear and easier to understand, however I wanted to show how
the behavior is well defined and can even be used intentionally for
useful purposes. :-)

Branches in a delay slot a long time ago were marked as undefined
behavior, but for some time now they are expected to behave in
a certain way, consider:

	ba	label_a
	ba	label_b

this is a neat way to execute one instruction at label_a then go
to label_b.

More generically:

	jmpl	%reg
	ba	interpreter_continue
interpreter_continue:

which you could use to build an instruction at a time execution
engine, which only must elide emulate instructions which use registers
in this special code sequence.

Gordan Irlam used such a technique for his sun4d tracer named SKY.
Unfortunately the SKY web page no longer exists, but this tracer led
to discoveries that brough about things such as the SLAB allocator.

Anyways, all delayed control transfer instructions do is set NPC.  So:

0x00	ba	0x20
0x04	ba	0x30
 ...
0x20	nop
 ...
0x30	nop
 ...

At the first instruction, PC=0x00 and NPC=0x04

PC will be set to NPC at the end of execution.
For non-branches NPC is set to NPC + 0x4 but for this
delayed control transfer NPC will be set to 0x20 instead.

So PC=0x04 and NPC=0x20 when we get to the second instruction
in the first instruction's delay slot.

PC will be set to 0x20 and NPC will be set to 0x30

The nop at 0x20 will be executed, PC will be set to 0x30 and NPC
will be set to 0x34.

Finally we execute the nop at 0x30 and continue to linearly execute.

Re: [PATCH 1/3] sparc64: correctly recognise M7 cpu type

[David Miller]

From: Richard Mortimer <richm@oldelvet.org.uk>
Date: Thu, 21 Aug 2014 13:29:36 +0100

> On 21/08/2014 13:14, Sam Ravnborg wrote:
>> Adding an extra nop would help the readability.
>>
> Agreed. This isn't performance sensitive code.

Alan, please resubmit your patch series with the nop added, thanks.