Message-ID: <e256ae12-bba0-fc39-6d14-89cb89d31a25@arm.com>
Date:   Wed, 24 Apr 2019 16:08:15 +0000
From:   Raphael Gault <Raphael.Gault@....com>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        Catalin Marinas <Catalin.Marinas@....com>,
        Will Deacon <Will.Deacon@....com>,
        Julien Thierry <Julien.Thierry@....com>
Subject: Re: [PATCH 0/6] objtool: Add support for Arm64

Hi Josh,

On 4/23/19 10:09 PM, Josh Poimboeuf wrote:
> On Tue, Apr 09, 2019 at 02:52:37PM +0100, Raphael Gault wrote:
>> Hi,
>>
>> As of now, objtool only supports the x86_64 architecture but the
>> groundwork has already been done in order to add support for other
>> architecture without too much effort.
>>
>> This series of patches adds support for the arm64 architecture
>> based on the Armv8.5 Architecture Reference Manual.
>>
>> * Patch 1 adapts the existing code to be able to add support for other
>>    architecture.
>> * Patch 2 provide implementation of the required function for the arm64
>>    architecture.
>> * Patch 3 adapts the checking of the stack state for the arm64
>>    architecture.
>> * Patch 4 & 5 fix some warning objtool raised in some particular
>>    functions of ~/arch/arm64/kernel/sleep.S. Patch 4 add a macro to
>>    signal that some function should be ignored by objtool.
>> * Patch 6 enables stack validation for arm64.
>>
>> Theses patches should provide support for the main cases and behaviour.
>> However a few corner cases are not yet handled by objtool:
>>
>> * In the `~/arch/arm64/crypto/` directory, I noticed that some plain
>>    data are sometimes stored in the `.text` section causing objtool to mistake
>>    this for instructions and trying (and failing) to interprete them.  If someone
>>    could explain to me why we store data directly in the .text section I would
>>    appreciate it.
>
> I haven't looked, but it should probably be moved to .rodata.  We had
> cases like that for x86.
>
>> * In the support for arm32 architecture such as in `~/arch/arm64/kernel/kuser32.S`
>>    some A32 instructions are used but such instructions are not understood by
>>    objtool causing a warning.
>>
>> I also have a few unclear points I would like to bring to your
>> attention:
>>
>> * For x86_64, when looking for a symbol relocation with explicit
>>    addend, objtool systematically adds a +4 offset to the addend.
>>    I don't understand why even if I have a feeling it is related
>>    to the type of relacation.
>
> This is because of how relative call/jump addresses are implemented on
> x86.  It calculates the call/jump destination by adding the encoded offset
> to the *ending* address of the instruction, rather than to the address
> of the encoded offset itself.
>
> For example:
>
>    119ca0:       e8 00 00 00 00          callq  119ca5 <__ia32_sys_sched_rr_get_interval+0x5>
>                          119ca1: R_X86_64_PC32   __fentry__-0x4
>
> This instruction is a call to the __fentry__ function.  The rela addend
> is the address of the destination function (__fentry__) minus 4.  After
> applying the relocation, it resolves to:
>
> ffffffff81002010:       e8 eb f7 9f 00          callq  ffffffff81a01800 <__fentry__>
>
> The destination address is "0x9ff7eb", which is indeed __fentry__ - 4.
>
> x86 expects it to be that way, because the x86 CPU adds the offset to
> the *end* of the instruction: ffffffff81002015 (last digit is 5, not 0).
>
> 0xffffffff81002015 + 0x9ff7eb = 0xfffffff81a01800, which is indeed the
> address of __fentry__.
>
> And there's always a 4-byte gap between the relocation target and the
> end of the instruction, so the rela addend always has the -4.  So when
> reading the relocation we have to add the 4 bytes back to get the actual
> destination address.
>

Thank you for the explanation!

>> * I currently don't have a clear understanding about how switch-tables
>>    are generated on arm64 and how to retrieve them (based on relocations).
>
> This is indeed a bit tricky on x86.
>
>> Please provide me with any feedback and comments as well on the content
>> than the style of these patches.
>
> Overall it looks like a great start.  I added some per-patch comments.
>

Thank you, I will address them.

> Has the cross-compile path been tested?  Specifically, compiling for a
> arm64 target on an x86 host?  In other words, objtool would be an x86
> binary which reads arm64 objects.  I imagine that will be a semi-common
> use case.  Objtool already supports cross-compiling for an x86-64 target
> on an x86-32 host (and also a powerpc host IIRC), so it should be
> do-able in theory, and it might "just work".
>

It has indeed been tested to build for arm64 target on a x86 host and it
works fine.

> For the next version, please base it on the -tip tree, as that's where
> all the latest objtool changes are.  Peter refactored some code which
> has some minor conflicts with yours.
>

Thanks for letting me know about this. Just a quick precision, when you
talk about the -tip tree, are you talking about this tree ?
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/

Thanks,

--
Raphael Gault
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Powered by blists - more mailing lists