On 15 June 2015 at 13:08, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxxx> wrote:
> On Mon, Jun 15, 2015 at 09:04:20AM +0200, Ard Biesheuvel wrote:
>> On 15 June 2015 at 01:13, Ben Hutchings <ben@xxxxxxxxxxxxxxx> wrote:
>> > On Tue, 2015-06-02 at 19:42 -0700, Kevin Hilman wrote:
>> >> From: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> >>
>> >> This code calls cpu_resume() using a straight branch (b), so
>> >> now that we have moved cpu_resume() back to .text, this should
>> >> be moved there as well. Any direct references to symbols that will
>> >> remain in the .data section are replaced with explicit PC-relative
>> >> references.
>> >
>> > I don't get it. cpu_resume() is still in the .data section in 4.0.
>> > This appears to depend on:
>> >
>> > commit d0776aff9a38b1390cc06ffc2c4dcf6ece7c05b9
>> > Author: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> > Date: Wed Mar 25 07:39:21 2015 +0100
>> >
>> > ARM: 8324/1: move cpu_resume() to .text section
>> >
>>
>> You are right. So this patch results in the Exynos resume function to
>> call cpu_resume() in .data from the .text section. This turns out to
>> work fine for normal configs (exynos_defconfig, multi_v7_defconfig)
>> built for ARM since the distance is < 16 MB, and -apparently- fixes an
>> issue Kevin spotted with the Thumb build on top of that. Whether
>> cpu_resume() may now be out of Thumb range (8 MB) in some configs is
>> irrelevant since the Thumb build was broken in the first place.
>
> Err, commit 12833bacf5d904c2dac0c3f52b2ebde5f2c5a2bc should not be
> taken into older kernels at all, precisely because 8324/1 is not
> backported either.
>
> The whole point of moving it (if you read the commit text) is because
> we've moved cpu_resume to .text, which then allows the mach-* asm
> which calls cpu_resume to also move. Without the first, the second
> doesn't make sense.
>
It appears that the introduction of open coded PC-relative references
works around an adr range issue on Thumb2. So it's kind of a side
effect of this patch, and not the original purpose. I was aware of
this when it was proposed for -stable, and I didn't see any harm. I
guess I should have said something ...
But looking at the original error:
../arch/arm/mach-exynos/sleep.S:72: Error: invalid immediate for
address calculation (value = 0x00000004)
../arch/arm/mach-exynos/sleep.S:74: Error: invalid immediate for
address calculation (value = 0x00000004)
there is something odd going on, since a value of 0x4 should obviously
be in range.
I can reproduce the error with this minimal .S:
"""
adr r0, cp15_save_power
.align
.globl cp15_save_power
cp15_save_power:
.long 0 @ cp15 power control
"""
$ arm-linux-gnueabihf-as -o /tmp/sleep.o /tmp/sleep.S -mthumb
/tmp/sleep.S: Assembler messages:
/tmp/sleep.S:1: Error: invalid immediate for address calculation
(value = 0x00000004)
The error goes away when I drop the -mthumb or when I replace the code with
"""
adr r0, 0f
.align
.globl cp15_save_power
cp15_save_power:
0: .long 0 @ cp15 power control
"""
In summary, this is an unrelated binutils issue that happens to go
away with $subject patch. (I am using binutils v2.24 btw)
> I think the question is - what's caused stable-4.0 to start spitting
> these errors? Presumably, 4.0 didn't, and stable-4.0 has regressed?
> Maybe, rather than trying to fix this new regression, the original
> cause should be reverted?
>
Not sure whether it's a regression. I think this code does not usually
get built in Thumb2 mode in the first place.
--
Ard.
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html