Date:   Fri, 18 Dec 2020 00:19:09 -0800
From:   Atish Patra <atishp@...shpatra.org>
To:     Palmer Dabbelt <palmer@...belt.com>
Cc:     Alistair Francis <Alistair.Francis@....com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Zong Li <zong.li@...ive.com>, Anup Patel <anup@...infault.org>,
        "linux-kernel@...r.kernel.org List" <linux-kernel@...r.kernel.org>,
        linux-riscv <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 <greentime.hu@...ive.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michel Lespinasse <walken@...gle.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Mike Rapoport <rppt@...nel.org>, Jim Wilson <jimw@...ive.com>
Subject: Re: [PATCH v3 3/5] RISC-V: Align the .init.text section

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/

> > > 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



-- 
Regards,
Atish

Powered by blists - more mailing lists