Received-SPF: pass (google.com: domain of linux-kernel+bounces-75029-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-153-g7e3bb84806-fm-20240215.007-g7e3bb848
Precedence: bulk
MIME-Version: 1.0
Message-Id: <30b01c65-12f2-4ee0-81d5-c7a2da2c36b4@app.fastmail.com>
In-Reply-To: <38c09a4b-69cc-4dc5-8a68-e5f5597613ac@huawei.com>
References: <20240220081527.23408-1-liuyuntao12@huawei.com>
 <1342759e-b967-4ec4-98d5-48146f81f695@app.fastmail.com>
 <38c09a4b-69cc-4dc5-8a68-e5f5597613ac@huawei.com>
Date: Wed, 21 Feb 2024 16:51:26 +0100
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Yuntao Liu" <liuyuntao12@huawei.com>,
 linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Cc: "Russell King" <linux@armlinux.org.uk>, "Andrew Davis" <afd@ti.com>,
 "Andrew Morton" <akpm@linux-foundation.org>,
 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
 "Geert Uytterhoeven" <geert+renesas@glider.be>,
 "Jonathan Corbet" <corbet@lwn.net>, "Mike Rapoport" <rppt@kernel.org>,
 "Eric DeVolder" <eric.devolder@oracle.com>, "Rob Herring" <robh@kernel.org>,
 "Thomas Gleixner" <tglx@linutronix.de>,
 "Linus Walleij" <linus.walleij@linaro.org>
Subject: Re: [PATCH -next] arm32: enable HAVE_LD_DEAD_CODE_DATA_ELIMINATION
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 20, 2024, at 10:53, liuyuntao (F) wrote:
> =E5=9C=A8 2024/2/20 16:40, Arnd Bergmann =E5=86=99=E9=81=93:
>> On Tue, Feb 20, 2024, at 09:15, Yuntao Liu wrote:
> #
> # ARM discards the .data section because it disallows r/w data in the
> # decompressor. So move our .data to .data.efistub and .bss to .bss.ef=
istub,
> # which are preserved explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM)	+=3D --rename-section .data=3D.data.efist=
ub	\
> 				   --rename-section .bss=3D.bss.efistub,load,alloc
>
> ---
>
> I think that .data.efistub represents the entire .data section, the sa=
me=20
> applies to .bss as well,
>
> so i move all .data and .bss into the stub here.
>

Ok, I see.=20

>>=20
>> I guess this prevents discarding any function that has a reference
>> from pv_table or ex_table, even if there are no other references,
>> right?
>
> Indeed so, if not keep ex_table,  the compilation process will result =
in
>
> an error:
>
>      no __ex_table in file: vmlinux
>
>      Failed to sort kernel tables

Sure, and without the ex_table contents, it would not be able
to recover from a failed uaccess either.

> and if not keep pv_table, It can be compiled successfully, but the QEM=
U=20
> boots will fail.

Right. The pv_table isn't technically necessary since it can
be disabled.  I think it was originally introduced in order
to avoid performance regressions when we introduced multiplatform
kernels that can run at arbitrary physical addresses rather than
having it as a build-time constant.

I don't know how much difference that actually makes for performance,
so turning it into a normal runtime lookup may or may not be
a good compromise when building with HAVE_LD_DEAD_CODE_DATA_ELIMINATION.

I have given your patch some build testing with random
configurations in my build setup and it seems to work
fine with gcc/binutils, but unfortunately I came across
a link failure using clang/lld:

ld.lld: error: ./arch/arm/kernel/vmlinux.lds:35: ( expected, but got }
>>>  __vectors_lma =3D .; OVERLAY 0xffff0000 : AT(__vectors_lma) { .vect=
ors { KEEP(*(.vectors)) } .vectors.bhb.loop8 { KEEP(*(.vectors.bhb.loop8=
)) } .vectors.bhb.bpiall { KEEP(*(.vectors.bhb.bpiall)) } } __vectors_st=
art =3D LOADADDR(.vectors); __vectors_end =3D LOADADDR(.vectors) + SIZEO=
F(.vectors); __vectors_bhb_loop8_start =3D LOADADDR(.vectors.bhb.loop8);=
 __vectors_bhb_loop8_end =3D LOADADDR(.vectors.bhb.loop8) + SIZEOF(.vect=
ors.bhb.loop8); __vectors_bhb_bpiall_start =3D LOADADDR(.vectors.bhb.bpi=
all); __vectors_bhb_bpiall_end =3D LOADADDR(.vectors.bhb.bpiall) + SIZEO=
F(.vectors.bhb.bpiall); . =3D __vectors_lma + SIZEOF(.vectors) + SIZEOF(=
vectors.bhb.loop8) + SIZEOF(.vectors.bhb.bpiall); __stubs_lma =3D .; .s=
tubs ADDR(.vectors) + 0x1000 : AT(__stubs_lma) { *(.stubs) } __stubs_sta=
rt =3D LOADADDR(.stubs); __stubs_end =3D LOADADDR(.stubs) + SIZEOF(.stub=
s); . =3D __stubs_lma + SIZEOF(.stubs); PROVIDE(vector_fiq_offset =3D ve=
ctor_fiq - ADDR(.vectors));
>>>                                                                     =
                      ^

I don't immediately see what the problem is here, I hope you
can address it before you send a v2.

     Arnd