public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport
@ 2013-06-13  0:02 Chris Packham
  2013-06-13  1:16 ` Chris Packham
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Packham @ 2013-06-13  0:02 UTC (permalink / raw)
  To: u-boot

Hi,

I've just found a crash in usb_stor_get_info (actually usb_inquiry
which gets auto-inlined). The cause seems to be that ss->transport is
set to the pre-relocation address of usb_stor_BBB_transport. Yet
ss->transport_reset is set to the correct relocated address of.

The difference between the two is that usb_stor_BBB_reset is declared
static and usb_stor_BBB_transport is not. Changing
usb_stor_BBB_transport to a static makes things work but I notice that
none of the other transport functions are static either so I'm
thinking I haven't actually fixed the problem rather just masked it.

I did  some poking with a lauterbach and from the disassembly it looks
like there is a translation table being used when the function
pointers are setup by usb_storage_probe and when declared normally
usb_stor_BBB_transport ends up at the end. Everything else has the
correct relocated address so I wonder if there is an off-by-one error
in whatever creates that table.

Does this sound familiar to anyone.

Thanks,
Chris

Extra debug info:

Board: Custom design based on P2041RDB
u-boot version: based on U-Boot 2012.10

=> usb start
(Re)start USB...
USB:   Register 10011 NbrPorts 1
USB EHCI 1.00
scanning bus for devices... 2 USB Device(s) found
       scanning bus for storage devices... i=0
i=1


USB Mass Storage device detected
Transport: Bulk/Bulk/Bulk
Endpoints In 1 Out 2 Int 0
Get Max LUN -> len = 1, result = 0
dev_desc 7ffbbe4c
&usb_ccb 7ffbc0c0
pccb 7ffbc0c0
 address 2
srb 7ffbc0c0
ss 7ffbbd48
ss->transport fffa98f4
ss->transport_reset 7ff586e8
Bad trap at PC: fffa98f4, SR: 29200, vector=e00
NIP: FFFA98F4 XER: 00000000 LR: 7FF59F24 REGS: 7fd2daa0 TRAP: 0e00 DAR: 00000000
MSR: 00029200 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 00

GPR00: FFFA98F4 7FD2DB90 7FD2DF30 7FFBC0C0 7FFBBD48 00000000 7FFBC0C8 00000010
GPR08: FFFFFFFE 00000020 00000020 7FD2DB90 42022044 21A40D64 7FD2E238 00000000
GPR16: 7FD2EA28 00000000 00000000 00000000 00000000 7FF94E29 00000012 00000024
GPR24: 00000024 0000000C 7FF94D18 7FD2DBC0 7FFBBD48 7FFBC0C0 7FF9BE78 7FFBBE4C
Call backtrace:
7FF59EDC 7FF5A35C 7FF4BBD0 7FF4CA48 7FF51E24 7FF5247C 7FF52600
7FF56178 7FF38C4C 7FF31650
Exception in kernel pc fffa98f4 signal 0

usb_inquiry
usb_storage.c:922
usb_stor_scan
usb_storage.c:280
do_usb
cmd_usb.c:388
cmd_process
command.c:544
run_pipe_real
hush.c:1668
run_list
hush.c:2021
parse_file_outer
hush.c:3273
main_loop
main.c:431
board_init_r
board.c:1089
trap_init
start.S:1824

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport
  2013-06-13  0:02 [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport Chris Packham
@ 2013-06-13  1:16 ` Chris Packham
  2013-06-13  5:43   ` Albert ARIBAUD
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Packham @ 2013-06-13  1:16 UTC (permalink / raw)
  To: u-boot

On Thu, Jun 13, 2013 at 12:02 PM, Chris Packham <judge.packham@gmail.com> wrote:
> Hi,
>
> I've just found a crash in usb_stor_get_info (actually usb_inquiry
> which gets auto-inlined). The cause seems to be that ss->transport is
> set to the pre-relocation address of usb_stor_BBB_transport. Yet
> ss->transport_reset is set to the correct relocated address of.
>
> The difference between the two is that usb_stor_BBB_reset is declared
> static and usb_stor_BBB_transport is not. Changing
> usb_stor_BBB_transport to a static makes things work but I notice that
> none of the other transport functions are static either so I'm
> thinking I haven't actually fixed the problem rather just masked it.

Actually I see commit 199adb60 (common/misc: sparse fixes) does change
the transport functions to static. Which is the change I was looking
at. I still don't know if it is fixing a problem or masking a
different one but this is probably why no-one else is complaining that
their usb mass storage devices are causing crashes. I'll cherry-pick
this to fix my problem.

>
> I did  some poking with a lauterbach and from the disassembly it looks
> like there is a translation table being used when the function
> pointers are setup by usb_storage_probe and when declared normally
> usb_stor_BBB_transport ends up at the end. Everything else has the
> correct relocated address so I wonder if there is an off-by-one error
> in whatever creates that table.
>
> Does this sound familiar to anyone.
>
> Thanks,
> Chris
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport
  2013-06-13  1:16 ` Chris Packham
