Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new uart0-helloworld-sdboot.sunxi bootable test image #44

Merged
merged 1 commit into from
Jun 3, 2016

Conversation

ssvb
Copy link
Contributor

@ssvb ssvb commented May 12, 2016

This is a universal bootable image, which just prints a hello message on UART0. For example, it is useful as a test payload when debugging the SPI boot functionality.

Still no support for A23, A33, A83T and A80, because I don't have any of these devices to test.

Patch revision v1 (53f7f14).

@ssvb
Copy link
Contributor Author

ssvb commented May 14, 2016

Updated the pull request. Changes in v2 (5c9331a):

ssvb added a commit to ssvb/u-boot-sunxi that referenced this pull request May 14, 2016
Now we can easily test SPI boot for any A10/A20/H3/A64 board without
all the hassle compiling and embedding a suitable U-Boot SPL or
boot0 blob.

Also see linux-sunxi/sunxi-tools#44

Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
@n1tehawk
Copy link
Collaborator

Nice work. I consider this very useful - not only as a test payload per se, but also in gaining the ability to make use of UART output for diagnostic purposes, e.g. when debugging the BROM → SPL transition.

@n1tehawk
Copy link
Collaborator

I've been struggling quite a bit with my armv7a-hardfloat-linux-gnueabi-gcc-4.9.3 compiler to get this working on a Banana Pi. The A20 would happily print Hello from Allwinner A20! and then just sit there, obviously not getting through the get_boot_device() function.

After a while I could narrow the hang down to the source line doing the spl_header[1] and spl_header[2] comparison, which seemed very odd. Upon closer inspection of the generated assembly, I finally found out that gcc-4.9.3 had inserted an undefined instruction (udf #0) - something that gcc-4.8.3 did not. Actually the udf #0 translates into __builtin_trap, which finally pointed me to an explanation: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63803.

The root cause is that spl_header equals a NULL pointer for those SoCs where the SPL starts at address 0x0. Dereferencing this pointer is "undefined behaviour" according to the C standard. The bugtracker comment by Marc Glisse seems to indicate the proper solution: For gcc 4.9.x we need to pass -fno-delete-null-pointer-checks to tell the compiler that dereferencing NULL is okay for us.

After adding this to ARM_ELF_FLAGS, the gcc-4.9.3 compiled binary works just fine - e.g. it returns gracefully to FEL.

@n1tehawk
Copy link
Collaborator

After adding the above -fno-delete-null-pointer-checks, I have successfully tested the resulting binary on A20 (Banana Pi) and A64 (Pine 64+) - both via FEL. I'll add a "reviewed" label now...

@ssvb
Copy link
Contributor Author

ssvb commented May 19, 2016

Oh, thanks for investigating this. My only concern is that the -fno-delete-null-pointer-checks option might be not supported by some compilers. We are slowly getting into the autotools or CMAKE territory with this stuff.

As an alternative solution, maybe inline assembly code could do the job and hide the NULL pointer from the compiler? Something like asm volatile("mov %0, #0" : "=r" (spl_header));?

@ssvb
Copy link
Contributor Author

ssvb commented May 19, 2016

Or we could pretend that the SPL header starts at the address 4, skipping the initial branch instruction.

@n1tehawk
Copy link
Collaborator

I just came up with that idea, too... Let's make it a "signature" pointer at 0x4 or 0x10004. 😃 I'll retest that when I get back home later (~20:30 UTC).

I also dislike having to add a bunch of compiler options to the Makefile that are hard to track / understand, and might not work well across a wide range of compilers.

@ssvb ssvb changed the title [RFC] Add new uart0-helloworld-sdboot.sunxi bootable test image Add new uart0-helloworld-sdboot.sunxi bootable test image May 19, 2016
@ssvb
Copy link
Contributor Author

ssvb commented May 19, 2016

BTW, this test program also relies on the existing stack setup. The BROM seesm to configure the initial stack location in a reasonable way (when booting from the SD card, eMMc, NAND, etc.), see the "Initial stack pointer value (SP register) " column in the table at https://linux-sunxi.org/BROM#U-Boot_SPL_limitations for more details.

As for the FEL mode, currently the stack is set up just below the thunk code. Most of the SoC variants have a lot of room for the stack below the thunk code in the FEL mode, but A31 and A80 are a little bit problematic. Either we need to change the thunk code location on A31 and A80 to provide more space for the stack, or the uart0-helloworld-sdboot.sunxi needs to setup it's own stack.

@n1tehawk
Copy link
Collaborator

Using an offset of 0x4 (= the start of the SPL signature) avoids the NULL pointer quirk without changes to the Makefile / ARM_ELF_FLAGS: https://gist.github.com/n1tehawk/776bdd1e3f0fdeb04deefbec3b531bdd. A test on A20 passed without problems.

I also like the idea of converting readl to a low-level assembly function, which shouldn't be affected by this issue. I tried to hack up a quick PoC, but failed so far...

@n1tehawk
Copy link
Collaborator

n1tehawk commented Jun 1, 2016

Siarhei, in case you're short on time / too busy to submit a v2: I'm also okay with merging this as-is, augmented by a fixup commit that addresses the possible "NULL pointer dereference" (by doing the suggested change to offset 0x4).

This is a universal bootable image, which just prints a hello
message on UART0. For example, it is useful as a test payload
when debugging the SPI boot functionality:
   https://linux-sunxi.org/Bootable_SPI_flash

Changes in v2:
 - A workaround for https://patchwork.ozlabs.org/patch/622173
 - Also print the type of the used bootable media

Changes in v3:
 - Bernhard Nortmann's workaround for
   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63803

More details in linux-sunxi#44

Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
@ssvb ssvb force-pushed the 20160512-uart0-helloworld branch from 5c9331a to 958143e Compare June 3, 2016 03:12
@ssvb
Copy link
Contributor Author

ssvb commented Jun 3, 2016

Updated the pull request. Changes in v3 (958143e ):

@ssvb ssvb merged commit a455787 into linux-sunxi:master Jun 3, 2016
@ssvb
Copy link
Contributor Author

ssvb commented Jun 3, 2016

Siarhei, in case you're short on time / too busy to submit a v2: I'm also okay with merging this as-is, augmented by a fixup commit that addresses the possible "NULL pointer dereference" (by doing the suggested change to offset 0x4).

Thanks, merged. We can do additional improvements (A80 support?) via other pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants