Date:   Tue, 22 Dec 2020 20:14:33 -0800 (PST)
From:   Palmer Dabbelt <palmer@...belt.com>
To:     atishp@...shpatra.org
CC:     Alistair Francis <Alistair.Francis@....com>, aou@...s.berkeley.edu,
        zong.li@...ive.com, anup@...infault.org,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
        Atish Patra <Atish.Patra@....com>, ren_guo@...ky.com,
        Paul Walmsley <paul.walmsley@...ive.com>, ojeda@...nel.org,
        greentime.hu@...ive.com, akpm@...ux-foundation.org,
        walken@...gle.com, ardb@...nel.org, rppt@...nel.org,
        Jim Wilson <jimw@...ive.com>
Subject:     Re: [PATCH v3 3/5] RISC-V: Align the .init.text section

On Fri, 18 Dec 2020 00:19:09 PST (-0800), atishp@...shpatra.org wrote:
> On Thu, Dec 17, 2020 at 12:33 AM Atish Patra <atishp@...shpatra.org> wrote:
>>
>> On Wed, Dec 16, 2020 at 10:51 PM Palmer Dabbelt <palmer@...belt.com> wrote:
>> >
>> > On Tue, 15 Dec 2020 22:02:54 PST (-0800), Palmer Dabbelt wrote:
>> > > On Wed, 04 Nov 2020 16:04:37 PST (-0800), Atish Patra wrote:
>> > >> In order to improve kernel text protection, we need separate .init.text/
>> > >> .init.data/.text in separate sections. However, RISC-V linker relaxation
>> > >> code is not aware of any alignment between sections. As a result, it may
>> > >> relax any RISCV_CALL relocations between sections to JAL without realizing
>> > >> that an inter section alignment may move the address farther. That may
>> > >> lead to a relocation truncated fit error. However, linker relaxation code
>> > >> is aware of the individual section alignments.
>> > >>
>> > >> The detailed discussion on this issue can be found here.
>> > >> https://github.com/riscv/riscv-gnu-toolchain/issues/738
>> > >>
>> > >> Keep the .init.text section aligned so that linker relaxation will take
>> > >> that as a hint while relaxing inter section calls.
>> > >> Here are the code size changes for each section because of this change.
>> > >>
>> > >> section         change in size (in bytes)
>> > >>   .head.text      +4
>> > >>   .text           +40
>> > >>   .init.text      +6530
>> > >>   .exit.text      +84
>> > >>
>> > >> The only significant increase in size happened for .init.text because
>> > >> all intra relocations also use 2MB alignment.
>> > >>
>> > >> Suggested-by: Jim Wilson <jimw@...ive.com>
>> > >> Signed-off-by: Atish Patra <atish.patra@....com>
>> > >> ---
>> > >>  arch/riscv/kernel/vmlinux.lds.S | 8 +++++++-
>> > >>  1 file changed, 7 insertions(+), 1 deletion(-)
>> > >>
>> > >> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
>> > >> index 3ffbd6cbdb86..cacd7898ba7f 100644
>> > >> --- a/arch/riscv/kernel/vmlinux.lds.S
>> > >> +++ b/arch/riscv/kernel/vmlinux.lds.S
>> > >> @@ -30,7 +30,13 @@ SECTIONS
>> > >>      . = ALIGN(PAGE_SIZE);
>> > >>
>> > >>      __init_begin = .;
>> > >> -    INIT_TEXT_SECTION(PAGE_SIZE)
>> > >> +    __init_text_begin = .;
>> > >> +    .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) ALIGN(SECTION_ALIGN) { \
>> > >> +            _sinittext = .;                                         \
>> > >> +            INIT_TEXT                                               \
>> > >> +            _einittext = .;                                         \
>> > >> +    }
>> > >> +
>> > >>      . = ALIGN(8);
>> > >>      __soc_early_init_table : {
>> > >>              __soc_early_init_table_start = .;
>> > >
>> > > Not sure what's going on here (or why I wasn't catching it earlier), but this
>> > > is breaking boot on one of my test configs.  I'm not getting any Linux boot
>> > > spew, so it's something fairly early.  I'm running defconfig with
>> > >
>> > >     CONFIG_PREEMPT=y
>> > >     CONFIG_DEBUG_PREEMPT=y
>> > >     CONFIG_PROVE_LOCKING=y
>> > >
>> > > It looks like that's been throwing a bunch of warnings for a while, but it did
>> > > at least used to boot.  No idea what PREEMPT would have to do with this, and
>> > > the other two don't generally trigger issues that early in boot (or at least,
>> > > trigger halts that early in boot).
>> > >
>
> I am able to reproduce this issue but with CONFIG_PROVE_LOCKING not
> CONFIG_PREEMPT.
> With CONFIG_PREEMPT, I see a bunch of warnings around smp_processor_id
> but it boots even with 5.0.
> If CONFIG_PROVE_LOCKING is enabled, I am not able to boot using 5.0.
> However, 5.2.0 works fine.
> I am going to take a look at the issue with 5.0 and PROVE_LOCKING.
>
> The config preempt warnings are resolved by the following patch. I
> have tested it in Qemu.
>
> https://patchwork.kernel.org/project/linux-riscv/patch/20201116081238.44223-1-wangkefeng.wang@huawei.com/

Thanks!

>
>> > > There's a bunch of other stuff that depends on this that's on for-next so I
>> > > don't want to just drop it, but I also don't want to break something.  I'm just
>> > > running QEMU's virt board.
>> > >
>>
>> I just verified for-next on QEMU 5.2.0 for virt (RV32,64, nommu) and
>> sifive_u as well.
>> I will give it a try on unleashed tomorrow as well with the above
>> configs enabled.
>>
>> > > I'll take a look again tomorrow night, but if anyone has some time to look
>> > > that'd be great!
>> >
>> > Looks like this breaks on QEMU 5.0.0 but works on 5.2.0.
>>
>> I will take a look tomorrow to check the root cause.
>>
>> I guess technically
>> > that means could be considered a regression, but as we don't really have any
>> > scheme for which old versions of QEMU we support it's not absolute.  I'd
>> > usually err on the side of keeping support for older platforms, but in this
>> > case it's probably just not worth the time so I'm going to just ignore it.
>> >
>> > _______________________________________________
>> > linux-riscv mailing list
>> > linux-riscv@...ts.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>>
>>
>> --
>> Regards,
>> Atish

Powered by blists - more mailing lists