Messages in this thread | | | Date | Fri, 15 Sep 2023 12:10:23 +0200 | Subject | Re: [PATCH] LoongArch: Fix lockdep static memory detection | From | Helge Deller <> |
| |
On 9/15/23 11:23, Huacai Chen wrote: > On Fri, Sep 15, 2023 at 4:16 PM Helge Deller <deller@gmx.de> wrote: >> >> On 9/15/23 05:22, Huacai Chen wrote: >>> Hi Helge, >>> >>> On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <deller@gmx.de> wrote: >>>> >>>> Since commit 0a6b58c5cd0d ("lockdep: fix static memory detection even >>>> more") the lockdep code uses is_kernel_core_data(), is_kernel_rodata() >>>> and init_section_contains() to verify if a lock is located inside a >>>> kernel static data section. >>>> >>>> This change triggers a failure on LoongArch, for which the vmlinux.lds.S >>>> script misses to put the locks (as part of in the .data.rel symbols) >>>> into the Linux data section. >>>> This patch fixes the lockdep problem by moving *(.data.rel*) symbols >>>> into the kernel data section (from _sdata to _edata). >>>> >>>> Additionally, move other wrongly assigned symbols too: >>>> - altinstructions into the _initdata section, >> >>> I think altinstructions cannot be put into _initdata because it will >>> be used by modules. >> >> No. >> arch/loongarch/kernel/vmlinux.lds.S is used for the static parts of the kernel >> and altinstructions are replaced before modules are loaded. >> For altinstructions in modules the linker script scripts/module.lds.S is used.
> OK, then what about .got/.plt? It seems arm64 also doesn't put them in > the data section.
arm64 seems to throw away all plt entries already at link time (and just keeps the got.plt in the read-only data section). It even checks at link time, that there are no plt entries in the binary: ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
I don't know for loongarch, but if you need the plt entries for loongarch, it's safest & best to put them into the read-only data section too, which is what my patch does. Up to now, you have them completely outside of code & data sections.
In the end you need to decide for your platform. My patch is a suggestion, which I think is correct (untested by me, but Guenter replied he tested it). But to fix the lockdep problem at minimum the move of the .data.rel section is needed.
Helge
>> >>>> - PLT symbols behind the read-only section, and >>>> - *(.la_abs) into the data section. >>>> >>>> Signed-off-by: Helge Deller <deller@gmx.de> >>>> Reported-by: Guenter Roeck <linux@roeck-us.net> >>>> Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more") >>>> Cc: stable <stable@kernel.org> # v6.4+ >>>> >>>> diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S >>>> index b1686afcf876..bb2ec86f37a8 100644 >>>> --- a/arch/loongarch/kernel/vmlinux.lds.S >>>> +++ b/arch/loongarch/kernel/vmlinux.lds.S >>>> @@ -53,33 +53,6 @@ SECTIONS >>>> . = ALIGN(PECOFF_SEGMENT_ALIGN); >>>> _etext = .; >>>> >>>> - /* >>>> - * struct alt_inst entries. From the header (alternative.h): >>>> - * "Alternative instructions for different CPU types or capabilities" >>>> - * Think locking instructions on spinlocks. >>>> - */ >>>> - . = ALIGN(4); >>>> - .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) { >>>> - __alt_instructions = .; >>>> - *(.altinstructions) >>>> - __alt_instructions_end = .; >>>> - } >>>> - >>>> -#ifdef CONFIG_RELOCATABLE >>>> - . = ALIGN(8); >>>> - .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) { >>>> - __la_abs_begin = .; >>>> - *(.la_abs) >>>> - __la_abs_end = .; >>>> - } >>>> -#endif >>>> - >>>> - .got : ALIGN(16) { *(.got) } >>>> - .plt : ALIGN(16) { *(.plt) } >>>> - .got.plt : ALIGN(16) { *(.got.plt) } >>>> - >>>> - .data.rel : { *(.data.rel*) } >>>> - >>>> . = ALIGN(PECOFF_SEGMENT_ALIGN); >>>> __init_begin = .; >>>> __inittext_begin = .; >>>> @@ -94,6 +67,18 @@ SECTIONS >>>> >>>> __initdata_begin = .; >>>> >>>> + /* >>>> + * struct alt_inst entries. From the header (alternative.h): >>>> + * "Alternative instructions for different CPU types or capabilities" >>>> + * Think locking instructions on spinlocks. >>>> + */ >>>> + . = ALIGN(4); >>>> + .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) { >>>> + __alt_instructions = .; >>>> + *(.altinstructions) >>>> + __alt_instructions_end = .; >>>> + } >>>> + >>>> INIT_DATA_SECTION(16) >>>> .exit.data : { >>>> EXIT_DATA >>>> @@ -113,6 +98,11 @@ SECTIONS >>>> >>>> _sdata = .; >>>> RO_DATA(4096) >>>> + >>>> + .got : ALIGN(16) { *(.got) } >>>> + .plt : ALIGN(16) { *(.plt) } >>>> + .got.plt : ALIGN(16) { *(.got.plt) } >>>> + >>>> RW_DATA(1 << CONFIG_L1_CACHE_SHIFT, PAGE_SIZE, THREAD_SIZE) >>>> >>>> .rela.dyn : ALIGN(8) { >>>> @@ -121,6 +111,17 @@ SECTIONS >>>> __rela_dyn_end = .; >>>> } >>>> >>>> + .data.rel : { *(.data.rel*) } >>>> + >>>> +#ifdef CONFIG_RELOCATABLE >>>> + . = ALIGN(8); >>>> + .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) { >>>> + __la_abs_begin = .; >>>> + *(.la_abs) >>>> + __la_abs_end = .; >>>> + } >>>> +#endif >>>> + >>>> .sdata : { >>>> *(.sdata) >>>> } >>
| |