@ 2013-06-13  5:43   ` Albert ARIBAUD
  2013-06-13 10:19     ` Chris Packham
  0 siblings, 1 reply; 7+ messages in thread
From: Albert ARIBAUD @ 2013-06-13  5:43 UTC (permalink / raw)
  To: u-boot

Hi Chris,

On Thu, 13 Jun 2013 13:16:17 +1200, Chris Packham
<judge.packham@gmail.com> wrote:

> On Thu, Jun 13, 2013 at 12:02 PM, Chris Packham <judge.packham@gmail.com> wrote:
> > Hi,
> >
> > I've just found a crash in usb_stor_get_info (actually usb_inquiry
> > which gets auto-inlined). The cause seems to be that ss->transport is
> > set to the pre-relocation address of usb_stor_BBB_transport. Yet
> > ss->transport_reset is set to the correct relocated address of.
> >
> > The difference between the two is that usb_stor_BBB_reset is declared
> > static and usb_stor_BBB_transport is not. Changing
> > usb_stor_BBB_transport to a static makes things work but I notice that
> > none of the other transport functions are static either so I'm
> > thinking I haven't actually fixed the problem rather just masked it.
> 
> Actually I see commit 199adb60 (common/misc: sparse fixes) does change
> the transport functions to static. Which is the change I was looking
> at. I still don't know if it is fixing a problem or masking a
> different one but this is probably why no-one else is complaining that
> their usb mass storage devices are causing crashes. I'll cherry-pick
> this to fix my problem.
> 
> >
> > I did  some poking with a lauterbach and from the disassembly it looks
> > like there is a translation table being used when the function
> > pointers are setup by usb_storage_probe and when declared normally
> > usb_stor_BBB_transport ends up at the end. Everything else has the
> > correct relocated address so I wonder if there is an off-by-one error
> > in whatever creates that table.

Can you elaborate? The only relocation-related table that I know of is
the one used in relocate_code(), and no other relocation-fix table
exists or is used anywhere else.

> > Does this sound familiar to anyone.

Familiar, no, but it does set in my mind, if not a blaring alarm with
flashing beacons, at least a blinking red light with a beep, so let's
analyize this.

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport
  2013-06-13  5:43   ` Albert ARIBAUD
@ 2013-06-13 10:19     ` Chris Packham
  2013-06-13 11:24       ` Albert ARIBAUD
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Packham @ 2013-06-13 10:19 UTC (permalink / raw)
  To: u-boot

Hi Albert,

On 13/06/13 17:43, Albert ARIBAUD wrote:
> Hi Chris,
> 
> On Thu, 13 Jun 2013 13:16:17 +1200, Chris Packham
> <judge.packham@gmail.com> wrote:
> 
>> On Thu, Jun 13, 2013 at 12:02 PM, Chris Packham <judge.packham@gmail.com> wrote:
>>> Hi,
>>>
>>> I've just found a crash in usb_stor_get_info (actually usb_inquiry
>>> which gets auto-inlined). The cause seems to be that ss->transport is
>>> set to the pre-relocation address of usb_stor_BBB_transport. Yet
>>> ss->transport_reset is set to the correct relocated address of.
>>>
>>> The difference between the two is that usb_stor_BBB_reset is declared
>>> static and usb_stor_BBB_transport is not. Changing
>>> usb_stor_BBB_transport to a static makes things work but I notice that
>>> none of the other transport functions are static either so I'm
>>> thinking I haven't actually fixed the problem rather just masked it.
>>
>> Actually I see commit 199adb60 (common/misc: sparse fixes) does change
>> the transport functions to static. Which is the change I was looking
>> at. I still don't know if it is fixing a problem or masking a
>> different one but this is probably why no-one else is complaining that
>> their usb mass storage devices are causing crashes. I'll cherry-pick
>> this to fix my problem.
>>
>>>
>>> I did  some poking with a lauterbach and from the disassembly it looks
>>> like there is a translation table being used when the function
>>> pointers are setup by usb_storage_probe and when declared normally
>>> usb_stor_BBB_transport ends up at the end. Everything else has the
>>> correct relocated address so I wonder if there is an off-by-one error
>>> in whatever creates that table.
> 
> Can you elaborate? The only relocation-related table that I know of is
> the one used in relocate_code(), and no other relocation-fix table
> exists or is used anywhere else.
> 
>>> Does this sound familiar to anyone.
> 
> Familiar, no, but it does set in my mind, if not a blaring alarm with
> flashing beacons, at least a blinking red light with a beep, so let's
> analyize this.
> 
> Amicalement,
> 

I'm at home right now so I don't have the board in front of me. Here's
some disassembly that gdb gives me

int usb_stor_BBB_transport(); (without 199adb60)

1272            case US_PR_BULK:
1273                    USB_STOR_PRINTF("Bulk/Bulk/Bulk\n");
1274                    ss->transport = usb_stor_BBB_transport;
   0xfffa9780 <+208>:   lwz     r0,-4(r30)
   0xfffa9784 <+212>:   stw     r0,48(r31)

1275                    ss->transport_reset = usb_stor_BBB_reset;
   0xfffa9788 <+216>:   lwz     r0,-4268(r30)
   0xfffa978c <+220>:   b       0xfffa9770 <usb_storage_probe+192>

1276                    break;

static int usb_stor_BBB_transport(); (with 199adb60)

1261            case US_PR_CB:
1262                    USB_STOR_PRINTF("Control/Bulk\n");
1263                    ss->transport = usb_stor_CB_transport;
   0xfffa9608 <+180>:   lwz     r0,-4240(r30)
   0xfffa960c <+184>:   stw     r0,48(r31)

1264                    ss->transport_reset = usb_stor_CB_reset;
   0xfffa9610 <+188>:   lwz     r0,-4248(r30)
   0xfffa9614 <+192>:   stw     r0,44(r31)

1265                    break;

So r30 is the table thing I was talking about. I'm assuming it's
something maintained by the compiler/linker. From memory -4(r30) was
0xfffaabcd everything else (including -4268(r30)) seemed to be the
relocated address for various symbols, hence my comment about a possible
off-by-one in whatever maintains that table.

Because it's probably relevant here are my compiler details
  $ powerpc-e500-linux-gnu-gcc --version
  powerpc-e500-linux-gnu-gcc (Gentoo 4.6.3-r1 p1.9, pie-0.5.2) 4.6.3

When I get back to work tomorrow I can post a dump of r30 from a running
system.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport
  2013-06-13 10:19     ` Chris Packham
@ 2013-06-13 11:24       ` Albert ARIBAUD
  2013-06-13 22:48         ` Chris Packham
  0 siblings, 1 reply; 7+ messages in thread
From: Albert ARIBAUD @ 2013-06-13 11:24 UTC (permalink / raw)
  To: u-boot

Hi Chris,

On Thu, 13 Jun 2013 22:19:54 +1200, Chris Packham
<judge.packham@gmail.com> wrote:

> Hi Albert,
> 
> On 13/06/13 17:43, Albert ARIBAUD wrote:
> > Hi Chris,
> > 
> > On Thu, 13 Jun 2013 13:16:17 +1200, Chris Packham
> > <judge.packham@gmail.com> wrote:
> > 
> >> On Thu, Jun 13, 2013 at 12:02 PM, Chris Packham <judge.packham@gmail.com> wrote:
> >>> Hi,
> >>>
> >>> I've just found a crash in usb_stor_get_info (actually usb_inquiry
> >>> which gets auto-inlined). The cause seems to be that ss->transport is
> >>> set to the pre-relocation address of usb_stor_BBB_transport. Yet
> >>> ss->transport_reset is set to the correct relocated address of.
> >>>
> >>> The difference between the two is that usb_stor_BBB_reset is declared
> >>> static and usb_stor_BBB_transport is not. Changing
> >>> usb_stor_BBB_transport to a static makes things work but I notice that
> >>> none of the other transport functions are static either so I'm
> >>> thinking I haven't actually fixed the problem rather just masked it.
> >>
> >> Actually I see commit 199adb60 (common/misc: sparse fixes) does change
> >> the transport functions to static. Which is the change I was looking
> >> at. I still don't know if it is fixing a problem or masking a
> >> different one but this is probably why no-one else is complaining that
> >> their usb mass storage devices are causing crashes. I'll cherry-pick
> >> this to fix my problem.
> >>
> >>>
> >>> I did  some poking with a lauterbach and from the disassembly it looks
> >>> like there is a translation table being used when the function
> >>> pointers are setup by usb_storage_probe and when declared normally
> >>> usb_stor_BBB_transport ends up at the end. Everything else has the
> >>> correct relocated address so I wonder if there is an off-by-one error
> >>> in whatever creates that table.
> > 
> > Can you elaborate? The only relocation-related table that I know of is
> > the one used in relocate_code(), and no other relocation-fix table
> > exists or is used anywhere else.
> > 
> >>> Does this sound familiar to anyone.
> > 
> > Familiar, no, but it does set in my mind, if not a blaring alarm with
> > flashing beacons, at least a blinking red light with a beep, so let's
> > analyize this.
> > 
> > Amicalement,
> > 
> 
> I'm at home right now so I don't have the board in front of me. Here's
> some disassembly that gdb gives me
> 
> int usb_stor_BBB_transport(); (without 199adb60)
> 
> 1272            case US_PR_BULK:
> 1273                    USB_STOR_PRINTF("Bulk/Bulk/Bulk\n");
> 1274                    ss->transport = usb_stor_BBB_transport;
>    0xfffa9780 <+208>:   lwz     r0,-4(r30)
>    0xfffa9784 <+212>:   stw     r0,48(r31)
> 
> 1275                    ss->transport_reset = usb_stor_BBB_reset;
>    0xfffa9788 <+216>:   lwz     r0,-4268(r30)
>    0xfffa978c <+220>:   b       0xfffa9770 <usb_storage_probe+192>
> 
> 1276                    break;
> 
> static int usb_stor_BBB_transport(); (with 199adb60)
> 
> 1261            case US_PR_CB:
> 1262                    USB_STOR_PRINTF("Control/Bulk\n");
> 1263                    ss->transport = usb_stor_CB_transport;
>    0xfffa9608 <+180>:   lwz     r0,-4240(r30)
>    0xfffa960c <+184>:   stw     r0,48(r31)
> 
> 1264                    ss->transport_reset = usb_stor_CB_reset;
>    0xfffa9610 <+188>:   lwz     r0,-4248(r30)
>    0xfffa9614 <+192>:   stw     r0,44(r31)
> 
> 1265                    break;
> 
> So r30 is the table thing I was talking about. I'm assuming it's
> something maintained by the compiler/linker. From memory -4(r30) was
> 0xfffaabcd everything else (including -4268(r30)) seemed to be the
> relocated address for various symbols, hence my comment about a possible
> off-by-one in whatever maintains that table.
> 
> Because it's probably relevant here are my compiler details
>   $ powerpc-e500-linux-gnu-gcc --version
>   powerpc-e500-linux-gnu-gcc (Gentoo 4.6.3-r1 p1.9, pie-0.5.2) 4.6.3
> 
> When I get back to work tomorrow I can post a dump of r30 from a running
> system.

