Re: [PATCH v3 3/5] RISC-V: Align the .init.text section

From: Palmer Dabbelt
Date: Tue Dec 22 2020 - 23:15:22 EST


On Fri, 18 Dec 2020 00:19:09 PST (-0800), atishp@xxxxxxxxxxxxxx wrote:
On Thu, Dec 17, 2020 at 12:33 AM Atish Patra <atishp@xxxxxxxxxxxxxx> wrote:

On Wed, Dec 16, 2020 at 10:51 PM Palmer Dabbelt <palmer@xxxxxxxxxxx> 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@xxxxxxxxxx>
> >> Signed-off-by: Atish Patra <atish.patra@xxxxxxx>
> >> ---
> >> 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@xxxxxxxxxx/

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@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish