Messages in this thread | | | Subject | Re: [PATCH] MIPS: fix boot with DT passed via UHI | From | David Daney <> | Date | Wed, 7 Jun 2017 15:47:02 -0700 |
| |
On 06/07/2017 06:16 AM, Daniel Schwierzeck wrote: > > > Am 06.06.2017 um 21:16 schrieb Andrea Merello: >> commit 15f37e158892 ("MIPS: store the appended dtb address in a variable") >> seems to have introduced code that relies on delay slots after branch, >> however it seems that, since no directive ".set noreorder" is present, the >> AS already fills delay slots with NOPs. >> >> This caused failure in assigning proper DT blob address to fw_passed_dtb >> variable, causing failure when booting passing DT via UHI; this has been >> seen on a Lantiq VR9 SoC (Fritzbox 3370) and u-boot as bootloader. >> >> [ 0.000000] Linux version 4.12.0-fritz+ (andrea@horizon) (gcc version 4.9.0 (GCC) ) #29 SMP Tue Jun 6 20:49:59 CEST 2017 >> [ 0.000000] SoC: xRX200 rev 1.2 >> [ 0.000000] bootconsole [early0] enabled >> [ 0.000000] CPU0 revision is: 00019556 (MIPS 34Kc) >> [ 0.000000] Determined physical RAM map: >> [ 0.000000] memory: 00696000 @ 00002000 (usable) >> [ 0.000000] memory: 00038000 @ 00698000 (usable after init) >> [ 0.000000] Wasting 64 bytes for tracking 2 unused pages >> [ 0.000000] Kernel panic - not syncing: No memory area to place a bootmap bitmap >> [ 0.000000] Rebooting in 1 seconds.. >> [ 0.000000] Reboot failed -- System halted >> >> This patch moves the instruction meant to be placed in the delay slot >> before the preceding BEQ instruction, while the delay slot will be >> filled with a NOP by the AS. >> >> After this patch the kernel fetches the DR correctly >> >> [ 0.000000] Linux version 4.12.0-fritz+ (andrea@horizon) (gcc version 4.9.0 (GCC) ) #30 SMP >> Tue Jun 6 20:52:40 CEST 2017 >> [ 0.000000] SoC: xRX200 rev 1.2 >> [ 0.000000] bootconsole [early0] enabled >> [ 0.000000] CPU0 revision is: 00019556 (MIPS 34Kc) >> [ 0.000000] MIPS: machine is FRITZ3370 - Fritz!Box WLAN 3370 >> [ 0.000000] Determined physical RAM map: >> [ 0.000000] memory: 08000000 @ 00000000 (usable) >> [ 0.000000] Detected 1 available secondary CPU(s) >> [ 0.000000] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes. >> [ 0.000000] Primary data cache 32kB, 4-way, VIPT, cache aliases, linesize 32 bytes >> [ 0.000000] Zone ranges: >> [ 0.000000] Normal [mem 0x0000000000000000-0x0000000007ffffff] >> [ 0.000000] Movable zone start for each node >> [ 0.000000] Early memory node ranges >> [ 0.000000] node 0: [mem 0x0000000000000000-0x0000000007ffffff] >> [ 0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x0000000007ffffff] >> [ 0.000000] percpu: Embedded 15 pages/cpu @8110c000 s30176 r8192 d23072 u61440 >> [ 0.000000] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 32512 >> [ 0.000000] Kernel command line: rootwait root=/dev/sda1 console=ttyLTQ0 >> ... >> >> Cc: linux-kernel@vger.kernel.org >> Cc: Jonas Gorski <jogo@openwrt.org> >> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com> >> Signed-off-by: Andrea Merello <andrea.merello@gmail.com> >> >> diff --git a/arch/mips/kernel/head.S b/arch/mips/kernel/head.S >> index cf05220..d1bb506 100644 >> --- a/arch/mips/kernel/head.S >> +++ b/arch/mips/kernel/head.S >> @@ -106,8 +106,8 @@ NESTED(kernel_entry, 16, sp) # kernel entry point >> beq t0, t1, dtb_found >> #endif >> li t1, -2 >> - beq a0, t1, dtb_found >> move t2, a1 >> + beq a0, t1, dtb_found >> >> li t2, 0 >> dtb_found: >> > > The fix looks correct. Without ".set noreorder" one should not
s/should/must/
> manually > put instructions in the delay slot. This should be left to the AS as an > option for optimization.
By definition, it is what the assembler does. When ".set noreorder" is not in effect, the source code *must* be written as if branch delay slots do not exist. There is no option here.
> > Acked-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
Acked-by: David Daney <david.daney@cavium.com>
>
| |