* Re: Fwd: sh:fixed issue in xchg_u32 function when val==r15.
2011-06-22 7:18 Fwd: sh:fixed issue in xchg_u32 function when val==r15 Carmelo AMOROSO
@ 2011-06-23 12:27 ` Srinivas KANDAGATLA
2011-06-23 13:17 ` Carmelo AMOROSO
1 sibling, 0 replies; 3+ messages in thread
From: Srinivas KANDAGATLA @ 2011-06-23 12:27 UTC (permalink / raw)
To: linux-sh
Hi Carmelo & Salvo,
I agree with your observation..
Initially we had similar understanding, however after discussing with compiler guys and reading http://gcc.gnu.org/bugzilla/show_bug.cgi?id\x11807 posts, which made us decide adding 'val' to constraints rather than clobber list.
As r15 is the stack pointer, adding r15(SP) to clobber list might not work as expected(as we have no idea what compiler is going to do), One guess is that It will have unknown side effects(which is proved as shown below...).
Yesterday, Stuart and I spend some time to test and see the side-effect of adding r15 to clobber list in xchg_u32 after reverting my changes on 7141(ST40) board, We got lucky.. and encountered crash in every boot.
PC and PR ended up at 0x0 as r15 got corrupted because of the below code generated as result of r15 in clobber list.
0x8004b128 <+36>: mova 0x8004b134 <bdi_sync_supers+48>,r0
0x8004b12a <+38>: nop
0x8004b12c <+40>: mov r15,r1
0x8004b12e <+42>: mov #-4,r15
0x8004b130 <+44>: mov.l @r2,r3
0x8004b132 <+46>: mov.l r8,@r2
0x8004b134 <+48>: mov r1,r15
0x8004b136 <+50>: jsr @r10
0x8004b138 <+52>: nop
0x8004b13a <+54>: jsr @r9
0x8004b13c <+56>: nop
0x8004b13e <+58>: jsr @r11
0x8004b140 <+60>: nop
0x8004b142 <+62>: tst r0,r0
=> 0x8004b144 <+64>: bt.s 0x8004b124 <bdi_sync_supers+32>
=> 0x8004b146 <+66>: lds.l @r15+,pr
0x8004b148 <+68>: mov.l @r15+,r11
0x8004b14a <+70>: mov #0,r0
0x8004b14c <+72>: mov.l @r15+,r10
0x8004b14e <+74>: mov.l @r15+,r9
0x8004b150 <+76>: rts
If you look at the highlighted lines of code, bt is delayed branch instruction, which ends up incrementing r15 for every loop, as a result we see PC and PR end's up at 0.
with latest code (with my patches) below code is generated for the same subroutine.
298: 02 c7 mova 2a4 <bdi_sync_supers+0x34>,r0
29a: 09 00 nop
29c: f3 61 mov r15,r1
29e: fc ef mov #-4,r15
2a0: 22 67 mov.l @r2,r7
2a2: 32 22 mov.l r3,@r2
2a4: 13 6f mov r1,r15
2a6: 0b 49 jsr @r9
2a8: 09 00 nop
2aa: 0b 48 jsr @r8
2ac: 09 00 nop
2ae: 0b 4b jsr @r11
2b0: 09 00 nop
2b2: 08 20 tst r0,r0
=> 2b4: ec 89 bt 290 <bdi_sync_supers+0x20>
=> 2b6: 26 4f lds.l @r15+,pr
2b8: f6 6b mov.l @r15+,r11
2ba: 00 e0 mov #0,r0
2bc: f6 6a mov.l @r15+,r10
2be: f6 69 mov.l @r15+,r9
2c0: 0b 00 rts
There might be other such instances of side effects of r15(stack pointer) in clobber list.
I hope we answered why 'val' ended up in input/output constraints rather than r15 in clobber list.
Thanks,
srini
Carmelo AMOROSO wrote:
> On 22/06/2011 9.05, Carmelo Amoroso wrote:
>> ---------- Forwarded message ----------
>> From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
>> Date: 19 April 2011 18:28
>> Subject: Re: sh:fixed issue in xchg_u32 function when val=r15.
>> To: Paul Mundt <lethal@linux-sh.org>
>> Cc: linux-sh@vger.kernel.org, Stuart Menefy <stuart.menefy@st.com>
>>
>>
>> Hi Paul,
>> Thanks for the suggestion.
>>
>> Attached is the reworked patch for potential gUSA rollback users.
>>
>> Thanks,
>> srini
>>
>>
>> Paul Mundt wrote:
>>> On Thu, Apr 07, 2011 at 01:57:09PM +0100, Srinivas KANDAGATLA wrote:
>>>
>>>> Recently we have discovered a bug in xchg_u32 function of GUSA_RB feature.
>>>> This function breaks if one of the input parameter 'val' is r15.
>>>>
>>>> 168: 02 c7 mova 174 <exit_mm+0x54>,r0
>>>> 16a: 09 00 nop 16c: f3 61 mov r15,r1
>>>> 16e: fc ef mov #-4,r15
>>>>
>>> The -4 here is part of the gUSA login sequence, so if you're seeing the
>>> problem with gUSA based xchg_u32 it seems like you're going to have to
>>> apply the same fix for all gUSA rollback users.
>>>
>
> Hi Paul, Srini
> instead of making 'val' an input/output contraint (to force gcc not
> using r15 directly), should not be better (or more correct) to add r15
> to the clobbered register list ? r15 is actually modified for the
> login/logout gUSA sequence.
> Indeed, 'val' is not an output parameter... so we are benefiting from a
> side effect.
>
> Further, looking at __cmpxchg_u32, we think that we should make the
> first argument (volatile int *m) an input/output constraint as well (as
> it happens in xchg_u8 and xchg_32), and include r15 instead in the
> clobber list, keeping 'val' just an input parameter.
>
> Comments are welcome.
>
> Cheers,
> Carmelo & Salvo
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Fwd: sh:fixed issue in xchg_u32 function when val==r15.
2011-06-22 7:18 Fwd: sh:fixed issue in xchg_u32 function when val==r15 Carmelo AMOROSO
2011-06-23 12:27 ` Srinivas KANDAGATLA
@ 2011-06-23 13:17 ` Carmelo AMOROSO
1 sibling, 0 replies; 3+ messages in thread
From: Carmelo AMOROSO @ 2011-06-23 13:17 UTC (permalink / raw)
To: linux-sh
On 23/06/2011 14.27, Srinivas KANDAGATLA wrote:
> Hi Carmelo & Salvo,
> I agree with your observation..
>
> Initially we had similar understanding, however after discussing with
> compiler guys and reading
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id\x11807 posts, which made us
> decide adding 'val' to constraints rather than clobber list.
> As r15 is the stack pointer, adding r15(SP) to clobber list might not
> work as expected(as we have no idea what compiler is going to do), One
> guess is that It will have unknown side effects(which is proved as shown
> below...).
>
> Yesterday, Stuart and I spend some time to test and see the side-effect
> of adding r15 to clobber list in xchg_u32 after reverting my changes on
> 7141(ST40) board, We got lucky.. and encountered crash in every boot.
>
> PC and PR ended up at 0x0 as r15 got corrupted because of the below code
> generated as result of r15 in clobber list.
>
>
> 0x8004b128 <+36>: mova 0x8004b134 <bdi_sync_supers+48>,r0
> 0x8004b12a <+38>: nop
> 0x8004b12c <+40>: mov r15,r1
> 0x8004b12e <+42>: mov #-4,r15
> 0x8004b130 <+44>: mov.l @r2,r3
> 0x8004b132 <+46>: mov.l r8,@r2
> 0x8004b134 <+48>: mov r1,r15
> 0x8004b136 <+50>: jsr @r10
> 0x8004b138 <+52>: nop
> 0x8004b13a <+54>: jsr @r9
> 0x8004b13c <+56>: nop
> 0x8004b13e <+58>: jsr @r11
> 0x8004b140 <+60>: nop
> 0x8004b142 <+62>: tst r0,r0
> => 0x8004b144 <+64>: bt.s 0x8004b124 <bdi_sync_supers+32>
> => 0x8004b146 <+66>: lds.l @r15+,pr
> 0x8004b148 <+68>: mov.l @r15+,r11
> 0x8004b14a <+70>: mov #0,r0
> 0x8004b14c <+72>: mov.l @r15+,r10
> 0x8004b14e <+74>: mov.l @r15+,r9
> 0x8004b150 <+76>: rts
>
>
> If you look at the highlighted lines of code, bt is delayed branch
> instruction, which ends up incrementing r15 for every loop, as a result
> we see PC and PR end's up at 0.
>
> with latest code (with my patches) below code is generated for the same
> subroutine.
> 298: 02 c7 mova 2a4 <bdi_sync_supers+0x34>,r0
> 29a: 09 00 nop
> 29c: f3 61 mov r15,r1
> 29e: fc ef mov #-4,r15
> 2a0: 22 67 mov.l @r2,r7
> 2a2: 32 22 mov.l r3,@r2
> 2a4: 13 6f mov r1,r15
> 2a6: 0b 49 jsr @r9
> 2a8: 09 00 nop
> 2aa: 0b 48 jsr @r8
> 2ac: 09 00 nop
> 2ae: 0b 4b jsr @r11
> 2b0: 09 00 nop
> 2b2: 08 20 tst r0,r0
> => 2b4: ec 89 bt 290 <bdi_sync_supers+0x20>
> => 2b6: 26 4f lds.l @r15+,pr
> 2b8: f6 6b mov.l @r15+,r11
> 2ba: 00 e0 mov #0,r0
> 2bc: f6 6a mov.l @r15+,r10
> 2be: f6 69 mov.l @r15+,r9
> 2c0: 0b 00 rts
>
>
> There might be other such instances of side effects of r15(stack
> pointer) in clobber list.
> I hope we answered why 'val' ended up in input/output constraints rather
> than r15 in clobber list.
>
> Thanks,
> srini
>
Hi Srini,
yes this clarified the reason. What about adding the same constraint to
'm' as well. I understand there are not failures up to now, but it could
happen as well.
Cheers,
Carmelo
> Carmelo AMOROSO wrote:
>> On 22/06/2011 9.05, Carmelo Amoroso wrote:
>>> ---------- Forwarded message ----------
>>> From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
>>> Date: 19 April 2011 18:28
>>> Subject: Re: sh:fixed issue in xchg_u32 function when val=r15.
>>> To: Paul Mundt <lethal@linux-sh.org>
>>> Cc: linux-sh@vger.kernel.org, Stuart Menefy <stuart.menefy@st.com>
>>>
>>>
>>> Hi Paul,
>>> Thanks for the suggestion.
>>>
>>> Attached is the reworked patch for potential gUSA rollback users.
>>>
>>> Thanks,
>>> srini
>>>
>>>
>>> Paul Mundt wrote:
>>>> On Thu, Apr 07, 2011 at 01:57:09PM +0100, Srinivas KANDAGATLA wrote:
>>>>
>>>>> Recently we have discovered a bug in xchg_u32 function of GUSA_RB
>>>>> feature.
>>>>> This function breaks if one of the input parameter 'val' is r15.
>>>>>
>>>>> 168: 02 c7 mova 174 <exit_mm+0x54>,r0
>>>>> 16a: 09 00 nop 16c: f3 61 mov
>>>>> r15,r1
>>>>> 16e: fc ef mov #-4,r15
>>>>>
>>>> The -4 here is part of the gUSA login sequence, so if you're seeing the
>>>> problem with gUSA based xchg_u32 it seems like you're going to have to
>>>> apply the same fix for all gUSA rollback users.
>>>>
>>
>> Hi Paul, Srini
>> instead of making 'val' an input/output contraint (to force gcc not
>> using r15 directly), should not be better (or more correct) to add r15
>> to the clobbered register list ? r15 is actually modified for the
>> login/logout gUSA sequence.
>> Indeed, 'val' is not an output parameter... so we are benefiting from a
>> side effect.
>>
>> Further, looking at __cmpxchg_u32, we think that we should make the
>> first argument (volatile int *m) an input/output constraint as well (as
>> it happens in xchg_u8 and xchg_32), and include r15 instead in the
>> clobber list, keeping 'val' just an input parameter.
>>
>> Comments are welcome.
>>
>> Cheers,
>> Carmelo & Salvo
>>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread