[PATCH 0/2] add interworking support to Thumb2 kernel

[Ard Biesheuvel]

On 20 August 2016 at 18:55, Russell King - ARM Linux
<linux at armlinux.org.uk> wrote:
> On Sat, Aug 20, 2016 at 06:36:44PM +0200, Ard Biesheuvel wrote:
>> No, that is not the point. The point is to lift the requirement that a
>> Thumb2 kernel must implement all its exported or modularised functions
>> in Thumb2, not only in the core kernel but also in modules themselves.
>> For example, the crypto modules I contributed contain .S core
>> implementations, and building those in Thumb2 requires tweaking the
>> implementations so that the assembler accepts it as Thumb2 code. There
>> are examples in the tree (sha1-v4 and aes-v4) where we had to add ugly
>> workarounds so that the code would build (and it was still broken for
>> a while). So in general, it would be preferable to have a single ARM
>> implementation for both modes. Also, it will become less likely for
>> the multi_v7_defconfig to break inadvertently due to the addition of
>> low level platform code with asm components that turn out not to build
>> in Thumb2.
>
> Right, so we're talking _purely_ about assembly level stuff here.
>

Primarily, yes.

> However, what I would point out is that some people will want to build
> their kernel as definitely ARM-only or definitely Thumb2-only.  While
> A-class accepts either, other class CPUs do not.  If we allow
> interworking, then we start breaking that.
>
> We do already support CPUs that only enter the kernel in Thumb2 mode or
> only enter in ARM mode, because the CPU has no support for the other
> instruction set.
>
>> So with these patches, it is no longer necessary for core helper
>> routines coded in assembler (or built in ARM mode for other reasons)
>> to go out of their way so that the assembler can turn them in both ARM
>> and Thumb2 code.
>
> I'm not sure what "core helper routines" you're referring to - all C
> functions will be built according to whether we want ARM mode or Thumb2
> mode.  We aren't going to end up with a mixture for C functions within
> the kernel.
>

Yes, but taking the NEON code in arch/arm/crypto as an example, this
can only execute on A cores anyway. When it is built into the kernel,
we can happily use the ARM version on a Thumb2 kernel, but as a module
it suddenly has to be adapted so that it is emitted as Thumb2 (while
upstream OpenSSL builds it in ARM mode)

>> I think there may be a performance benefit in some cases as well,
>> although this is anecdotal hearsay. The primary benefit of Thumb2 is
>> code size, and multi_v7_defconfig is a couple of MBs smaller when
>> built in Thumb2. In any case, lifting the restriction only affects
>> configurations that set CONFIG_THUMB2_KERNEL globally, and THUMB2
>> modules can still only be loaded into a THUMB2 kernel (due to the
>> THUMB2 tag in the version string).
>>
>> Oh, and the first patch is arguably a bugfix. An exported ARM function
>> will not be rejected atm, but called in Thumb2 mode.
>
> Sorry, I don't see it as a bug fix at all - I see it as a regression
> causing change for those classes of CPU which only accept one or
> other of the ISAs.
>

At the moment, the module loader allows R_ARM_THM_CALL relocations
against imported ARM symbols, under the assumption that an untyped
symbol is a Thumb2 symbol that simply has its Thumb bit cleared due to
the fact that it lacks a function annotation. However, if the
relocation is resolved across a module boundary, this annotation is
missing as well, and so imported ARM symbols are mistaken for Thumb2
symbols, causing a crash when called. Patch #1 fixes just that.

So do you (or anyone else) have any comments on the performance delta
between Thumb2 and ARM? I can imagine fixed size instructions make for
more efficient pipelines, but as I said, this is only a hunch.