Message-ID: <CAOnJCUJ4Z6wGQMy+7ELMNxRby5joe_U+9zjO66cmXGHKHkzLsg@mail.gmail.com>
Date:   Thu, 17 Dec 2020 00:33:28 -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 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).
> >
> > 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