diff mbox

[0/3] kallsyms: remove special handling for CONFIG_ARM

Message ID CAKv+Gu8ZTHAqviveLMizRKPfeXwb6kcXi4cVQNYVYuPcVMgMeQ@mail.gmail.com
State New
Headers show

Commit Message

Ard Biesheuvel Feb. 3, 2016, 1:41 p.m. UTC 
On 3 February 2016 at 14:33, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On 2 Feb 2016, Chris Brandt wrote:

>> I then applied the 3 patches and tried again and this time it booted

>> up....almost.

>> It looks like it makes it all the way up to when it is going to mount

>> my rootfs, but then dies.

>

>

> I did some debugging, and here's why I'm crashing.

>

>

> If you look at my System.map file, you'll see that I've got 2 "__stubs_start" symbols.

>

> One at 0x bf353edc, the other at 0x bf353ee0.

>

> bf353edc T __stubs_start


This is a global symbol

> bf353edc T _etext

> bf353ee0 t __stubs_start


This is a local symbol

> bf353ee4 t vector_rst

> bf353f00 t vector_irq

> bf353f80 t vector_dabt

> bf354000 t vector_pabt

> bf354080 t vector_und

> bf354100 t vector_addrexcptn

> bf354120 T vector_fiq

> bf3541a0 T __stubs_end

> bf3541a0 T __vectors_start

> bf3541c0 t __mmap_switched

> bf3541c0 T __vectors_end

>

>

> So, when you get to early_trap_init(), it grabs the first __stubs_start for the memcpy:

>

>         memcpy((void *)vectors + 0x1000, __stubs_start, __stubs_end - __stubs_start);

>


Indeed. It grabs the first one, which is the one defined by the linker script.

>

> And when a printed out the destination, I get:

>

>    Stubs at c09ff000 :00000000 BF006F20 EF9F0000 EA000064 etc...

>

> Notice the first entry is 0....that's not right...and we know it's using that first __stubs_start symbol.

>


The first quantity after __stubs_start should be the address of
vector_swi. Can you check if the second value coincides with that?

>

> So, I hacked the code and put an offset of 4 like this:

>

>         memcpy((void *)vectors + 0x1000, __stubs_start+4, __stubs_end - (__stubs_start+4));

>

>

> And sure enough it booted fine.

>

>

> So, why are there 2 __stubs_start symbols???

> I haven't figured that one out yet.

>


One defined by entry-armv.S and one defined by the linker script

> And, I'm assuming you guys don't have 2 __stubs_start symbols either, correct?

>


Yes, we do.

This is caused by the fact that __stubs_start is not aligned correctly on XIP.

Could you try this, please?


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Comments

Ard Biesheuvel Feb. 3, 2016, 2:17 p.m. UTC | #1 
On 3 February 2016 at 15:15, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On 2 Feb 2016, Chris Brandt wrote

>> > And when a printed out the destination, I get:

>> >

>> >    Stubs at c09ff000 :00000000 BF006F20 EF9F0000 EA000064 etc...

>> >

>> > Notice the first entry is 0....that's not right...and we know it's using that

>> > first __stubs_start symbol.

>> >

>>

>> The first quantity after __stubs_start should be the address of vector_swi. Can

>>  you check if the second value coincides with that?

>

> Yes, it does.

>

> bf006f20 T vector_swi

>

> Which is why it booted once I modified the start address.

>

>

>> This is caused by the fact that __stubs_start is not aligned correctly on XIP.

>>

>> Could you try this, please?

>>

>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S index 7160658fd5d4..a5b8e7b80d17 100644

>> --- a/arch/arm/kernel/vmlinux.lds.S

>> +++ b/arch/arm/kernel/vmlinux.lds.S

>> @@ -164,8 +164,8 @@ SECTIONS

>>          * The vectors and stubs are relocatable code, and the

>>          * only thing that matters is their relative offsets

>>          */

>> -       __stubs_start = .;

>>         .stubs : {

>> +               __stubs_start = .;

>>                 *(.stubs)

>>         }

>>         __stubs_end = .;

>

>

> Yup, that fixed it!

>

> bf353edc T _etext

> bf353ee0 t __stubs_start

> bf353ee0 T __stubs_start

> bf353ee4 t vector_rst

> bf353f00 t vector_irq

>

> and my new print out:

>

>      Stubs at c09ff000 :BF006F20 EF9F0000 EA000064 E320F000 etc...

>

>

> Now that it is working on my XIP system, I will say that I'm also happy to see that extra junk in kallsyms.c and link-vmlinux.sh be removed. Those things also caused me some issues when I first started trying to get the XIP_KERNEL stuff back working.

>


Glad to hear that it works now. Thanks a lot for testing.

-- 
Ard.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel Feb. 3, 2016, 8:02 p.m. UTC | #2 
On 3 February 2016 at 21:01, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:

>> > bf353edc T __stubs_start

>>

>> This is a global symbol

>>

>> > bf353edc T _etext

>> > bf353ee0 t __stubs_start

>>

>> This is a local symbol

>

> Stop this madness.  This gets you an immediate NAK on the patch set.

>


What madness? This does not change at all with my patches, these
duplicates already exist in the current code.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Nicolas Pitre Feb. 3, 2016, 8:23 p.m. UTC | #3 
On Wed, 3 Feb 2016, Ard Biesheuvel wrote:

> On 3 February 2016 at 21:01, Russell King - ARM Linux

> <linux@arm.linux.org.uk> wrote:

> > On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:

> >> > bf353edc T __stubs_start

> >>

> >> This is a global symbol

> >>

> >> > bf353edc T _etext

> >> > bf353ee0 t __stubs_start

> >>

> >> This is a local symbol

> >

> > Stop this madness.  This gets you an immediate NAK on the patch set.

> >

> 

> What madness? This does not change at all with my patches, these

> duplicates already exist in the current code.


That doesn't make it any better, and was cause for issue as Chris 
demonstrated.  Can we get rid of that duplication please?


Nicolas

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel Feb. 3, 2016, 8:30 p.m. UTC | #4 
On 3 February 2016 at 21:23, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Wed, 3 Feb 2016, Ard Biesheuvel wrote:

>

>> On 3 February 2016 at 21:01, Russell King - ARM Linux

>> <linux@arm.linux.org.uk> wrote:

>> > On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:

>> >> > bf353edc T __stubs_start

>> >>

>> >> This is a global symbol

>> >>

>> >> > bf353edc T _etext

>> >> > bf353ee0 t __stubs_start

>> >>

>> >> This is a local symbol

>> >

>> > Stop this madness.  This gets you an immediate NAK on the patch set.

>> >

>>

>> What madness? This does not change at all with my patches, these

>> duplicates already exist in the current code.

>

> That doesn't make it any better, and was cause for issue as Chris

> demonstrated.  Can we get rid of that duplication please?

>


4/4 in my v2 removes the redundant symbols.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel Feb. 3, 2016, 8:41 p.m. UTC | #5 
On 3 February 2016 at 21:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 3 February 2016 at 21:23, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

>> On Wed, 3 Feb 2016, Ard Biesheuvel wrote:

>>

>>> On 3 February 2016 at 21:01, Russell King - ARM Linux

>>> <linux@arm.linux.org.uk> wrote:

>>> > On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:

>>> >> > bf353edc T __stubs_start

>>> >>

>>> >> This is a global symbol

>>> >>

>>> >> > bf353edc T _etext

>>> >> > bf353ee0 t __stubs_start

>>> >>

>>> >> This is a local symbol

>>> >

>>> > Stop this madness.  This gets you an immediate NAK on the patch set.

>>> >

>>>

>>> What madness? This does not change at all with my patches, these

>>> duplicates already exist in the current code.

>>

>> That doesn't make it any better, and was cause for issue as Chris

>> demonstrated.  Can we get rid of that duplication please?

>>

>

> 4/4 in my v2 removes the redundant symbols.


... actually, that only removes the redundant __stub_start, and the
redundant __vectors_start is already removed in patch #1. Patch #4
(v2) may be shot down for other reasons, in that case I can simply
move the removal of __stubs_start to patch #1 instead.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox

Patch

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 7160658fd5d4..a5b8e7b80d17 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -164,8 +164,8 @@  SECTIONS
         * The vectors and stubs are relocatable code, and the
         * only thing that matters is their relative offsets
         */
-       __stubs_start = .;
        .stubs : {
+               __stubs_start = .;
                *(.stubs)
        }
        __stubs_end = .;