So that's PPC. Maybe PPC does manual fixing -- I was being ARM-centric
there.

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport
  2013-06-13 11:24       ` Albert ARIBAUD
@ 2013-06-13 22:48         ` Chris Packham
  2013-06-14  2:21           ` Chris Packham
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Packham @ 2013-06-13 22:48 UTC (permalink / raw)
  To: u-boot

On Thu, Jun 13, 2013 at 11:24 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Chris,
>
> On Thu, 13 Jun 2013 22:19:54 +1200, Chris Packham
> <judge.packham@gmail.com> wrote:
>
>> Hi Albert,
>>
>> On 13/06/13 17:43, Albert ARIBAUD wrote:
>> > Hi Chris,
>> >
>> > On Thu, 13 Jun 2013 13:16:17 +1200, Chris Packham
>> > <judge.packham@gmail.com> wrote:
>> >
>> >> On Thu, Jun 13, 2013 at 12:02 PM, Chris Packham <judge.packham@gmail.com> wrote:
>> >>> Hi,
>> >>>
>> >>> I've just found a crash in usb_stor_get_info (actually usb_inquiry
>> >>> which gets auto-inlined). The cause seems to be that ss->transport is
>> >>> set to the pre-relocation address of usb_stor_BBB_transport. Yet
>> >>> ss->transport_reset is set to the correct relocated address of.
>> >>>
>> >>> The difference between the two is that usb_stor_BBB_reset is declared
>> >>> static and usb_stor_BBB_transport is not. Changing
>> >>> usb_stor_BBB_transport to a static makes things work but I notice that
>> >>> none of the other transport functions are static either so I'm
>> >>> thinking I haven't actually fixed the problem rather just masked it.
>> >>
>> >> Actually I see commit 199adb60 (common/misc: sparse fixes) does change
>> >> the transport functions to static. Which is the change I was looking
>> >> at. I still don't know if it is fixing a problem or masking a
>> >> different one but this is probably why no-one else is complaining that
>> >> their usb mass storage devices are causing crashes. I'll cherry-pick
>> >> this to fix my problem.
>> >>
>> >>>
>> >>> I did  some poking with a lauterbach and from the disassembly it looks
>> >>> like there is a translation table being used when the function
>> >>> pointers are setup by usb_storage_probe and when declared normally
>> >>> usb_stor_BBB_transport ends up at the end. Everything else has the
>> >>> correct relocated address so I wonder if there is an off-by-one error
>> >>> in whatever creates that table.
>> >
>> > Can you elaborate? The only relocation-related table that I know of is
>> > the one used in relocate_code(), and no other relocation-fix table
>> > exists or is used anywhere else.
>> >
>> >>> Does this sound familiar to anyone.
>> >
>> > Familiar, no, but it does set in my mind, if not a blaring alarm with
>> > flashing beacons, at least a blinking red light with a beep, so let's
>> > analyize this.
>> >
>> > Amicalement,
>> >
>>
>> I'm at home right now so I don't have the board in front of me. Here's
>> some disassembly that gdb gives me
>>
>> int usb_stor_BBB_transport(); (without 199adb60)
>>
>> 1272            case US_PR_BULK:
>> 1273                    USB_STOR_PRINTF("Bulk/Bulk/Bulk\n");
>> 1274                    ss->transport = usb_stor_BBB_transport;
>>    0xfffa9780 <+208>:   lwz     r0,-4(r30)
>>    0xfffa9784 <+212>:   stw     r0,48(r31)
>>
>> 1275                    ss->transport_reset = usb_stor_BBB_reset;
>>    0xfffa9788 <+216>:   lwz     r0,-4268(r30)
>>    0xfffa978c <+220>:   b       0xfffa9770 <usb_storage_probe+192>
>>
>> 1276                    break;
>>
>> static int usb_stor_BBB_transport(); (with 199adb60)
>>
>> 1261            case US_PR_CB:
>> 1262                    USB_STOR_PRINTF("Control/Bulk\n");
>> 1263                    ss->transport = usb_stor_CB_transport;
>>    0xfffa9608 <+180>:   lwz     r0,-4240(r30)
>>    0xfffa960c <+184>:   stw     r0,48(r31)
>>
>> 1264                    ss->transport_reset = usb_stor_CB_reset;
>>    0xfffa9610 <+188>:   lwz     r0,-4248(r30)
>>    0xfffa9614 <+192>:   stw     r0,44(r31)
>>
>> 1265                    break;
>>
>> So r30 is the table thing I was talking about. I'm assuming it's
>> something maintained by the compiler/linker. From memory -4(r30) was
>> 0xfffaabcd everything else (including -4268(r30)) seemed to be the
>> relocated address for various symbols, hence my comment about a possible
>> off-by-one in whatever maintains that table.
>>
>> Because it's probably relevant here are my compiler details
>>   $ powerpc-e500-linux-gnu-gcc --version
>>   powerpc-e500-linux-gnu-gcc (Gentoo 4.6.3-r1 p1.9, pie-0.5.2) 4.6.3
>>
>> When I get back to work tomorrow I can post a dump of r30 from a running
>> system.
>
> So that's PPC. Maybe PPC does manual fixing -- I was being ARM-centric
> there.
>
> Amicalement,
> --
> Albert.

Another relevant detail that I left out is that I'm booting via the
CPC (from SPI or NOR). So the pre-relocation address is definitely
invalid for me, but it might be valid for others (problems with
re-flashing the board aside).

Here's some info I was able to extract using the lauterbach.

R(R30) = 7FF9AE00
R(R30) - 0x4    7FF9ADFC = FFFA94A8
R(R30) - 0x10AC 7FF99D54 = 7FF586E8
.u-boot\usb_storage\usb_stor_BBB_reset
R(R30) - 0x8    7FF9ADF8 = 7FFB017C  .u-boot\Global\NetArpWaitTimerStart


And fom u-boot.map
 .got           0x00000000fffe8024     0x2de8 arch/powerpc/cpu/mpc85xx/start.o
                0x00000000fffeae00                _GLOBAL_OFFSET_TABLE_
                0x00000000fffeae10                PROVIDE
(_GLOBAL_OFFSET_TABLE_, (. + 0x4))
                0x00000000fffeae0c                _FIXUP_TABLE_ = .

so 7FF9AE00 is the relocated address of _GLOBAL_OFFSET_TABLE_

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport
  2013-06-13 22:48         ` Chris Packham
@ 2013-06-14  2:21           ` Chris Packham
  0 siblings, 0 replies; 7+ messages in thread
From: Chris Packham @ 2013-06-14  2:21 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 14, 2013 at 10:48 AM, Chris Packham <judge.packham@gmail.com> wrote:
> On Thu, Jun 13, 2013 at 11:24 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Hi Chris,
>>
>> On Thu, 13 Jun 2013 22:19:54 +1200, Chris Packham
>> <judge.packham@gmail.com> wrote:
>>
>>> Hi Albert,
>>>
>>> On 13/06/13 17:43, Albert ARIBAUD wrote:
>>> > Hi Chris,
>>> >
>>> > On Thu, 13 Jun 2013 13:16:17 +1200, Chris Packham
>>> > <judge.packham@gmail.com> wrote:
>>> >
>>> >> On Thu, Jun 13, 2013 at 12:02 PM, Chris Packham <judge.packham@gmail.com> wrote:
>>> >>> Hi,
>>> >>>
>>> >>> I've just found a crash in usb_stor_get_info (actually usb_inquiry
>>> >>> which gets auto-inlined). The cause seems to be that ss->transport is
>>> >>> set to the pre-relocation address of usb_stor_BBB_transport. Yet
>>> >>> ss->transport_reset is set to the correct relocated address of.
>>> >>>
>>> >>> The difference between the two is that usb_stor_BBB_reset is declared
>>> >>> static and usb_stor_BBB_transport is not. Changing
>>> >>> usb_stor_BBB_transport to a static makes things work but I notice that
>>> >>> none of the other transport functions are static either so I'm
>>> >>> thinking I haven't actually fixed the problem rather just masked it.
>>> >>
>>> >> Actually I see commit 199adb60 (common/misc: sparse fixes) does change
>>> >> the transport functions to static. Which is the change I was looking
>>> >> at. I still don't know if it is fixing a problem or masking a
>>> >> different one but this is probably why no-one else is complaining that
>>> >> their usb mass storage devices are causing crashes. I'll cherry-pick
>>> >> this to fix my problem.
>>> >>
>>> >>>
>>> >>> I did  some poking with a lauterbach and from the disassembly it looks
>>> >>> like there is a translation table being used when the function
>>> >>> pointers are setup by usb_storage_probe and when declared normally
>>> >>> usb_stor_BBB_transport ends up at the end. Everything else has the
>>> >>> correct relocated address so I wonder if there is an off-by-one error
>>> >>> in whatever creates that table.
>>> >
>>> > Can you elaborate? The only relocation-related table that I know of is
>>> > the one used in relocate_code(), and no other relocation-fix table
>>> > exists or is used anywhere else.
>>> >
>>> >>> Does this sound familiar to anyone.
>>> >
>>> > Familiar, no, but it does set in my mind, if not a blaring alarm with
>>> > flashing beacons, at least a blinking red light with a beep, so let's
>>> > analyize this.
>>> >
>>> > Amicalement,
>>> >
>>>
>>> I'm at home right now so I don't have the board in front of me. Here's
>>> some disassembly that gdb gives me
>>>
>>> int usb_stor_BBB_transport(); (without 199adb60)
>>>
>>> 1272            case US_PR_BULK:
>>> 1273                    USB_STOR_PRINTF("Bulk/Bulk/Bulk\n");
>>> 1274                    ss->transport = usb_stor_BBB_transport;
>>>    0xfffa9780 <+208>:   lwz     r0,-4(r30)
>>>    0xfffa9784 <+212>:   stw     r0,48(r31)
>>>
>>> 1275                    ss->transport_reset = usb_stor_BBB_reset;
>>>    0xfffa9788 <+216>:   lwz     r0,-4268(r30)
>>>    0xfffa978c <+220>:   b       0xfffa9770 <usb_storage_probe+192>
>>>
>>> 1276                    break;
>>>
>>> static int usb_stor_BBB_transport(); (with 199adb60)
>>>
>>> 1261            case US_PR_CB:
>>> 1262                    USB_STOR_PRINTF("Control/Bulk\n");
>>> 1263                    ss->transport = usb_stor_CB_transport;
>>>    0xfffa9608 <+180>:   lwz     r0,-4240(r30)
>>>    0xfffa960c <+184>:   stw     r0,48(r31)
>>>
>>> 1264                    ss->transport_reset = usb_stor_CB_reset;
>>>    0xfffa9610 <+188>:   lwz     r0,-4248(r30)
>>>    0xfffa9614 <+192>:   stw     r0,44(r31)
>>>
>>> 1265                    break;
>>>
>>> So r30 is the table thing I was talking about. I'm assuming it's
>>> something maintained by the compiler/linker. From memory -4(r30) was
>>> 0xfffaabcd everything else (including -4268(r30)) seemed to be the
>>> relocated address for various symbols, hence my comment about a possible
>>> off-by-one in whatever maintains that table.
>>>
>>> Because it's probably relevant here are my compiler details
>>>   $ powerpc-e500-linux-gnu-gcc --version
>>>   powerpc-e500-linux-gnu-gcc (Gentoo 4.6.3-r1 p1.9, pie-0.5.2) 4.6.3
>>>
>>> When I get back to work tomorrow I can post a dump of r30 from a running
>>> system.
>>
>> So that's PPC. Maybe PPC does manual fixing -- I was being ARM-centric
>> there.
>>
>> Amicalement,
>> --
>> Albert.
>
> Another relevant detail that I left out is that I'm booting via the
> CPC (from SPI or NOR). So the pre-relocation address is definitely
> invalid for me, but it might be valid for others (problems with
> re-flashing the board aside).
>
> Here's some info I was able to extract using the lauterbach.
>
> R(R30) = 7FF9AE00
> R(R30) - 0x4    7FF9ADFC = FFFA94A8
> R(R30) - 0x10AC 7FF99D54 = 7FF586E8
> .u-boot\usb_storage\usb_stor_BBB_reset
> R(R30) - 0x8    7FF9ADF8 = 7FFB017C  .u-boot\Global\NetArpWaitTimerStart
>
>
> And fom u-boot.map
>  .got           0x00000000fffe8024     0x2de8 arch/powerpc/cpu/mpc85xx/start.o
>                 0x00000000fffeae00                _GLOBAL_OFFSET_TABLE_
>                 0x00000000fffeae10                PROVIDE
> (_GLOBAL_OFFSET_TABLE_, (. + 0x4))
>                 0x00000000fffeae0c                _FIXUP_TABLE_ = .
>
> so 7FF9AE00 is the relocated address of _GLOBAL_OFFSET_TABLE_

More digging.

Albert, this is all looking PowerPC specific so I'm not sure how much
you care. I'll dump what I've found here in this thread in case
someone else wants to look at it in the future. For now 199adb60 fixes
my problem.

The following code snippet from arch/powerpc/cpu/mpc85xx/start.S
handles the relocation for the global offset table.

in_ram:

    /*
     * Relocation Function, r12 point to got2+0x8000
     *
     * Adjust got2 pointers, no need to check for 0, this code
     * already puts a few entries in the table.
     */
    li    r0,__got2_entries at sectoff@l
    la    r3,GOT(_GOT2_TABLE_)
    lwz    r11,GOT(_GOT2_TABLE_)
    mtctr    r0
    sub    r11,r3,r11
    addi    r3,r3,-4
1:    lwzu    r0,4(r3)
    cmpwi    r0,0
    beq-    2f
    add    r0,r0,r11
    stw    r0,0(r3)
2:    bdnz    1b

Walking through this code I can see all the entries get changed to
their relocated addresses, except for the last one which happens to be
usb_stor_BBB_transport. So as I suspected there is probably an off by
one error in the code snippet above, nothing leaps out at me as
obviously wrong but my PowerPC assembly is a bit rusty.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-06-14  2:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-13  0:02 [U-Boot] crash in usb_stor_get_info using pre-relocation address for ss->transport Chris Packham
2013-06-13  1:16 ` Chris Packham
2013-06-13  5:43   ` Albert ARIBAUD
2013-06-13 10:19     ` Chris Packham
2013-06-13 11:24       ` Albert ARIBAUD
2013-06-13 22:48         ` Chris Packham
2013-06-14  2:21           ` Chris Packham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox