Enable boot module for arm
PATCH 1/1 Enable boot module for arm
Since the last merge of mainline into the arm branch, building for arm fails because commands/boot.c is not compiled anymore. The boot module should be enabled for arm too.
2013-07-21 Francesco Lavra <[email protected]>
* grub-core/Makefile.core.def (boot): Enable on ARM.
=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +1 -0
@@ -691,6 +691,7 @@ common = commands/boot.c; i386_pc = lib/i386/pc/biosnum.c; enable = x86; + enable = arm; enable = emu; enable = sparc64_ieee1275; enable = powerpc_ieee1275;
Quoted 4 lines · Francesco Lavra
Since the last merge of mainline into the arm branch, building for arm fails because
commands/boot.cis not compiled anymore. The boot module should be enabled for arm too.
Few lines down you have:
enable = arm_efi;
enable = arm_uboot;
Didn't it work for you? Did you redo autogen after checkout in the first place?
Quoted 21 lines
2013-07-21 Francesco Lavra <[email protected]>
* grub-core/Makefile.core.def (boot): Enable on ARM.
=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +1
-0@@ -691,6 +691,7 @@ common = commands/boot.c; i386_pc = lib/i386/pc/biosnum.c; enable = x86; + enable = arm; enable = emu; enable = sparc64_ieee1275; enable = powerpc_ieee1275;_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Quoted 11 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 4 lines · Francesco Lavra
Since the last merge of mainline into the arm branch, building for arm fails because
commands/boot.cis not compiled anymore. The boot module should be enabled for arm too.Few lines down you have:
enable =arm_efi;
enable =arm_uboot;Didn't it work for you? Did you redo autogen after checkout in the first place?
Actually this patch was against the arm branch, not trunk. Only later I noticed that the arm branch had been merged back into trunk, so maybe the arm branch is not going to be maintained anymore... in this case, sorry for the noise.
Quoted 10 lines · Francesco Lavra
Quoted 9 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 4 lines · Francesco Lavra
Since the last merge of mainline into the arm branch, building for arm fails because
commands/boot.cis not compiled anymore. The boot module should be enabled for arm too.Few lines down you have:
enable = arm_efi; enable = arm_uboot;
Quoted 8 lines
Quoted 2 lines
Didn't it work for you? Did you redo autogen after checkout in the first place?
Actually this patch was against the arm branch, not trunk. Only later I noticed that the arm branch had been merged back into trunk, so maybe the arm branch is not going to be maintained anymore... in this case, sorry for the noise.
ARM branch won't be maintained anymore. My main problem with arm currently is that testsuite works neither for arm-efi nor for arm-uboot and I didn't find a way to make qemu load file from iso from either u-boot or efi.
В Thu, 25 Jul 2013 17:04:34 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:
Quoted 5 lines
Quoted 1 line
ARM branch won't be maintained anymore. My main problem with arm currently is that testsuite works neither for arm-efi nor for arm-uboot and I didn't find a way to make qemu load file from iso from either u-boot or efi.
Could you show command line used to run grub under qemu? So far I was unsuccessful to find working combination. Do you need special u-boot build?
Quoted 10 lines · Andrey Borzenkov
В Thu, 25 Jul 2013 17:04:34 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:Quoted 5 lines
Quoted 1 line
ARM branch won't be maintained anymore. My main problem with arm currently is that testsuite works neither for arm-efi nor for arm-uboot and I didn't find a way to make qemu load file from iso from either u-boot or efi.
Quoted 3 lines
Could you show command line used to run grub under qemu? So far I was unsuccessful to find working combination. Do you need special u-boot build?
I have exactly the same problem. It's possible to support verstile/pb with armv5 but available u-boot tutorials for qemu ask you to concatenate payload with u-boot itself which is annoying to handle.
Quoted 6 lines
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
В Sat, 27 Jul 2013 06:21:56 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:
Quoted 11 lines · Andrey Borzenkov
Quoted 10 lines
В Thu, 25 Jul 2013 17:04:34 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:Quoted 5 lines
Quoted 1 line
ARM branch won't be maintained anymore. My main problem with arm currently is that testsuite works neither for arm-efi nor for arm-uboot and I didn't find a way to make qemu load file from iso from either u-boot or efi.
Quoted 6 lines
Quoted 3 lines
Could you show command line used to run grub under qemu? So far I was unsuccessful to find working combination. Do you need special u-boot build?
I have exactly the same problem. It's possible to support verstile/pb with armv5 but available u-boot tutorials for qemu ask you to concatenate payload with u-boot itself which is annoying to handle.
It would be the first step, but my problem is, I cannot make it work even using concatenation. u-boot does show me correct image info for embedded grub, but I cannot launch it. Using default load address 0x08000000 qemu errors out because address is not in RAM (and it will not let me change amount of RAM for versatilepb); I tried to change load address to something else, like 0x04000000 and it just immediately resets in this case after "bootm 0x04000000".
Quoted 8 lines
It would be the first step, but my problem is, I cannot make it work even using concatenation. u-boot does show me correct image info for embedded grub, but I cannot launch it. Using default load address
0x08000000qemu errors out because address is not in RAM (and it will not let me change amount of RAM for versatilepb); I tried to change load address to something else, like0x04000000and it just immediately resets in this case after "bootm0x04000000".
You probably have to change GRUB link address. And not that the address is where to find the source image not where to load to.
В Sat, 27 Jul 2013 16:44:39 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:
Quoted 11 lines
Quoted 8 lines
It would be the first step, but my problem is, I cannot make it work even using concatenation. u-boot does show me correct image info for embedded grub, but I cannot launch it. Using default load address
0x08000000qemu errors out because address is not in RAM (and it will not let me change amount of RAM for versatilepb); I tried to change load address to something else, like0x04000000and it just immediately resets in this case after "bootm0x04000000".You probably have to change GRUB link address. And not that the address is where to find the source image not where to load to.
I did try to change link address (arm_uboot_ldflags and GRUB_KERNEL_ARM_UBOOT_LINK_ADDR). As I understand, bootm is expected to relocate image to load address and jump to start address.
What I have now is
1. U-Boot is configured for versatileqemu. It works in the sense I can start example standalone hello_world using bootm.
2. Grub us build for arm-uboot
bor@opensuse:~/build/grub> ./configure --with-platform=uboot TARGET_CC=arm-linux-gnueabi-gcc --target=arm TARGET_LD=arm-linux-gnueabi-ld
(I also tried with TARGET_CFLAGS="-marm -mno-thumb-interwork -mabi=aapcs-linux -march=armv5te" to match U-Boot flags)
3. core.img is generated using
bor@opensuse:~/build/grub> ./grub-mkimage --verbose -d grub-core -O arm-uboot -o core.img
./grub-mkimage: info: the total module size is 0x20.
./grub-mkimage: info: getting the size of grub-core/kernel.img.
./grub-mkimage: info: reading grub-core/kernel.img.
./grub-mkimage: info: getting the size of grub-core/kernel.img.
./grub-mkimage: info: locating the section .text at 0x0.
./grub-mkimage: info: locating the section .rodata at 0x14750.
./grub-mkimage: info: locating the section .data at 0x165a0.
./grub-mkimage: info: locating the section .module_license at 0x16fa8.
./grub-mkimage: info: locating the section .bss at 0x16fc8.
./grub-mkimage: info: kernel_img=0x144dda0, kernel_size=0x16fc8.
./grub-mkimage: info: the core size is 0x16fe8.
./grub-mkimage: info: writing 0x17028 bytes.
bor@opensuse:~/build/grub> file core.img
core.img: u-boot legacy uImage, , Linux/ARM, OS Kernel Image (Not compressed), 94184 bytes, Sun Jul 28 20:46:54 2013, Load Address: 0x08000000, Entry Point: 0x08000000, Header CRC: 0x5F6F0B3E, Data CRC: 0x474F38F8
4. "flash" is built using
bor@opensuse:~/src/u-boot-2013.04> cat u-boot.bin ~/build/grub/core.img > test.bin
Offset of core.img is
bor@opensuse:~/src/u-boot-2013.04> printf "0x%X\n" $(expr $(stat -c%s u-boot.bin) + 65536) 0x2B558
5. I now can start it
bor@opensuse:~/src/u-boot-2013.04> qemu-system-arm -M versatilepb -m 256M -serial stdio -kernel test.bin
Warning: default mac address being used, creating potential for address conflict
audio: Unknown audio driver `pa'
audio: Run with -audio-help to list available drivers
U-Boot 2013.04 (Jul 28 2013 - 20:16:17)
DRAM: 128 MiB
WARNING: Caches not enabled
Using default environment
In: serial Out: serial Err: serial Net: SMC91111-0 Warning: SMC91111-0 using MAC address from net device
VersatilePB #
And it shows correct image info
VersatilePB # iminfo 0x2B558
## Checking Image at 0002b558 ...
Legacy image found
Image Name:
Image Type: ARM Linux Kernel Image (uncompressed)
Data Size: 94184 Bytes = 92 KiB
Load Address: 08000000
Entry Point: 08000000
Verifying Checksum ... OK
VersatilePB #
But it simply resets at the attempt to launch it
VersatilePB # bootm 0x2B558
## Booting kernel from Legacy Image at 0002b558 ...
Image Name:
Image Type: ARM Linux Kernel Image (uncompressed)
Data Size: 94184 Bytes = 92 KiB
Load Address: 08000000
Entry Point: 08000000
Loading Kernel Image ... OK
OK
Starting kernel ...
resetting ...
Quoted 2 lines · Andrey Borzenkov
Load Address: 08000000
Entry Point: 08000000This obviously can't work if this address is unavailable. You need to change linking address.
В Sun, 28 Jul 2013 20:01:29 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:
Quoted 6 lines · Andrey Borzenkov
Quoted 2 lines
Load Address: 08000000 Entry Point: 08000000This obviously can't work if this address is unavailable. You need to change linking address.
If you know which value it needs to have, you could just tell ...
Anyway the problem was that GRUB uboot code depends on U-Boot providing API, and as it turned out it is optional (and not default). After rebuilding U-Boot with CONFIG_API I now get
VersatilePB # bootm 0x2F7CC
## Current stack ends at 0x07facae8 * kernel: cmdline image address = 0x0002f7cc ## Booting kernel from Legacy Image at 0002f7cc ...
Image Name:
Image Type: ARM Linux Kernel Image (uncompressed)
Data Size: 94184 Bytes = 92 KiB
Load Address: 08000000
Entry Point: 08000000
kernel data at 0x0002f80c, len = 0x00016fe8 (94184)
## No init Ramdisk
ramdisk start = 0x00000000, ramdisk end = 0x00000000
Loading Kernel Image ... OK
OK
kernel loaded at 0x08000000, end = 0x08016fe8
using: ATAGS
## Transferring control to Linux (at address 08000000)...
Starting kernel ...
Welcome to GRUB!
error: variable `root' isn't set.
Entering rescue mode...
grub rescue>
В Sun, 28 Jul 2013 22:18:41 +0400
Andrey Borzenkov <[email protected]> пишет:
Quoted 33 lines
В Sun, 28 Jul 2013 20:01:29 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:Quoted 6 lines · Andrey Borzenkov
Quoted 2 lines
Load Address: 08000000 Entry Point: 08000000This obviously can't work if this address is unavailable. You need to change linking address.
If you know which value it needs to have, you could just tell ...
Anyway the problem was that GRUB uboot code depends on U-Boot providing API, and as it turned out it is optional (and not default). After rebuilding U-Boot with
CONFIG_APII now getVersatilePB # bootm
0x2F7CC
## Current stack ends at0x07facae8* kernel: cmdline image address =0x0002f7cc## Booting kernel from Legacy Image at0002f7cc...
Image Name:Image Type: ARM Linux Kernel Image (uncompressed) Data Size: 94184 Bytes = 92 KiB Load Address: 08000000 Entry Point: 08000000 kernel data at 0x0002f80c, len = 0x00016fe8 (94184)## No init Ramdisk
ramdisk start =0x00000000, ramdisk end =0x00000000Loading Kernel Image ... OK
OK
kernel loaded at0x08000000, end =0x08016fe8
using: ATAGS
## Transferring control to Linux (at address 08000000)...
Quoted 7 lines
Starting kernel ...
Welcome to GRUB!
error: variable `root' isn't set.
Entering rescue mode...
grub rescue>
Unfortunately attempt to do anything more useful ended up in
Starting kernel ...
Welcome to GRUB!
Unsupported ARM ID 0x41069265
Aborted. Press any key to exit.resetting ...
Which makes sense as we do not have any cache implementation for ARMv5 ... sigh, all other platforms listed by QEMU I tried would not even run U-Boot (at least, would not produce any output on serial console).
В Sat, 27 Jul 2013 06:21:56 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:
Quoted 11 lines · Andrey Borzenkov
Quoted 10 lines
В Thu, 25 Jul 2013 17:04:34 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:Quoted 5 lines
Quoted 1 line
ARM branch won't be maintained anymore. My main problem with arm currently is that testsuite works neither for arm-efi nor for arm-uboot and I didn't find a way to make qemu load file from iso from either u-boot or efi.
Quoted 6 lines
Quoted 3 lines
Could you show command line used to run grub under qemu? So far I was unsuccessful to find working combination. Do you need special u-boot build?
I have exactly the same problem. It's possible to support verstile/pb with armv5 but available u-boot tutorials for qemu ask you to concatenate payload with u-boot itself which is annoying to handle.
With current grub (that added support for transparent caches on armv5) I can finally boot grub over PXE (core.img or rescue image; need to add netboot support) using qemu versatilepb.
Now the problem is to do anything useful hard disk is needed and default u-boot for versatileqemu does not seem to offer any support (or may be it is qemu -M versatilepb). Were you successful in running arm port under qemu?
Quoted 21 lines · Andrey Borzenkov
В Sat, 27 Jul 2013 06:21:56 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:Quoted 11 lines · Andrey Borzenkov
Quoted 10 lines
В Thu, 25 Jul 2013 17:04:34 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:Quoted 5 lines
Quoted 1 line
ARM branch won't be maintained anymore. My main problem with arm currently is that testsuite works neither for arm-efi nor for arm-uboot and I didn't find a way to make qemu load file from iso from either u-boot or efi.
Quoted 6 lines
Quoted 3 lines
Could you show command line used to run grub under qemu? So far I was unsuccessful to find working combination. Do you need special u-boot build?
I have exactly the same problem. It's possible to support verstile/pb with armv5 but available u-boot tutorials for qemu ask you to concatenate payload with u-boot itself which is annoying to handle.
Quoted 9 lines
With current grub (that added support for transparent caches on armv5) I can finally boot grub over PXE (core.img or rescue image; need to add netboot support) using qemu versatilepb.
Now the problem is to do anything useful hard disk is needed and default u-boot for versatileqemu does not seem to offer any support (or may be it is qemu -M versatilepb). Were you successful in running arm port under qemu?
I didn't do netboot but concatenated with u-boot. It worked to similar extent as what you describe. Another promising board is vexpress but I had problems with MMC support in u-boot don't remember what it was exactly.
Quoted 5 lines
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Quoted 8 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 5 lines
Now the problem is to do anything useful hard disk is needed and default u-boot for versatileqemu does not seem to offer any support (or may be it is qemu -M versatilepb). Were you successful in running arm port under qemu?
I didn't do netboot but concatenated with u-boot. It worked to similar extent as what you describe. Another promising board is vexpress but I had problems with MMC support in u-boot don't remember what it was exactly.
I have been looking at this today together with Ian Campbell at the mini Debconf-UK. Commit c9cd02c cleans up the uboot syscall interface, but unfortunately it also corrupts the stack for va_arg passing. It is only evident when passing a sufficient number of arguments, which I think is only triggered by the disk accesses.
Patch attached, verified on Ian's qemu vexpress, which he may be posting some updates on later.
/
Leif
From 7d317fc56bb7c5e54b85caa9f749075f4e5bfaf2 Mon Sep 17 00:00:00 2001
From: Leif Lindholm <[email protected]>
Date: Sat, 16 Nov 2013 12:15:53 +0000
Subject: [PATCH] arm: fix u-boot port syscall interface va_arg handling
Commit c9cd02c broke the u-boot syscall API for va_args that spill over to the stack, causing the disk support to stop working. This patch resolves the problem, while keeping the new, cleaner transition_space handling.
--- ChangeLog | 5 +++++ grub-core/kern/arm/uboot/startup.S | 11 ----------- 2 files changed, 5 insertions(+), 11 deletions(-)
ChangeLog +5 -0
@@ -1,3 +1,8 @@ +2013-11-16 Leif Lindholm <[email protected]> + + * grub-core/kern/arm/uboot/startup.S: fix grub_uboot_syscall va_arg + handling + 2013-11-15 Vladimir Serbinenko <[email protected]> Replace libgcc version of ctz with our own.
grub-core/kern/arm/uboot/startup.S +0 -11
@@ -130,15 +130,8 @@ FUNCTION(codestart) FUNCTION(grub_uboot_syscall) str r8, transition_space str lr, transition_space + 4 - str r9, transition_space + 8 - str sp, transition_space + 12 - - sub sp, sp, #0x20 - lsr sp, sp, #3 - lsl sp, sp, #3 ldr r8, gd_backup - ldr r9, gd_backup + 4 mov lr, pc ldr pc, grub_uboot_syscall_ptr
@@ -146,8 +139,6 @@ FUNCTION(grub_uboot_syscall) ldr r8, transition_space ldr lr, transition_space + 4 - ldr r9, transition_space + 8 - ldr sp, transition_space + 12 bx lr
@@ -179,8 +170,6 @@ entry_state: @ backup for U-Boot context transition_space: .long 0 @ r8 .long 0 @ lr - .long 0 @ r9 - .long 0 @ sp VARIABLE(grub_uboot_syscall_ptr) .long 0 @
Signature · 1 line
1.8.4.rc3
PATCH 1/1 arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
grub-core/kern/arm/uboot/startup.S FUNCTION(codestart) · line 133
Quoted 1 line · Leif Lindholm
grub-core/kern/arm/uboot/startup.S +0
-1- str r9, transition_space + 8
You need to save r9. Otherwise GRUB will crash if compiled with clang.
В Sat, 16 Nov 2013 01:45:31 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:
Quoted 26 lines · Andrey Borzenkov
Quoted 25 lines
В Sat, 27 Jul 2013 06:21:56 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:Quoted 11 lines · Andrey Borzenkov
Quoted 10 lines
В Thu, 25 Jul 2013 17:04:34 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:Quoted 5 lines
Quoted 1 line
ARM branch won't be maintained anymore. My main problem with arm currently is that testsuite works neither for arm-efi nor for arm-uboot and I didn't find a way to make qemu load file from iso from either u-boot or efi.
Quoted 6 lines
Quoted 3 lines
Could you show command line used to run grub under qemu? So far I was unsuccessful to find working combination. Do you need special u-boot build?
I have exactly the same problem. It's possible to support verstile/pb with armv5 but available u-boot tutorials for qemu ask you to concatenate payload with u-boot itself which is annoying to handle.
With current grub (that added support for transparent caches on armv5) I can finally boot grub over PXE (core.img or rescue image; need to add netboot support) using qemu versatilepb.
Quoted 8 lines
Quoted 5 lines
Now the problem is to do anything useful hard disk is needed and default u-boot for versatileqemu does not seem to offer any support (or may be it is qemu -M versatilepb). Were you successful in running arm port under qemu?
I didn't do netboot but concatenated with u-boot. It worked to similar extent as what you describe. Another promising board is vexpress but I had problems with MMC support in u-boot don't remember what it was exactly.
Do you mean physical board? The only qemu machine for which I was able to build working u-boot was versatilepb.
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
In reply to Vladimir 'φ-coder/phcoder' SerbinenkoQuoted 3 lines · Vladimir 'φ-coder/phcoder' Serbinenko
grub-core/kern/arm/uboot/startup.S FUNCTION(codestart) · line 133
Quoted 1 line · Leif Lindholm
grub-core/kern/arm/uboot/startup.S +0
-1- str r9, transition_space + 8You need to save r9. Otherwise GRUB will crash if compiled with clang.
Hmm, but this is a bug in clang in that case.
From an ABI perspective, grub_uboot_syscall is a veneer:
the only visible side effect it is permitted to have is to corrupt r12.
We need to additionally switch r8 between grub and u-boot copy because u-boot uses it for its global data pointer, and lr because we need to corrupt it to make u-boot return to this veneer to switch the r8 back again.
This veneer should not touch anything else - so if we need to temporarily work around a toolchain ABI bug, can we do this via an ifdef and a config option?
/
Leif
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
Quoted 10 lines · Leif Lindholm
Quoted 3 lines · Vladimir 'φ-coder/phcoder' Serbinenko
grub-core/kern/arm/uboot/startup.S FUNCTION(codestart) · line 133
Quoted 1 line · Leif Lindholm
grub-core/kern/arm/uboot/startup.S +0
-1- str r9, transition_space + 8You need to save r9. Otherwise GRUB will crash if compiled with clang.
Hmm, but this is a bug in clang in that case.
From an ABI perspective,
grub_uboot_syscallis a veneer:
the only visible side effect it is permitted to have is to corrupt r12.
According to wikipedia:
"Subroutines must preserve the contents of r4 to r11 and the stack pointer." So changing r9 sounds to me like this is actually U-Boot bug and preserving it sounds like right way to handle it.
What's the harm in preserving r9? Is it used for something in calling?
Quoted 9 lines
We need to additionally switch r8 between grub and u-boot copy because u-boot uses it for its global data pointer, and lr because we need to corrupt it to make u-boot return to this veneer to switch the r8 back again.
This veneer should not touch anything else - so if we need to temporarily work around a toolchain ABI bug, can we do this via an ifdef and a config option?
If the bug is on clang side (I doubt it right now), there is no way to handle it without side effects and we can invent a test for it, we can reject buggy compilers (clang is secondary compiler)
Quoted 8 lines
/
Leif_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
В Sat, 16 Nov 2013 14:17:51 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> пишет:
Quoted 6 lines · Leif Lindholm
Quoted 5 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 3 lines · Leif Lindholm
grub-core/kern/arm/uboot/startup.S FUNCTION(codestart) · line 133
Quoted 1 line
grub-core/kern/arm/uboot/startup.S +0
-1- str r9, transition_space + 8You need to save r9. Otherwise GRUB will crash if compiled with clang.
Quoted 7 lines
Quoted 5 lines
Hmm, but this is a bug in clang in that case.
From an ABI perspective,
grub_uboot_syscallis a veneer:
the only visible side effect it is permitted to have is to corrupt r12.According to wikipedia:
"Subroutines must preserve the contents of r4 to r11 and the stack pointer."
According to ARM ABI documentation
"A subroutine must preserve the contents of the registers r4 - r8, r10, r11 and SP (and r9 in PCS variants that designate r9 as v6)."
So it sounds like whether r9 is preserved depends on platform.
Quoted 25 lines
So changing r9 sounds to me like this is actually U-Boot bug and preserving it sounds like right way to handle it.
What's the harm in preserving r9? Is it used for something in calling?Quoted 9 lines
We need to additionally switch r8 between grub and u-boot copy because u-boot uses it for its global data pointer, and lr because we need to corrupt it to make u-boot return to this veneer to switch the r8 back again.
This veneer should not touch anything else - so if we need to temporarily work around a toolchain ABI bug, can we do this via an ifdef and a config option?
If the bug is on clang side (I doubt it right now), there is no way to handle it without side effects and we can invent a test for it, we can reject buggy compilers (clang is secondary compiler)
Quoted 8 lines
/
Leif_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
In reply to Vladimir 'φ-coder/phcoder' SerbinenkoQuoted 7 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 3 lines
From an ABI perspective,
grub_uboot_syscallis a veneer:
the only visible side effect it is permitted to have is to corrupt r12.According to wikipedia:
"Subroutines must preserve the contents of r4 to r11 and the stack pointer." So changing r9 sounds to me like this is actually U-Boot bug and preserving it sounds like right way to handle it.
No. grub_uboot_syscall is not a subroutine - it is a veneer. We have a specific reason to need to preserve r8, over and above what the ABI says, because u-boot has hijacked it.
But now I went to look at the u-boot code, and suddenly I want to cry. Commit fe1378a961e508b31b1f29a2bb08ba1dac063155 changes the register reserved for global data from r8 to r9. Which means we need to preserve both since they didn't step the API version number.
*sigh*
Updated patch attached.
/
Leif
From d1926ea10b47442b4302e0d474cf18dbe65167de Mon Sep 17 00:00:00 2001
From: Leif Lindholm <[email protected]>
Date: Sat, 16 Nov 2013 12:15:53 +0000
Subject: [PATCH] arm: fix u-boot port syscall interface va_arg handling
Commit c9cd02c broke the u-boot syscall API for va_args that spill over to the stack, causing the disk support to stop working. This patch resolves the problem, while keeping the new, cleaner transition_space handling.
--- ChangeLog | 5 +++++ grub-core/kern/arm/uboot/startup.S | 11 ++--------- 2 files changed, 7 insertions(+), 9 deletions(-)
ChangeLog +5 -0
@@ -1,3 +1,8 @@ +2013-11-16 Leif Lindholm <[email protected]> + + * grub-core/kern/arm/uboot/startup.S: fix grub_uboot_syscall va_arg + handling + 2013-11-15 Vladimir Serbinenko <[email protected]> Replace libgcc version of ctz with our own.
grub-core/kern/arm/uboot/startup.S +2 -9
@@ -131,11 +131,6 @@ FUNCTION(grub_uboot_syscall) str r8, transition_space str lr, transition_space + 4 str r9, transition_space + 8 - str sp, transition_space + 12 - - sub sp, sp, #0x20 - lsr sp, sp, #3 - lsl sp, sp, #3 ldr r8, gd_backup ldr r9, gd_backup + 4
@@ -147,7 +142,6 @@ FUNCTION(grub_uboot_syscall) ldr r8, transition_space ldr lr, transition_space + 4 ldr r9, transition_space + 8 - ldr sp, transition_space + 12 bx lr
@@ -166,8 +160,8 @@ entry_state_end: .long 0 @ r6 .long 0 @ r7 gd_backup: - .long 0 @ r8 - U-Boot global data pointer - .long 0 @ r9 + .long 0 @ r8 - U-Boot global data pointer up to 2013-09-21 + .long 0 @ r9 - U-Boot global data pointer 2013-09-21 onwards .long 0 @ r10 .long 0 @ r11 VARIABLE(grub_uboot_search_hint)@ U-Boot stack pointer -
@@ -180,7 +174,6 @@ transition_space: .long 0 @ r8 .long 0 @ lr .long 0 @ r9 - .long 0 @ sp VARIABLE(grub_uboot_syscall_ptr) .long 0 @
Signature · 1 line
1.8.4.rc3
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
Quoted 13 lines · Leif Lindholm
Quoted 7 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 3 lines
From an ABI perspective,
grub_uboot_syscallis a veneer:
the only visible side effect it is permitted to have is to corrupt r12.According to wikipedia:
"Subroutines must preserve the contents of r4 to r11 and the stack pointer." So changing r9 sounds to me like this is actually U-Boot bug and preserving it sounds like right way to handle it.No.
grub_uboot_syscallis not a subroutine - it is a veneer. We have a specific reason to need to preserve r8, over and above what the ABI says, because u-boot has hijacked it.
Quoted 8 lines
But now I went to look at the u-boot code, and suddenly I want to cry. Commit
fe1378a961e508b31b1f29a2bb08ba1dac063155changes the register reserved for global data from r8 to r9. Which means we need to preserve both since they didn't step the API version number.*sigh*
Updated patch attached.
Go ahead.
Quoted 11 lines
/
Leif_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
Quoted 3 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 1 line
Updated patch attached.
Go ahead.
Argh...
I pushed the above, and almost immediately I realised that in this situation we also need to store u-boot's version of r9 on return:
grub-core/kern/arm/uboot/startup.S +1 -0
@@ -138,6 +138,7 @@ FUNCTION(grub_uboot_syscall) mov lr, pc ldr pc, grub_uboot_syscall_ptr str r8, gd_backup + str r9, gd_backup + 4 ldr r8, transition_space ldr lr, transition_space + 4
Can I push this fix too?
/
Leif
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
Quoted 18 lines · Leif Lindholm
Argh...
I pushed the above, and almost immediately I realised that in this situation we also need to store u-boot's version of r9 on return:
grub-core/kern/arm/uboot/startup.S +1
-0@@ -138,6 +138,7 @@ FUNCTION(grub_uboot_syscall) mov lr, pc ldr pc, grub_uboot_syscall_ptr str r8, gd_backup + str r9, gd_backup + 4 ldr r8, transition_space ldr lr, transition_space + 4
Quoted 2 lines
Can I push this fix too?
But why does global pointer change at all?
What about exit function?
Quoted 2 lines
/
Leif
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
In reply to Leif LindholmQuoted 16 lines · Leif Lindholm
I pushed the above, and almost immediately I realised that in this situation we also need to store u-boot's version of r9 on return:
grub-core/kern/arm/uboot/startup.S +1
-0@@ -138,6 +138,7 @@ FUNCTION(grub_uboot_syscall) mov lr, pc ldr pc, grub_uboot_syscall_ptr str r8, gd_backup + str r9, gd_backup + 4 ldr r8, transition_space ldr lr, transition_space + 4
Quoted 1 line
Can I push this fix too?
Umm, at second thought - the gd pointer(s) wouldn't be changing. So the correct fix would rather be to
- str r8, gd_backup
?
/
Leif
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
Quoted 17 lines · Leif Lindholm
Quoted 16 lines · Leif Lindholm
I pushed the above, and almost immediately I realised that in this situation we also need to store u-boot's version of r9 on return:
grub-core/kern/arm/uboot/startup.S +1
-0@@ -138,6 +138,7 @@ FUNCTION(grub_uboot_syscall) mov lr, pc ldr pc, grub_uboot_syscall_ptr str r8, gd_backup + str r9, gd_backup + 4 ldr r8, transition_space ldr lr, transition_space + 4
Quoted 7 lines
Quoted 1 line
Can I push this fix too?
Umm, at second thought - the gd pointer(s) wouldn't be changing. So the correct fix would rather be to
- str r8,gd_backup
?
Was this tested?
Quoted 8 lines
/
Leif_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
Quoted 9 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 9 lines
Quoted 9 lines
grub-core/kern/arm/uboot/startup.S +1
-0@@ -138,6 +138,7 @@ FUNCTION(grub_uboot_syscall) mov lr, pc ldr pc, grub_uboot_syscall_ptr str r8, gd_backup + str r9, gd_backup + 4 ldr r8, transition_space ldr lr, transition_space + 4
Quoted 8 lines
Quoted 7 lines
Quoted 1 line
Can I push this fix too?
Umm, at second thought - the gd pointer(s) wouldn't be changing. So the correct fix would rather be to
- str r8,gd_backup
?Was this tested?
Tested on Ian's qemu, with an "mmc" block device..
/
Leif
Re: [PATCH] arm: fix u-boot port syscall interface va_arg handling (was Re: [PATCH] [ARM] Enable boot module for arm)
Quoted 17 lines · Leif Lindholm
Quoted 16 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 16 lines
Quoted 10 lines
grub-core/kern/arm/uboot/startup.S +1
-0@@ -138,6 +138,7 @@ FUNCTION(grub_uboot_syscall) mov lr, pc ldr pc, grub_uboot_syscall_ptr str r8, gd_backup + str r9, gd_backup + 4 ldr r8, transition_space ldr lr, transition_space + 4Can I push this fix too?
Umm, at second thought - the gd pointer(s) wouldn't be changing. So the correct fix would rather be to
- str r8,gd_backup
?
Quoted 4 lines
Quoted 1 line
Was this tested?
Tested on Ian's qemu, with an "mmc" block device..
Go ahead.
Quoted 8 lines
/
Leif_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel