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

From: Linus Walleij <linus.walleij@linaro.org>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	"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: Mon, 11 Mar 2024 23:17:21 +0100	[thread overview]
Message-ID: <CACRpkdaVkaM4ERUcK-3BqHK6O9hBtj4tEHZaQXbvW9Oh23c8Gw@mail.gmail.com> (raw)
In-Reply-To: <CABCJKuf7xmCBs8UrpEbeRtQnofuq2Pp4M=RGT4TUw1+HsvS_CA@mail.gmail.com>

On Mon, Mar 11, 2024 at 10:37 PM Sami Tolvanen <samitolvanen@google.com> wrote:
> On Mon, Mar 11, 2024 at 7:51 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Mon, Mar 11, 2024 at 4:35 PM Sami Tolvanen <samitolvanen@google.com> wrote:
> > > On Mon, Mar 11, 2024 at 3:04 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>
> > > > This works for some function attributes (e.g., __efiapi is used like
> > > > this), but the attribute specifier to which __nocfi resolves does not
> > > > appear to be usable in the same manner.
> > > >
> > > > Best would be to annotate the asm code using
> > > > SYM_TYPED_FUNC_START/_END, so that the CFI machinery is invoked at the
> > > > call site to validate the function type of the destination.
> > >
> > > Agreed, ideally we would annotate indirectly called assembly functions
> > > with CFI types and avoid __nocfi wrappers.
> >
> > I'm taking a stab at SYM_TYPED_FUNC_* for ARM, as we don't have them
> > yet.
>
> Does the default implementation in include/linux/cfi_types.h not work
> on arm for some reason?

For example I try to switch over the TLB symbols like this:

diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index e43f6d716b4b..bbe47ca32e55 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -341,7 +341,7 @@ ENTRY(\name\()_cache_fns)
 .macro define_tlb_functions name:req, flags_up:req, flags_smp
        .type   \name\()_tlb_fns, #object
        .align 2
-ENTRY(\name\()_tlb_fns)
+SYM_TYPED_FUNC_START(\name\()_tlb_fns)
        .long   \name\()_flush_user_tlb_range
        .long   \name\()_flush_kern_tlb_range
        .ifnb \flags_smp
diff --git a/arch/arm/mm/tlb-v7.S b/arch/arm/mm/tlb-v7.S
index 35fd6d4f0d03..aff9d884c30d 100644
--- a/arch/arm/mm/tlb-v7.S
+++ b/arch/arm/mm/tlb-v7.S
@@ -10,6 +10,7 @@
  */
 #include <linux/init.h>
 #include <linux/linkage.h>
+#include <linux/cfi_types.h>
 #include <asm/assembler.h>
 #include <asm/asm-offsets.h>
 #include <asm/page.h>
@@ -31,7 +32,7 @@
  *     - the "Invalidate single entry" instruction will invalidate
  *       both the I and the D TLBs on Harvard-style TLBs
  */
-ENTRY(v7wbi_flush_user_tlb_range)
+SYM_TYPED_FUNC_START(v7wbi_flush_user_tlb_range)
        vma_vm_mm r3, r2                        @ get vma->vm_mm
        mmid    r3, r3                          @ get vm_mm->context.id
        dsb     ish
@@ -57,7 +58,7 @@ ENTRY(v7wbi_flush_user_tlb_range)
        blo     1b
        dsb     ish
        ret     lr
-ENDPROC(v7wbi_flush_user_tlb_range)
+SYM_FUNC_END(v7wbi_flush_user_tlb_range)

 /*
  *     v7wbi_flush_kern_tlb_range(start,end)
@@ -67,7 +68,7 @@ ENDPROC(v7wbi_flush_user_tlb_range)
  *     - start - start address (may not be aligned)
  *     - end   - end address (exclusive, may not be aligned)
  */
-ENTRY(v7wbi_flush_kern_tlb_range)
+SYM_TYPED_FUNC_START(v7wbi_flush_kern_tlb_range)
        dsb     ish
        mov     r0, r0, lsr #PAGE_SHIFT         @ align address
        mov     r1, r1, lsr #PAGE_SHIFT
@@ -86,7 +87,7 @@ ENTRY(v7wbi_flush_kern_tlb_range)
        dsb     ish
        isb
        ret     lr
-ENDPROC(v7wbi_flush_kern_tlb_range)
+SYM_FUNC_END(v7wbi_flush_kern_tlb_range)

        __INIT


Compiling results in:

  AR      vmlinux.a
  LD      vmlinux.o
  OBJCOPY modules.builtin.modinfo
  GEN     modules.builtin
  MODPOST vmlinux.symvers
  UPD     include/generated/utsversion.h
  CC      init/version-timestamp.o
  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

Yours,
Linus Walleij
