diff mbox

Bisected: RED State Exception in 4.5 on E420R

Message ID 20160427.165222.722779265816957227.davem@davemloft.net
State Accepted
Delegated to: David Miller
Headers show

Commit Message

David Miller April 27, 2016, 8:52 p.m. UTC
Hey folks, I just pushed the following fix into the sparc GIT tree,
give it a spin.

Thanks!

Comments

Joerg Abraham April 28, 2016, 4:58 a.m. UTC | #1
Am 27.04.2016 um 22:52 schrieb David Miller:
>
> Hey folks, I just pushed the following fix into the sparc GIT tree,
> give it a spin.

yep, I can confirm boot problem fixed for me.

Tested-by: Joerg Abraham <joerg.abraham@nokia.com>

Thanks
>
> Thanks!
>
> ====================
> [PATCH] sparc64: Fix bootup regressions on some Kconfig combinations.
>
> The system call tracing bug fix mentioned in the Fixes tag
> below increased the amount of assembler code in the sequence
> of assembler files included by head_64.S
>
> This caused to total set of code to exceed 0x4000 bytes in
> size, which overflows the expression in head_64.S that works
> to place swapper_tsb at address 0x408000.
>
> When this is violated, the TSB is not properly aligned, and
> also the trap table is not aligned properly either.  All of
> this together results in failed boots.
>
> So, do two things:
>
> 1) Simplify some code by using ba,a instead of ba/nop to get
>     those bytes back.
>
> 2) Add a linker script assertion to make sure that if this
>     happens again the build will fail.
>
> Fixes: 1a40b95374f6 ("sparc: Fix system call tracing register handling.")
> Reported-by: Meelis Roos <mroos@linux.ee>
> Reported-by: Joerg Abraham <joerg.abraham@nokia.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>   arch/sparc/kernel/cherrs.S      | 14 +++++---------
>   arch/sparc/kernel/fpu_traps.S   | 11 +++++------
>   arch/sparc/kernel/head_64.S     | 24 ++++++++----------------
>   arch/sparc/kernel/misctrap.S    | 12 ++++--------
>   arch/sparc/kernel/spiterrs.S    | 18 ++++++------------
>   arch/sparc/kernel/utrap.S       |  3 +--
>   arch/sparc/kernel/vmlinux.lds.S |  4 ++++
>   arch/sparc/kernel/winfixup.S    |  3 +--
>   8 files changed, 34 insertions(+), 55 deletions(-)
>
> diff --git a/arch/sparc/kernel/cherrs.S b/arch/sparc/kernel/cherrs.S
> index 4ee1ad4..655628de 100644
> --- a/arch/sparc/kernel/cherrs.S
> +++ b/arch/sparc/kernel/cherrs.S
> @@ -214,8 +214,7 @@ do_dcpe_tl1_nonfatal:	/* Ok we may use interrupt globals safely. */
>   	subcc		%g1, %g2, %g1		! Next cacheline
>   	bge,pt		%icc, 1b
>   	 nop
> -	ba,pt		%xcc, dcpe_icpe_tl1_common
> -	 nop
> +	ba,a,pt		%xcc, dcpe_icpe_tl1_common
>
>   do_dcpe_tl1_fatal:
>   	sethi		%hi(1f), %g7
> @@ -224,8 +223,7 @@ do_dcpe_tl1_fatal:
>   	mov		0x2, %o0
>   	call		cheetah_plus_parity_error
>   	 add		%sp, PTREGS_OFF, %o1
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		do_dcpe_tl1,.-do_dcpe_tl1
>
>   	.globl		do_icpe_tl1
> @@ -259,8 +257,7 @@ do_icpe_tl1_nonfatal:	/* Ok we may use interrupt globals safely. */
>   	subcc		%g1, %g2, %g1
>   	bge,pt		%icc, 1b
>   	 nop
> -	ba,pt		%xcc, dcpe_icpe_tl1_common
> -	 nop
> +	ba,a,pt		%xcc, dcpe_icpe_tl1_common
>
>   do_icpe_tl1_fatal:
>   	sethi		%hi(1f), %g7
> @@ -269,8 +266,7 @@ do_icpe_tl1_fatal:
>   	mov		0x3, %o0
>   	call		cheetah_plus_parity_error
>   	 add		%sp, PTREGS_OFF, %o1
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		do_icpe_tl1,.-do_icpe_tl1
>   	
>   	.type		dcpe_icpe_tl1_common,#function
> @@ -456,7 +452,7 @@ __cheetah_log_error:
>   	 cmp		%g2, 0x63
>   	be		c_cee
>   	 nop
> -	ba,pt		%xcc, c_deferred
> +	ba,a,pt		%xcc, c_deferred
>   	.size		__cheetah_log_error,.-__cheetah_log_error
>
>   	/* Cheetah FECC trap handling, we get here from tl{0,1}_fecc
> diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S
> index a686482..336d275 100644
> --- a/arch/sparc/kernel/fpu_traps.S
> +++ b/arch/sparc/kernel/fpu_traps.S
> @@ -100,8 +100,8 @@ do_fpdis:
>   	fmuld		%f0, %f2, %f26
>   	faddd		%f0, %f2, %f28
>   	fmuld		%f0, %f2, %f30
> -	b,pt		%xcc, fpdis_exit
> -	 nop
> +	ba,a,pt		%xcc, fpdis_exit
> +
>   2:	andcc		%g5, FPRS_DU, %g0
>   	bne,pt		%icc, 3f
>   	 fzero		%f32
> @@ -144,8 +144,8 @@ do_fpdis:
>   	fmuld		%f32, %f34, %f58
>   	faddd		%f32, %f34, %f60
>   	fmuld		%f32, %f34, %f62
> -	ba,pt		%xcc, fpdis_exit
> -	 nop
> +	ba,a,pt		%xcc, fpdis_exit
> +
>   3:	mov		SECONDARY_CONTEXT, %g3
>   	add		%g6, TI_FPREGS, %g1
>
> @@ -197,8 +197,7 @@ fpdis_exit2:
>   fp_other_bounce:
>   	call		do_fpother
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		fp_other_bounce,.-fp_other_bounce
>
>   	.align		32
> diff --git a/arch/sparc/kernel/head_64.S b/arch/sparc/kernel/head_64.S
> index 5b4f5c3..a076b42 100644
> --- a/arch/sparc/kernel/head_64.S
> +++ b/arch/sparc/kernel/head_64.S
> @@ -466,9 +466,8 @@ sun4v_chip_type:
>   	subcc	%g3, 1, %g3
>   	bne,pt	%xcc, 41b
>   	add	%g1, 1, %g1
> -	mov	SUN4V_CHIP_SPARC64X, %g4
>   	ba,pt	%xcc, 5f
> -	nop
> +	 mov	SUN4V_CHIP_SPARC64X, %g4
>
>   49:
>   	mov	SUN4V_CHIP_UNKNOWN, %g4
> @@ -553,8 +552,7 @@ sun4u_init:
>   	stxa		%g0, [%g7] ASI_DMMU
>   	membar	#Sync
>
> -	ba,pt		%xcc, sun4u_continue
> -	 nop
> +	ba,a,pt		%xcc, sun4u_continue
>
>   sun4v_init:
>   	/* Set ctx 0 */
> @@ -565,14 +563,12 @@ sun4v_init:
>   	mov		SECONDARY_CONTEXT, %g7
>   	stxa		%g0, [%g7] ASI_MMU
>   	membar		#Sync
> -	ba,pt		%xcc, niagara_tlb_fixup
> -	 nop
> +	ba,a,pt		%xcc, niagara_tlb_fixup
>
>   sun4u_continue:
>   	BRANCH_IF_ANY_CHEETAH(g1, g7, cheetah_tlb_fixup)
>
> -	ba,pt	%xcc, spitfire_tlb_fixup
> -	 nop
> +	ba,a,pt	%xcc, spitfire_tlb_fixup
>
>   niagara_tlb_fixup:
>   	mov	3, %g2		/* Set TLB type to hypervisor. */
> @@ -647,8 +643,7 @@ niagara_patch:
>   	call	hypervisor_patch_cachetlbops
>   	 nop
>
> -	ba,pt	%xcc, tlb_fixup_done
> -	 nop
> +	ba,a,pt	%xcc, tlb_fixup_done
>
>   cheetah_tlb_fixup:
>   	mov	2, %g2		/* Set TLB type to cheetah+. */
> @@ -667,8 +662,7 @@ cheetah_tlb_fixup:
>   	call	cheetah_patch_cachetlbops
>   	 nop
>
> -	ba,pt	%xcc, tlb_fixup_done
> -	 nop
> +	ba,a,pt	%xcc, tlb_fixup_done
>
>   spitfire_tlb_fixup:
>   	/* Set TLB type to spitfire. */
> @@ -782,8 +776,7 @@ setup_trap_table:
>   	call	%o1
>   	 add	%sp, (2047 + 128), %o0
>
> -	ba,pt	%xcc, 2f
> -	 nop
> +	ba,a,pt	%xcc, 2f
>
>   1:	sethi	%hi(sparc64_ttable_tl0), %o0
>   	set	prom_set_trap_table_name, %g2
> @@ -822,8 +815,7 @@ setup_trap_table:
>
>   	BRANCH_IF_ANY_CHEETAH(o2, o3, 1f)
>
> -	ba,pt	%xcc, 2f
> -	 nop
> +	ba,a,pt	%xcc, 2f
>
>   	/* Disable STICK_INT interrupts. */
>   1:
> diff --git a/arch/sparc/kernel/misctrap.S b/arch/sparc/kernel/misctrap.S
> index 753b4f0..34b4933 100644
> --- a/arch/sparc/kernel/misctrap.S
> +++ b/arch/sparc/kernel/misctrap.S
> @@ -18,8 +18,7 @@ __do_privact:
>   109:	or		%g7, %lo(109b), %g7
>   	call		do_privact
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		__do_privact,.-__do_privact
>
>   	.type		do_mna,#function
> @@ -46,8 +45,7 @@ do_mna:
>   	mov		%l5, %o2
>   	call		mem_address_unaligned
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		do_mna,.-do_mna
>
>   	.type		do_lddfmna,#function
> @@ -65,8 +63,7 @@ do_lddfmna:
>   	mov		%l5, %o2
>   	call		handle_lddfmna
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		do_lddfmna,.-do_lddfmna
>
>   	.type		do_stdfmna,#function
> @@ -84,8 +81,7 @@ do_stdfmna:
>   	mov		%l5, %o2
>   	call		handle_stdfmna
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		do_stdfmna,.-do_stdfmna
>
>   	.type		breakpoint_trap,#function
> diff --git a/arch/sparc/kernel/spiterrs.S b/arch/sparc/kernel/spiterrs.S
> index c357e40..4a73009 100644
> --- a/arch/sparc/kernel/spiterrs.S
> +++ b/arch/sparc/kernel/spiterrs.S
> @@ -85,8 +85,7 @@ __spitfire_cee_trap_continue:
>   	ba,pt		%xcc, etraptl1
>   	 rd		%pc, %g7
>
> -	ba,pt		%xcc, 2f
> -	 nop
> +	ba,a,pt		%xcc, 2f
>
>   1:	ba,pt		%xcc, etrap_irq
>   	 rd		%pc, %g7
> @@ -100,8 +99,7 @@ __spitfire_cee_trap_continue:
>   	mov		%l5, %o2
>   	call		spitfire_access_error
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		__spitfire_access_error,.-__spitfire_access_error
>
>   	/* This is the trap handler entry point for ECC correctable
> @@ -179,8 +177,7 @@ __spitfire_data_access_exception_tl1:
>   	mov		%l5, %o2
>   	call		spitfire_data_access_exception_tl1
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		__spitfire_data_access_exception_tl1,.-__spitfire_data_access_exception_tl1
>
>   	.type		__spitfire_data_access_exception,#function
> @@ -200,8 +197,7 @@ __spitfire_data_access_exception:
>   	mov		%l5, %o2
>   	call		spitfire_data_access_exception
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		__spitfire_data_access_exception,.-__spitfire_data_access_exception
>
>   	.type		__spitfire_insn_access_exception_tl1,#function
> @@ -220,8 +216,7 @@ __spitfire_insn_access_exception_tl1:
>   	mov		%l5, %o2
>   	call		spitfire_insn_access_exception_tl1
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		__spitfire_insn_access_exception_tl1,.-__spitfire_insn_access_exception_tl1
>
>   	.type		__spitfire_insn_access_exception,#function
> @@ -240,6 +235,5 @@ __spitfire_insn_access_exception:
>   	mov		%l5, %o2
>   	call		spitfire_insn_access_exception
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>   	.size		__spitfire_insn_access_exception,.-__spitfire_insn_access_exception
> diff --git a/arch/sparc/kernel/utrap.S b/arch/sparc/kernel/utrap.S
> index b7f0f3f..c731e80 100644
> --- a/arch/sparc/kernel/utrap.S
> +++ b/arch/sparc/kernel/utrap.S
> @@ -11,8 +11,7 @@ utrap_trap:		/* %g3=handler,%g4=level */
>   	mov		%l4, %o1
>           call		bad_trap
>   	 add		%sp, PTREGS_OFF, %o0
> -	ba,pt		%xcc, rtrap
> -	 nop
> +	ba,a,pt		%xcc, rtrap
>
>   invoke_utrap:
>   	sllx		%g3, 3, %g3
> diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S
> index aadd321..7d02b1f 100644
> --- a/arch/sparc/kernel/vmlinux.lds.S
> +++ b/arch/sparc/kernel/vmlinux.lds.S
> @@ -33,6 +33,10 @@ ENTRY(_start)
>   jiffies = jiffies_64;
>   #endif
>
> +#ifdef CONFIG_SPARC64
> +ASSERT((swapper_tsb == 0x0000000000408000), "Error: sparc64 early assembler too large")
> +#endif
> +
>   SECTIONS
>   {
>   #ifdef CONFIG_SPARC64
> diff --git a/arch/sparc/kernel/winfixup.S b/arch/sparc/kernel/winfixup.S
> index 1e67ce9..855019a 100644
> --- a/arch/sparc/kernel/winfixup.S
> +++ b/arch/sparc/kernel/winfixup.S
> @@ -32,8 +32,7 @@ fill_fixup:
>   	 rd	%pc, %g7
>   	call	do_sparc64_fault
>   	 add	%sp, PTREGS_OFF, %o0
> -	ba,pt	%xcc, rtrap
> -	 nop
> +	ba,a,pt	%xcc, rtrap
>
>   	/* Be very careful about usage of the trap globals here.
>   	 * You cannot touch %g5 as that has the fault information.
>
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Meelis Roos April 28, 2016, 5:26 p.m. UTC | #2
> Hey folks, I just pushed the following fix into the sparc GIT tree,
> give it a spin.

Works for me on my V240. Thank you!
David Miller April 28, 2016, 5:38 p.m. UTC | #3
From: mroos@linux.ee
Date: Thu, 28 Apr 2016 20:26:10 +0300 (EEST)

>> Hey folks, I just pushed the following fix into the sparc GIT tree,
>> give it a spin.
> 
> Works for me on my V240. Thank you!

Thanks for testing everyone.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg April 28, 2016, 7:49 p.m. UTC | #4
Hi David.

Took a quick skim through the patch.
But first let me say that it was a good spot of the bug.
It was not obvious the the size restriction was the culprint.

A few random comments - but nothing that needs to be updated.

	Sam

> diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S
> index a686482..336d275 100644
> --- a/arch/sparc/kernel/fpu_traps.S
> +++ b/arch/sparc/kernel/fpu_traps.S
> @@ -100,8 +100,8 @@ do_fpdis:
>  	fmuld		%f0, %f2, %f26
>  	faddd		%f0, %f2, %f28
>  	fmuld		%f0, %f2, %f30
> -	b,pt		%xcc, fpdis_exit
> -	 nop
> +	ba,a,pt		%xcc, fpdis_exit
> +
My sparc assembler foo is not good.
And I could not find any references to "b," branch instructions.
So I assume the "b," is equivalent to "ba,".
Which makes this good as the code now uses a documented variant.


>  	add	%g1, 1, %g1
> -	mov	SUN4V_CHIP_SPARC64X, %g4
>  	ba,pt	%xcc, 5f
> -	nop
> +	 mov	SUN4V_CHIP_SPARC64X, %g4

Nice..
Maybe we could have done so in mor places, but I did not spot them.

> diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S
> index aadd321..7d02b1f 100644
> --- a/arch/sparc/kernel/vmlinux.lds.S
> +++ b/arch/sparc/kernel/vmlinux.lds.S
> @@ -33,6 +33,10 @@ ENTRY(_start)
>  jiffies = jiffies_64;
>  #endif
>  
> +#ifdef CONFIG_SPARC64
> +ASSERT((swapper_tsb == 0x0000000000408000), "Error: sparc64 early assembler too large")
> +#endif

You did not use:
. = ASSERT() so older binutils are not supportet.
This is no problem because my boxes both have newer binutils.
And the testers that reported back on the mailing list did not
see any problems either - so I will assume this is OK.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 28, 2016, 8:10 p.m. UTC | #5
From: Sam Ravnborg <sam@ravnborg.org>
Date: Thu, 28 Apr 2016 21:49:39 +0200

>> diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S
>> index a686482..336d275 100644
>> --- a/arch/sparc/kernel/fpu_traps.S
>> +++ b/arch/sparc/kernel/fpu_traps.S
>> @@ -100,8 +100,8 @@ do_fpdis:
>>  	fmuld		%f0, %f2, %f26
>>  	faddd		%f0, %f2, %f28
>>  	fmuld		%f0, %f2, %f30
>> -	b,pt		%xcc, fpdis_exit
>> -	 nop
>> +	ba,a,pt		%xcc, fpdis_exit
>> +
> My sparc assembler foo is not good.
> And I could not find any references to "b," branch instructions.
> So I assume the "b," is equivalent to "ba,".
> Which makes this good as the code now uses a documented variant.

In v9, 'b' got changed into 'ba'.  There is no difference except that
with 'ba' you can add the prediction tags like ",pt" and ",pnt".

>>  	add	%g1, 1, %g1
>> -	mov	SUN4V_CHIP_SPARC64X, %g4
>>  	ba,pt	%xcc, 5f
>> -	nop
>> +	 mov	SUN4V_CHIP_SPARC64X, %g4
> 
> Nice..
> Maybe we could have done so in mor places, but I did not spot them.

I tried to find more low hanging fruit, if you do end up spotting some
more let me know. :-)

>> diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S
>> index aadd321..7d02b1f 100644
>> --- a/arch/sparc/kernel/vmlinux.lds.S
>> +++ b/arch/sparc/kernel/vmlinux.lds.S
>> @@ -33,6 +33,10 @@ ENTRY(_start)
>>  jiffies = jiffies_64;
>>  #endif
>>  
>> +#ifdef CONFIG_SPARC64
>> +ASSERT((swapper_tsb == 0x0000000000408000), "Error: sparc64 early assembler too large")
>> +#endif
> 
> You did not use:
> . = ASSERT() so older binutils are not supportet.
> This is no problem because my boxes both have newer binutils.
> And the testers that reported back on the mailing list did not
> see any problems either - so I will assume this is OK.

Do you know exactly when plain "ASSERT()" starts working and doesn't
require the ". = " part?

Thanks Sam.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg April 28, 2016, 8:57 p.m. UTC | #6
> 
> Do you know exactly when plain "ASSERT()" starts working and doesn't
> require the ". = " part?

I have digged a little. The original bug report is here:
http://marc.info/?l=linux-kbuild&m=124930110427870&w=2

Here Jean report that is fails with binutils 2.14, and
Documentation/Changes list 2.12 as the minimum required version.

The bug will only manifest itself with a "plain" ASSERT.

So ASSERT placed inside:

SECTIONS
{
	.text TEXTSTART:
	{
	}

	ASSERT(bla bla, bla);
}

Will to my best understanding not trigger the bug.
x86 has a similar assert in line 186.

I did not test it - getting late here.

Will try to go through the .S files tomorrow to check
if I can find additional places where we can save
a few bytes.

And I may try to implment the above too.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Gardner April 28, 2016, 10:54 p.m. UTC | #7
On 04/28/2016 02:10 PM, David Miller wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> Date: Thu, 28 Apr 2016 21:49:39 +0200
>
>>> diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S
>>> index a686482..336d275 100644
>>> --- a/arch/sparc/kernel/fpu_traps.S
>>> +++ b/arch/sparc/kernel/fpu_traps.S
>>> @@ -100,8 +100,8 @@ do_fpdis:
>>>   	fmuld		%f0, %f2, %f26
>>>   	faddd		%f0, %f2, %f28
>>>   	fmuld		%f0, %f2, %f30
>>> -	b,pt		%xcc, fpdis_exit
>>> -	 nop
>>> +	ba,a,pt		%xcc, fpdis_exit
>>> +
>> My sparc assembler foo is not good.
>> And I could not find any references to "b," branch instructions.
>> So I assume the "b," is equivalent to "ba,".
>> Which makes this good as the code now uses a documented variant.
> In v9, 'b' got changed into 'ba'.  There is no difference except that
> with 'ba' you can add the prediction tags like ",pt" and ",pnt".
>
>>>   	add	%g1, 1, %g1
>>> -	mov	SUN4V_CHIP_SPARC64X, %g4
>>>   	ba,pt	%xcc, 5f
>>> -	nop
>>> +	 mov	SUN4V_CHIP_SPARC64X, %g4
>> Nice..
>> Maybe we could have done so in mor places, but I did not spot them.
> I tried to find more low hanging fruit, if you do end up spotting some
> more let me know. :-)
>


It's good to free up a few bytes here and there, but I think we're going 
to need to go further than this in the near future since code in this 
tiny region between 404000 and 0x408000 is going to grow. For example, 
code to check for new cpu chips is always being added. Also, new code 
such as Khalid's ADI patch adds a new exception handler for memory 
corruption detection.

Having run into this problem myself, I tried to understand why all this 
code needed to be squeezed into this small area, and as best I could 
determine, it was just an attempt to not "waste" the space. So why 
couldn't some of the code be moved to the region after the trap table? 
As long as code in the trap table can reach (in the branch displacement 
sense) all the code it needs to, is there any problem with moving 
things? For instance, there is a list of includes beginning with  
"etrap_64.S", and one or more of those can be moved to after the include 
"ttable_64.S", which will free up a chunk of space immediately.

Rob Gardner

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Julian Calaby April 29, 2016, 12:16 a.m. UTC | #8
Hi,

On Fri, Apr 29, 2016 at 8:54 AM, Rob Gardner <rob.gardner@oracle.com> wrote:
> On 04/28/2016 02:10 PM, David Miller wrote:
>>
>> From: Sam Ravnborg <sam@ravnborg.org>
>> Date: Thu, 28 Apr 2016 21:49:39 +0200
>>
>>>> diff --git a/arch/sparc/kernel/fpu_traps.S
>>>> b/arch/sparc/kernel/fpu_traps.S
>>>> index a686482..336d275 100644
>>>> --- a/arch/sparc/kernel/fpu_traps.S
>>>> +++ b/arch/sparc/kernel/fpu_traps.S
>>>> @@ -100,8 +100,8 @@ do_fpdis:
>>>>         fmuld           %f0, %f2, %f26
>>>>         faddd           %f0, %f2, %f28
>>>>         fmuld           %f0, %f2, %f30
>>>> -       b,pt            %xcc, fpdis_exit
>>>> -        nop
>>>> +       ba,a,pt         %xcc, fpdis_exit
>>>> +
>>>
>>> My sparc assembler foo is not good.
>>> And I could not find any references to "b," branch instructions.
>>> So I assume the "b," is equivalent to "ba,".
>>> Which makes this good as the code now uses a documented variant.
>>
>> In v9, 'b' got changed into 'ba'.  There is no difference except that
>> with 'ba' you can add the prediction tags like ",pt" and ",pnt".
>>
>>>>         add     %g1, 1, %g1
>>>> -       mov     SUN4V_CHIP_SPARC64X, %g4
>>>>         ba,pt   %xcc, 5f
>>>> -       nop
>>>> +        mov    SUN4V_CHIP_SPARC64X, %g4
>>>
>>> Nice..
>>> Maybe we could have done so in mor places, but I did not spot them.
>>
>> I tried to find more low hanging fruit, if you do end up spotting some
>> more let me know. :-)
>>
>
>
> It's good to free up a few bytes here and there, but I think we're going to
> need to go further than this in the near future since code in this tiny
> region between 404000 and 0x408000 is going to grow. For example, code to
> check for new cpu chips is always being added. Also, new code such as
> Khalid's ADI patch adds a new exception handler for memory corruption
> detection.
>
> Having run into this problem myself, I tried to understand why all this code
> needed to be squeezed into this small area, and as best I could determine,
> it was just an attempt to not "waste" the space. So why couldn't some of the
> code be moved to the region after the trap table? As long as code in the
> trap table can reach (in the branch displacement sense) all the code it
> needs to, is there any problem with moving things? For instance, there is a
> list of includes beginning with  "etrap_64.S", and one or more of those can
> be moved to after the include "ttable_64.S", which will free up a chunk of
> space immediately.

I have a similar question: Why not move all the "optional" code after
swapper_tsb?

Thanks,
David Miller May 1, 2016, 11:36 p.m. UTC | #9
From: Rob Gardner <rob.gardner@oracle.com>
Date: Thu, 28 Apr 2016 16:54:29 -0600

> It's good to free up a few bytes here and there, but I think we're
> going to need to go further than this in the near future since code in
> this tiny region between 404000 and 0x408000 is going to grow.

We should simply monitor the situation and just make sure to minimize
the wastage in this region, moving code out of the area as needed.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller May 1, 2016, 11:41 p.m. UTC | #10
From: Julian Calaby <julian.calaby@gmail.com>
Date: Fri, 29 Apr 2016 10:16:29 +1000

> Why not move all the "optional" code after swapper_tsb?

As Rob imagined, it's to keep this area from being wasted space in the
kernel image.  We pack as much code into this area as possible.  It's
also beneficial to group all of this trap processing code into the same
set of cache lines too.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" 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

====================
[PATCH] sparc64: Fix bootup regressions on some Kconfig combinations.

The system call tracing bug fix mentioned in the Fixes tag
below increased the amount of assembler code in the sequence
of assembler files included by head_64.S

This caused to total set of code to exceed 0x4000 bytes in
size, which overflows the expression in head_64.S that works
to place swapper_tsb at address 0x408000.

When this is violated, the TSB is not properly aligned, and
also the trap table is not aligned properly either.  All of
this together results in failed boots.

So, do two things:

1) Simplify some code by using ba,a instead of ba/nop to get
   those bytes back.

2) Add a linker script assertion to make sure that if this
   happens again the build will fail.

Fixes: 1a40b95374f6 ("sparc: Fix system call tracing register handling.")
Reported-by: Meelis Roos <mroos@linux.ee>
Reported-by: Joerg Abraham <joerg.abraham@nokia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc/kernel/cherrs.S      | 14 +++++---------
 arch/sparc/kernel/fpu_traps.S   | 11 +++++------
 arch/sparc/kernel/head_64.S     | 24 ++++++++----------------
 arch/sparc/kernel/misctrap.S    | 12 ++++--------
 arch/sparc/kernel/spiterrs.S    | 18 ++++++------------
 arch/sparc/kernel/utrap.S       |  3 +--
 arch/sparc/kernel/vmlinux.lds.S |  4 ++++
 arch/sparc/kernel/winfixup.S    |  3 +--
 8 files changed, 34 insertions(+), 55 deletions(-)

diff --git a/arch/sparc/kernel/cherrs.S b/arch/sparc/kernel/cherrs.S
index 4ee1ad4..655628de 100644
--- a/arch/sparc/kernel/cherrs.S
+++ b/arch/sparc/kernel/cherrs.S
@@ -214,8 +214,7 @@  do_dcpe_tl1_nonfatal:	/* Ok we may use interrupt globals safely. */
 	subcc		%g1, %g2, %g1		! Next cacheline
 	bge,pt		%icc, 1b
 	 nop
-	ba,pt		%xcc, dcpe_icpe_tl1_common
-	 nop
+	ba,a,pt		%xcc, dcpe_icpe_tl1_common
 
 do_dcpe_tl1_fatal:
 	sethi		%hi(1f), %g7
@@ -224,8 +223,7 @@  do_dcpe_tl1_fatal:
 	mov		0x2, %o0
 	call		cheetah_plus_parity_error
 	 add		%sp, PTREGS_OFF, %o1
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		do_dcpe_tl1,.-do_dcpe_tl1
 
 	.globl		do_icpe_tl1
@@ -259,8 +257,7 @@  do_icpe_tl1_nonfatal:	/* Ok we may use interrupt globals safely. */
 	subcc		%g1, %g2, %g1
 	bge,pt		%icc, 1b
 	 nop
-	ba,pt		%xcc, dcpe_icpe_tl1_common
-	 nop
+	ba,a,pt		%xcc, dcpe_icpe_tl1_common
 
 do_icpe_tl1_fatal:
 	sethi		%hi(1f), %g7
@@ -269,8 +266,7 @@  do_icpe_tl1_fatal:
 	mov		0x3, %o0
 	call		cheetah_plus_parity_error
 	 add		%sp, PTREGS_OFF, %o1
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		do_icpe_tl1,.-do_icpe_tl1
 	
 	.type		dcpe_icpe_tl1_common,#function
@@ -456,7 +452,7 @@  __cheetah_log_error:
 	 cmp		%g2, 0x63
 	be		c_cee
 	 nop
-	ba,pt		%xcc, c_deferred
+	ba,a,pt		%xcc, c_deferred
 	.size		__cheetah_log_error,.-__cheetah_log_error
 
 	/* Cheetah FECC trap handling, we get here from tl{0,1}_fecc
diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S
index a686482..336d275 100644
--- a/arch/sparc/kernel/fpu_traps.S
+++ b/arch/sparc/kernel/fpu_traps.S
@@ -100,8 +100,8 @@  do_fpdis:
 	fmuld		%f0, %f2, %f26
 	faddd		%f0, %f2, %f28
 	fmuld		%f0, %f2, %f30
-	b,pt		%xcc, fpdis_exit
-	 nop
+	ba,a,pt		%xcc, fpdis_exit
+
 2:	andcc		%g5, FPRS_DU, %g0
 	bne,pt		%icc, 3f
 	 fzero		%f32
@@ -144,8 +144,8 @@  do_fpdis:
 	fmuld		%f32, %f34, %f58
 	faddd		%f32, %f34, %f60
 	fmuld		%f32, %f34, %f62
-	ba,pt		%xcc, fpdis_exit
-	 nop
+	ba,a,pt		%xcc, fpdis_exit
+
 3:	mov		SECONDARY_CONTEXT, %g3
 	add		%g6, TI_FPREGS, %g1
 
@@ -197,8 +197,7 @@  fpdis_exit2:
 fp_other_bounce:
 	call		do_fpother
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		fp_other_bounce,.-fp_other_bounce
 
 	.align		32
diff --git a/arch/sparc/kernel/head_64.S b/arch/sparc/kernel/head_64.S
index 5b4f5c3..a076b42 100644
--- a/arch/sparc/kernel/head_64.S
+++ b/arch/sparc/kernel/head_64.S
@@ -466,9 +466,8 @@  sun4v_chip_type:
 	subcc	%g3, 1, %g3
 	bne,pt	%xcc, 41b
 	add	%g1, 1, %g1
-	mov	SUN4V_CHIP_SPARC64X, %g4
 	ba,pt	%xcc, 5f
-	nop
+	 mov	SUN4V_CHIP_SPARC64X, %g4
 
 49:
 	mov	SUN4V_CHIP_UNKNOWN, %g4
@@ -553,8 +552,7 @@  sun4u_init:
 	stxa		%g0, [%g7] ASI_DMMU
 	membar	#Sync
 
-	ba,pt		%xcc, sun4u_continue
-	 nop
+	ba,a,pt		%xcc, sun4u_continue
 
 sun4v_init:
 	/* Set ctx 0 */
@@ -565,14 +563,12 @@  sun4v_init:
 	mov		SECONDARY_CONTEXT, %g7
 	stxa		%g0, [%g7] ASI_MMU
 	membar		#Sync
-	ba,pt		%xcc, niagara_tlb_fixup
-	 nop
+	ba,a,pt		%xcc, niagara_tlb_fixup
 
 sun4u_continue:
 	BRANCH_IF_ANY_CHEETAH(g1, g7, cheetah_tlb_fixup)
 
-	ba,pt	%xcc, spitfire_tlb_fixup
-	 nop
+	ba,a,pt	%xcc, spitfire_tlb_fixup
 
 niagara_tlb_fixup:
 	mov	3, %g2		/* Set TLB type to hypervisor. */
@@ -647,8 +643,7 @@  niagara_patch:
 	call	hypervisor_patch_cachetlbops
 	 nop
 
-	ba,pt	%xcc, tlb_fixup_done
-	 nop
+	ba,a,pt	%xcc, tlb_fixup_done
 
 cheetah_tlb_fixup:
 	mov	2, %g2		/* Set TLB type to cheetah+. */
@@ -667,8 +662,7 @@  cheetah_tlb_fixup:
 	call	cheetah_patch_cachetlbops
 	 nop
 
-	ba,pt	%xcc, tlb_fixup_done
-	 nop
+	ba,a,pt	%xcc, tlb_fixup_done
 
 spitfire_tlb_fixup:
 	/* Set TLB type to spitfire. */
@@ -782,8 +776,7 @@  setup_trap_table:
 	call	%o1
 	 add	%sp, (2047 + 128), %o0
 
-	ba,pt	%xcc, 2f
-	 nop
+	ba,a,pt	%xcc, 2f
 
 1:	sethi	%hi(sparc64_ttable_tl0), %o0
 	set	prom_set_trap_table_name, %g2
@@ -822,8 +815,7 @@  setup_trap_table:
 
 	BRANCH_IF_ANY_CHEETAH(o2, o3, 1f)
 
-	ba,pt	%xcc, 2f
-	 nop
+	ba,a,pt	%xcc, 2f
 
 	/* Disable STICK_INT interrupts. */
 1:
diff --git a/arch/sparc/kernel/misctrap.S b/arch/sparc/kernel/misctrap.S
index 753b4f0..34b4933 100644
--- a/arch/sparc/kernel/misctrap.S
+++ b/arch/sparc/kernel/misctrap.S
@@ -18,8 +18,7 @@  __do_privact:
 109:	or		%g7, %lo(109b), %g7
 	call		do_privact
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		__do_privact,.-__do_privact
 
 	.type		do_mna,#function
@@ -46,8 +45,7 @@  do_mna:
 	mov		%l5, %o2
 	call		mem_address_unaligned
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		do_mna,.-do_mna
 
 	.type		do_lddfmna,#function
@@ -65,8 +63,7 @@  do_lddfmna:
 	mov		%l5, %o2
 	call		handle_lddfmna
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		do_lddfmna,.-do_lddfmna
 
 	.type		do_stdfmna,#function
@@ -84,8 +81,7 @@  do_stdfmna:
 	mov		%l5, %o2
 	call		handle_stdfmna
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		do_stdfmna,.-do_stdfmna
 
 	.type		breakpoint_trap,#function
diff --git a/arch/sparc/kernel/spiterrs.S b/arch/sparc/kernel/spiterrs.S
index c357e40..4a73009 100644
--- a/arch/sparc/kernel/spiterrs.S
+++ b/arch/sparc/kernel/spiterrs.S
@@ -85,8 +85,7 @@  __spitfire_cee_trap_continue:
 	ba,pt		%xcc, etraptl1
 	 rd		%pc, %g7
 
-	ba,pt		%xcc, 2f
-	 nop
+	ba,a,pt		%xcc, 2f
 
 1:	ba,pt		%xcc, etrap_irq
 	 rd		%pc, %g7
@@ -100,8 +99,7 @@  __spitfire_cee_trap_continue:
 	mov		%l5, %o2
 	call		spitfire_access_error
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		__spitfire_access_error,.-__spitfire_access_error
 
 	/* This is the trap handler entry point for ECC correctable
@@ -179,8 +177,7 @@  __spitfire_data_access_exception_tl1:
 	mov		%l5, %o2
 	call		spitfire_data_access_exception_tl1
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		__spitfire_data_access_exception_tl1,.-__spitfire_data_access_exception_tl1
 
 	.type		__spitfire_data_access_exception,#function
@@ -200,8 +197,7 @@  __spitfire_data_access_exception:
 	mov		%l5, %o2
 	call		spitfire_data_access_exception
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		__spitfire_data_access_exception,.-__spitfire_data_access_exception
 
 	.type		__spitfire_insn_access_exception_tl1,#function
@@ -220,8 +216,7 @@  __spitfire_insn_access_exception_tl1:
 	mov		%l5, %o2
 	call		spitfire_insn_access_exception_tl1
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		__spitfire_insn_access_exception_tl1,.-__spitfire_insn_access_exception_tl1
 
 	.type		__spitfire_insn_access_exception,#function
@@ -240,6 +235,5 @@  __spitfire_insn_access_exception:
 	mov		%l5, %o2
 	call		spitfire_insn_access_exception
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 	.size		__spitfire_insn_access_exception,.-__spitfire_insn_access_exception
diff --git a/arch/sparc/kernel/utrap.S b/arch/sparc/kernel/utrap.S
index b7f0f3f..c731e80 100644
--- a/arch/sparc/kernel/utrap.S
+++ b/arch/sparc/kernel/utrap.S
@@ -11,8 +11,7 @@  utrap_trap:		/* %g3=handler,%g4=level */
 	mov		%l4, %o1
         call		bad_trap
 	 add		%sp, PTREGS_OFF, %o0
-	ba,pt		%xcc, rtrap
-	 nop
+	ba,a,pt		%xcc, rtrap
 
 invoke_utrap:
 	sllx		%g3, 3, %g3
diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S
index aadd321..7d02b1f 100644
--- a/arch/sparc/kernel/vmlinux.lds.S
+++ b/arch/sparc/kernel/vmlinux.lds.S
@@ -33,6 +33,10 @@  ENTRY(_start)
 jiffies = jiffies_64;
 #endif
 
+#ifdef CONFIG_SPARC64
+ASSERT((swapper_tsb == 0x0000000000408000), "Error: sparc64 early assembler too large")
+#endif
+
 SECTIONS
 {
 #ifdef CONFIG_SPARC64
diff --git a/arch/sparc/kernel/winfixup.S b/arch/sparc/kernel/winfixup.S
index 1e67ce9..855019a 100644
--- a/arch/sparc/kernel/winfixup.S
+++ b/arch/sparc/kernel/winfixup.S
@@ -32,8 +32,7 @@  fill_fixup:
 	 rd	%pc, %g7
 	call	do_sparc64_fault
 	 add	%sp, PTREGS_OFF, %o0
-	ba,pt	%xcc, rtrap
-	 nop
+	ba,a,pt	%xcc, rtrap
 
 	/* Be very careful about usage of the trap globals here.
 	 * You cannot touch %g5 as that has the fault information.