Re: [PATCH v3 2/9] ARM: tlbflush: Make TLB flushes into static inlines

From: Ard Biesheuvel <ardb@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Sami Tolvanen <samitolvanen@google.com>,
	 "Russell King (Oracle)" <linux@armlinux.org.uk>,
	Kees Cook <keescook@chromium.org>,
	 Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	 Arnd Bergmann <arnd@arndb.de>,
	linux-arm-kernel@lists.infradead.org, llvm@lists.linux.dev
Subject: Re: [PATCH v3 2/9] ARM: tlbflush: Make TLB flushes into static inlines
Date: Tue, 12 Mar 2024 08:24:45 +0100	[thread overview]
Message-ID: <CAMj1kXFHFBKbspu6yBg14KzLNv-jHHjHbnuYf1avZx8C9t6tsQ@mail.gmail.com> (raw)
In-Reply-To: <CACRpkdZOsST_QtouokNgF0XRVSyoOJQr0cfAGTMjSCb7ezBLPg@mail.gmail.com>

On Tue, 12 Mar 2024 at 00:56, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Mar 11, 2024 at 11:28 PM Sami Tolvanen <samitolvanen@google.com> wrote:
> > On Mon, Mar 11, 2024 at 3:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > >
> > >   LD      .tmp_vmlinux.kallsyms1
> > > ld.lld: error: undefined symbol: __kcfi_typeid_v7wbi_flush_user_tlb_range
> > > >>> referenced by arch/arm/mm/tlb-v7.o:(.text+0x0) in archive vmlinux.a
> > >
> > > ld.lld: error: undefined symbol: __kcfi_typeid_v7wbi_flush_kern_tlb_range
> > > >>> referenced by tlb-v7.S:60 (/mnt/storage/linus/linux-integrator/build-vexpress/../arch/arm/mm/tlb-v7.S:60)
> > > >>>               arch/arm/mm/tlb-v7.o:(.text+0x40) in archive vmlinux.a
> > >
> > > ld.lld: error: undefined symbol: __kcfi_typeid_v7wbi_tlb_fns
> > > >>> referenced by arch/arm/mm/tlb-v7.o:(.init.text+0x0) in archive vmlinux.a
> >
> > Clang only emits __kcfi_typeid symbols for functions that are
> > address-taken in C code. You need to add __ADDRESSABLE(function)
> > references to a C file somewhere for functions that otherwise are not
> > address-taken.
>
> Hey it works. So for example if for these functions I also add:
>
> diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
> index d19d140a10c7..23eb0f9358cb 100644
> --- a/arch/arm/mm/flush.c
> +++ b/arch/arm/mm/flush.c
> @@ -18,6 +18,11 @@
>
>  #include "mm.h"
>
> +void v7wbi_flush_user_tlb_range(unsigned long, unsigned long, struct
> vm_area_struct *);
> +void v7wbi_flush_kern_tlb_range(unsigned long, unsigned long);
> +__ADDRESSABLE(v7wbi_flush_user_tlb_range);
> +__ADDRESSABLE(v7wbi_flush_kern_tlb_range);
>
> Then that works.
>
> The problem is that I also have to define all these function signatures that
> are never used in C and there are quite a few of them, if I start listing them
> all and #ifdefining them for selected CPUs it's not going to be pretty.
>
> It can be done and they can be in a cfi-defs.c file though.
> And it's better than __nocfi.
>
> The complexity comes from the fact that arm can boot a kernel
> with support for several different CPU:s.
>
> The different CPU management functions are put in a list of supported
> processors by the linker, and then e.g. the tlb maintenance functions
> are dereferenced directly from *list->tlb in setup_processor()
> in arch/arm/kernel/setup.c.
>

Another option is to move the struct definitions to C entirely. For
example, the branch below implements this for the tlbflush code:

https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm-cfi

However, doing the same will be tricky for the proc_info structs, as
they have a member that contains a place-relative offset, and those
cannot be easily emitted in C (similar to the SMP_ON_UP hack in the
TLB code above).