956133 - Can we use CONFIG_THUMB2_KERNEL on b2g?

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

This is the geeksphone keon kernel:
   text    data     bss     dec     hex filename
6560212  288672  912504 7761388  766dec out/target/product/keon/obj/KERNEL_OBJ/vmlinux

And here it is with CONFIG_THUMB2_KERNEL set (and fixed so that it builds):
   text    data     bss     dec     hex filename
5030000  288576  912504 6231080  5f1428 out/target/product/keon/obj/KERNEL_OBJ/vmlinux

If other devices' kernels are of similar size, then we should be able to save 1.5 MB with no loss of functionality, and hopefully little/no loss of performance.

Problem: the patch I have lets the kernel build, but it crashes early in boot, and since I don't have a serial console or a hardware debugger I can't easily investigate.  I suspect there's assembly somewhere using an instruction that's UNPREDICTABLE in Thumb2 but not rejected by the assembler.  Also, the problem may be in drivers that are device-specific (meaning that my keon might have more/fewer/different issues as compared to tarako, for example).

So this would need coordination with hardware manufacturers, both to determine how much effort would be needed, and how feasible it might be to ship this.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug956133-thumb2-experiment.diff

This is my experiment with the keon kernel.

I also had to copy the assembler binaries from the arm-linux-androideabi-4.4.x toolchain into the arm-eabi-4.4.3 toolchain — this needs a newer assembler (binutils 2.20 vs. 2.19, I think), but the linker in arm-linux-androideabi-4.4.x is an early version of gold that crashes with an assertion failure.

Specifically:
> cd prebuilt/linux/toolchain
> cp arm-linux-androideabi-4.4.x/bin/arm-linux-androideabi-as arm-eabi-4.4.3/bin/arm-eabi-as
> cp arm-linux-androideabi-4.4.x/arm-linux-androideabi/bin/as arm-eabi-4.4.3/arm-eabi/bin/as

Comment 2

[Andreas Gal :gal]

Nice work Jed. I bet our chipset friends can make this work on their devices.

Comment 3

[Andreas Gal :gal]

Should we try this for gecko as well?

Comment 4

[James Zhang (Spreadtrum)]

We'll try on kernel side.

Comment 5

[James Zhang (Spreadtrum)]

I've tested this patch but no effect, the boot image size is not changed.

Comment 6

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Andreas Gal :gal from comment #3)
> Should we try this for gecko as well?

Gecko's C++ code is already compiled to Thumb2.  JITcode is still ARM; I recall seeing that someone was working on Thumb2 support at some point but I don't know what state it's in or how much it would help.

Comment 7

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to James Zhang from comment #5)
> I've tested this patch but no effect, the boot image size is not changed.

You'll need to add CONFIG_THUMB2_KERNEL=y to the kernel config file for your device; my patch changes arch/arm/configs/C8666_defconfig which is the config for the device I was using.  Sorry for the confusion.

Comment 8

[Andreas Gal :gal]

JIT code is pretty rare. Not a high priority right now. Thanks!

Comment 9

[James Zhang (Spreadtrum)]

(In reply to Jed Davis [:jld] from comment #7)
> (In reply to James Zhang from comment #5)
> > I've tested this patch but no effect, the boot image size is not changed.
> 
> You'll need to add CONFIG_THUMB2_KERNEL=y to the kernel config file for your
> device; my patch changes arch/arm/configs/C8666_defconfig which is the
> config for the device I was using.  Sorry for the confusion.

Yes, I have modified fugu's defconfig and makefile, no effect. The boot image size is still 7.4M.

Comment 10

[James Zhang (Spreadtrum)]

WONTFIX on my side, thanks.

Comment 11

[Michael Wu [:mwu]]

(In reply to James Zhang from comment #9)
> (In reply to Jed Davis [:jld] from comment #7)
> > (In reply to James Zhang from comment #5)
> > > I've tested this patch but no effect, the boot image size is not changed.
> > 
> > You'll need to add CONFIG_THUMB2_KERNEL=y to the kernel config file for your
> > device; my patch changes arch/arm/configs/C8666_defconfig which is the
> > config for the device I was using.  Sorry for the confusion.
> 
> Yes, I have modified fugu's defconfig and makefile, no effect. The boot
> image size is still 7.4M.

CONFIG_THUMB2_KERNEL=y is insufficient to turn it on. I did:

CONFIG_EXPERIMENTAL=y
CONFIG_THUMB2_KERNEL=y
CONFIG_THUMB2_AVOID_R_ARM_THM_JUMP11=y
CONFIG_ARM_ASM_UNIFIED=y

And the build broke on some assembly that doesn't work with thumb. The kernel version here is pretty old though, so we should be able to find some patches to fix that assembly.