[PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Kees Cook]

When KCOV is enabled all functions get instrumented, unless the
__no_sanitize_coverage attribute is used. To prepare for
__no_sanitize_coverage being applied to __init functions, we have to
handle differences in how GCC's inline optimizations get resolved. For
x86 this means forcing several functions to be inline with
__always_inline.

Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: <x86@kernel.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Michal Wilczynski <michal.wilczynski@intel.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Roger Pau Monne <roger.pau@citrix.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Usama Arif <usama.arif@bytedance.com>
Cc: "Guilherme G. Piccoli" <gpiccoli@igalia.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: <kvm@vger.kernel.org>
Cc: <ibm-acpi-devel@lists.sourceforge.net>
Cc: <platform-driver-x86@vger.kernel.org>
Cc: <linux-acpi@vger.kernel.org>
Cc: <linux-trace-kernel@vger.kernel.org>
Cc: <linux-efi@vger.kernel.org>
Cc: <linux-mm@kvack.org>
---
 arch/x86/include/asm/acpi.h     | 4 ++--
 arch/x86/include/asm/realmode.h | 2 +-
 include/linux/acpi.h            | 4 ++--
 include/linux/bootconfig.h      | 2 +-
 include/linux/efi.h             | 2 +-
 include/linux/memblock.h        | 2 +-
 include/linux/smp.h             | 2 +-
 arch/x86/kernel/kvm.c           | 2 +-
 arch/x86/mm/init_64.c           | 2 +-
 kernel/kexec_handover.c         | 4 ++--
 10 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 5ab1a4598d00..a03aa6f999d1 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -158,13 +158,13 @@ static inline bool acpi_has_cpu_in_madt(void)
 }
 
 #define ACPI_HAVE_ARCH_SET_ROOT_POINTER
-static inline void acpi_arch_set_root_pointer(u64 addr)
+static __always_inline void acpi_arch_set_root_pointer(u64 addr)
 {
 	x86_init.acpi.set_root_pointer(addr);
 }
 
 #define ACPI_HAVE_ARCH_GET_ROOT_POINTER
-static inline u64 acpi_arch_get_root_pointer(void)
+static __always_inline u64 acpi_arch_get_root_pointer(void)
 {
 	return x86_init.acpi.get_root_pointer();
 }
diff --git a/arch/x86/include/asm/realmode.h b/arch/x86/include/asm/realmode.h
index f607081a022a..e406a1e92c63 100644
--- a/arch/x86/include/asm/realmode.h
+++ b/arch/x86/include/asm/realmode.h
@@ -78,7 +78,7 @@ extern unsigned char secondary_startup_64[];
 extern unsigned char secondary_startup_64_no_verify[];
 #endif
 
-static inline size_t real_mode_size_needed(void)
+static __always_inline size_t real_mode_size_needed(void)
 {
 	if (real_mode_header)
 		return 0;	/* already allocated. */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 71e692f95290..1c5bb1e887cd 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -759,13 +759,13 @@ int acpi_arch_timer_mem_init(struct arch_timer_mem *timer_mem, int *timer_count)
 #endif
 
 #ifndef ACPI_HAVE_ARCH_SET_ROOT_POINTER
-static inline void acpi_arch_set_root_pointer(u64 addr)
+static __always_inline void acpi_arch_set_root_pointer(u64 addr)
 {
 }
 #endif
 
 #ifndef ACPI_HAVE_ARCH_GET_ROOT_POINTER
-static inline u64 acpi_arch_get_root_pointer(void)
+static __always_inline u64 acpi_arch_get_root_pointer(void)
 {
 	return 0;
 }
diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index 3f4b4ac527ca..25df9260d206 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -290,7 +290,7 @@ int __init xbc_get_info(int *node_size, size_t *data_size);
 /* XBC cleanup data structures */
 void __init _xbc_exit(bool early);
 
-static inline void xbc_exit(void)
+static __always_inline void xbc_exit(void)
 {
 	_xbc_exit(false);
 }
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 7d63d1d75f22..e3776d9cad07 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1334,7 +1334,7 @@ struct linux_efi_initrd {
 
 bool xen_efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table);
 
-static inline
+static __always_inline
 bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
 {
 	if (!IS_ENABLED(CONFIG_XEN_EFI))
diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index bb19a2534224..b96746376e17 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
 					  NUMA_NO_NODE);
 }
 
-static inline void *memblock_alloc_from(phys_addr_t size,
+static __always_inline void *memblock_alloc_from(phys_addr_t size,
 						phys_addr_t align,
 						phys_addr_t min_addr)
 {
diff --git a/include/linux/smp.h b/include/linux/smp.h
index bea8d2826e09..18e9c918325e 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -221,7 +221,7 @@ static inline void wake_up_all_idle_cpus(void) {  }
 
 #ifdef CONFIG_UP_LATE_INIT
 extern void __init up_late_init(void);
-static inline void smp_init(void) { up_late_init(); }
+static __always_inline void smp_init(void) { up_late_init(); }
 #else
 static inline void smp_init(void) { }
 #endif
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 921c1c783bc1..8ae750cde0c6 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -420,7 +420,7 @@ static u64 kvm_steal_clock(int cpu)
 	return steal;
 }
 
-static inline void __set_percpu_decrypted(void *ptr, unsigned long size)
+static inline __init void __set_percpu_decrypted(void *ptr, unsigned long size)
 {
 	early_set_memory_decrypted((unsigned long) ptr, size);
 }
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index fdb6cab524f0..76e33bd7c556 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -805,7 +805,7 @@ kernel_physical_mapping_change(unsigned long paddr_start,
 }
 
 #ifndef CONFIG_NUMA
-static inline void x86_numa_init(void)
+static __always_inline void x86_numa_init(void)
 {
 	memblock_set_node(0, PHYS_ADDR_MAX, &memblock.memory, 0);
 }
diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
index 49634cc3fb43..e49743ae52c5 100644
--- a/kernel/kexec_handover.c
+++ b/kernel/kexec_handover.c
@@ -310,8 +310,8 @@ static int kho_mem_serialize(struct kho_serialization *ser)
 	return -ENOMEM;
 }
 
-static void deserialize_bitmap(unsigned int order,
-			       struct khoser_mem_bitmap_ptr *elm)
+static void __init deserialize_bitmap(unsigned int order,
+				      struct khoser_mem_bitmap_ptr *elm)
 {
 	struct kho_mem_phys_bits *bitmap = KHOSER_LOAD_PTR(elm->bitmap);
 	unsigned long bit;
-- 
2.34.1

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Mike Rapoport]

Hi Kees,

On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> When KCOV is enabled all functions get instrumented, unless the
> __no_sanitize_coverage attribute is used. To prepare for
> __no_sanitize_coverage being applied to __init functions, we have to
> handle differences in how GCC's inline optimizations get resolved. For
> x86 this means forcing several functions to be inline with
> __always_inline.
> 
> Signed-off-by: Kees Cook <kees@kernel.org>

...

> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index bb19a2534224..b96746376e17 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
>  					  NUMA_NO_NODE);
>  }
>  
> -static inline void *memblock_alloc_from(phys_addr_t size,
> +static __always_inline void *memblock_alloc_from(phys_addr_t size,
>  						phys_addr_t align,
>  						phys_addr_t min_addr)

I'm curious why from all memblock_alloc* wrappers this is the only one that
needs to be __always_inline?

-- 
Sincerely yours,
Mike.

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Kees Cook]

On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> Hi Kees,
> 
> On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > When KCOV is enabled all functions get instrumented, unless the
> > __no_sanitize_coverage attribute is used. To prepare for
> > __no_sanitize_coverage being applied to __init functions, we have to
> > handle differences in how GCC's inline optimizations get resolved. For
> > x86 this means forcing several functions to be inline with
> > __always_inline.
> > 
> > Signed-off-by: Kees Cook <kees@kernel.org>
> 
> ...
> 
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index bb19a2534224..b96746376e17 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> >  					  NUMA_NO_NODE);
> >  }
> >  
> > -static inline void *memblock_alloc_from(phys_addr_t size,
> > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> >  						phys_addr_t align,
> >  						phys_addr_t min_addr)
> 
> I'm curious why from all memblock_alloc* wrappers this is the only one that
> needs to be __always_inline?

Thread-merge[1], adding Will Deacon, who was kind of asking the same
question.

Based on what I can tell, GCC has kind of fragile inlining logic, in the
sense that it can change whether or not it inlines something based on
optimizations. It looks like the kcov instrumentation being added (or in
this case, removed) from a function changes the optimization results,
and some functions marked "inline" are _not_ inlined. In that case, we end up
with __init code calling a function not marked __init, and we get the
build warnings I'm trying to eliminate.

So, to Will's comment, yes, the problem is somewhat fragile (though
using either __always_inline or __init will deterministically solve it).
We've tripped over this before with GCC and the solution has usually
been to just use __always_inline and move on.

For memblock_alloc*, it appears to be that the heuristic GCC uses
resulted in only memblock_alloc_from() being a problem in this case. I
can certainly mark them all as __always_inline if that is preferred.

Some maintainers have wanted things marked __init, some have wanted
__always_inline. I opted for __always_inline since that was basically
the intent of marking a function "inline" in the first place. I am happy
to do whatever. :)

-Kees

[1] https://lore.kernel.org/lkml/aHouXI5-tyQw78Ht@willie-the-truck/

-- 
Kees Cook

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Ard Biesheuvel]

On Sat, 19 Jul 2025 at 08:51, Kees Cook <kees@kernel.org> wrote:
>
> On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> > Hi Kees,
> >
> > On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > > When KCOV is enabled all functions get instrumented, unless the
> > > __no_sanitize_coverage attribute is used. To prepare for
> > > __no_sanitize_coverage being applied to __init functions, we have to
> > > handle differences in how GCC's inline optimizations get resolved. For
> > > x86 this means forcing several functions to be inline with
> > > __always_inline.
> > >
> > > Signed-off-by: Kees Cook <kees@kernel.org>
> >
> > ...
> >
> > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > index bb19a2534224..b96746376e17 100644
> > > --- a/include/linux/memblock.h
> > > +++ b/include/linux/memblock.h
> > > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> > >                                       NUMA_NO_NODE);
> > >  }
> > >
> > > -static inline void *memblock_alloc_from(phys_addr_t size,
> > > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> > >                                             phys_addr_t align,
> > >                                             phys_addr_t min_addr)
> >
> > I'm curious why from all memblock_alloc* wrappers this is the only one that
> > needs to be __always_inline?
>
> Thread-merge[1], adding Will Deacon, who was kind of asking the same
> question.
>
> Based on what I can tell, GCC has kind of fragile inlining logic, in the
> sense that it can change whether or not it inlines something based on
> optimizations. It looks like the kcov instrumentation being added (or in
> this case, removed) from a function changes the optimization results,
> and some functions marked "inline" are _not_ inlined. In that case, we end up
> with __init code calling a function not marked __init, and we get the
> build warnings I'm trying to eliminate.
>
> So, to Will's comment, yes, the problem is somewhat fragile (though
> using either __always_inline or __init will deterministically solve it).
> We've tripped over this before with GCC and the solution has usually
> been to just use __always_inline and move on.
>

Given that 'inline' is already a macro in the kernel, could we just
add __attribute__((__always_inline__)) to it when KCOV is enabled?

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Will Deacon]

On Sun, Jul 20, 2025 at 04:10:01PM +1000, Ard Biesheuvel wrote:
> On Sat, 19 Jul 2025 at 08:51, Kees Cook <kees@kernel.org> wrote:
> > On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> > > On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > > > When KCOV is enabled all functions get instrumented, unless the
> > > > __no_sanitize_coverage attribute is used. To prepare for
> > > > __no_sanitize_coverage being applied to __init functions, we have to
> > > > handle differences in how GCC's inline optimizations get resolved. For
> > > > x86 this means forcing several functions to be inline with
> > > > __always_inline.
> > > >
> > > > Signed-off-by: Kees Cook <kees@kernel.org>
> > >
> > > ...
> > >
> > > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > > index bb19a2534224..b96746376e17 100644
> > > > --- a/include/linux/memblock.h
> > > > +++ b/include/linux/memblock.h
> > > > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> > > >                                       NUMA_NO_NODE);
> > > >  }
> > > >
> > > > -static inline void *memblock_alloc_from(phys_addr_t size,
> > > > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> > > >                                             phys_addr_t align,
> > > >                                             phys_addr_t min_addr)
> > >
> > > I'm curious why from all memblock_alloc* wrappers this is the only one that
> > > needs to be __always_inline?
> >
> > Thread-merge[1], adding Will Deacon, who was kind of asking the same
> > question.
> >
> > Based on what I can tell, GCC has kind of fragile inlining logic, in the
> > sense that it can change whether or not it inlines something based on
> > optimizations. It looks like the kcov instrumentation being added (or in
> > this case, removed) from a function changes the optimization results,
> > and some functions marked "inline" are _not_ inlined. In that case, we end up
> > with __init code calling a function not marked __init, and we get the
> > build warnings I'm trying to eliminate.

Got it, thanks for the explanation!

> > So, to Will's comment, yes, the problem is somewhat fragile (though
> > using either __always_inline or __init will deterministically solve it).
> > We've tripped over this before with GCC and the solution has usually
> > been to just use __always_inline and move on.
> >
> 
> Given that 'inline' is already a macro in the kernel, could we just
> add __attribute__((__always_inline__)) to it when KCOV is enabled?

That sounds like a more robust approach and, by the sounds of it, we
could predicate it on GCC too. That would also provide a neat place for
a comment describing the problem.

Kees, would that work for you?

Will

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Kees Cook]

On Mon, Jul 21, 2025 at 01:47:55PM +0100, Will Deacon wrote:
> On Sun, Jul 20, 2025 at 04:10:01PM +1000, Ard Biesheuvel wrote:
> > On Sat, 19 Jul 2025 at 08:51, Kees Cook <kees@kernel.org> wrote:
> > > On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> > > > On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > > > > When KCOV is enabled all functions get instrumented, unless the
> > > > > __no_sanitize_coverage attribute is used. To prepare for
> > > > > __no_sanitize_coverage being applied to __init functions, we have to
> > > > > handle differences in how GCC's inline optimizations get resolved. For
> > > > > x86 this means forcing several functions to be inline with
> > > > > __always_inline.
> > > > >
> > > > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > >
> > > > ...
> > > >
> > > > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > > > index bb19a2534224..b96746376e17 100644
> > > > > --- a/include/linux/memblock.h
> > > > > +++ b/include/linux/memblock.h
> > > > > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> > > > >                                       NUMA_NO_NODE);
> > > > >  }
> > > > >
> > > > > -static inline void *memblock_alloc_from(phys_addr_t size,
> > > > > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> > > > >                                             phys_addr_t align,
> > > > >                                             phys_addr_t min_addr)
> > > >
> > > > I'm curious why from all memblock_alloc* wrappers this is the only one that
> > > > needs to be __always_inline?
> > >
> > > Thread-merge[1], adding Will Deacon, who was kind of asking the same
> > > question.
> > >
> > > Based on what I can tell, GCC has kind of fragile inlining logic, in the
> > > sense that it can change whether or not it inlines something based on
> > > optimizations. It looks like the kcov instrumentation being added (or in
> > > this case, removed) from a function changes the optimization results,
> > > and some functions marked "inline" are _not_ inlined. In that case, we end up
> > > with __init code calling a function not marked __init, and we get the
> > > build warnings I'm trying to eliminate.
> 
> Got it, thanks for the explanation!
> 
> > > So, to Will's comment, yes, the problem is somewhat fragile (though
> > > using either __always_inline or __init will deterministically solve it).
> > > We've tripped over this before with GCC and the solution has usually
> > > been to just use __always_inline and move on.
> > >
> > 
> > Given that 'inline' is already a macro in the kernel, could we just
> > add __attribute__((__always_inline__)) to it when KCOV is enabled?
> 
> That sounds like a more robust approach and, by the sounds of it, we
> could predicate it on GCC too. That would also provide a neat place for
> a comment describing the problem.
> 
> Kees, would that work for you?

That seems like an extremely large hammer for this problem, IMO. It
feels like it could cause new strange corner cases. I'd much prefer the
small fixes I've currently got since it keeps it focused. KCOV is
already enabled for "allmodconfig", so any new instances would be found
very quickly, etc. (And GCC's fragility in this regard has already been
exposed to these cases -- it's just that I changed one of the
combinations of __init vs inline vs instrumentation.

I could give it a try, if you really prefer the big hammer approach...

-- 
Kees Cook

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Kees Cook]

On Mon, Jul 21, 2025 at 01:14:36PM -0700, Kees Cook wrote:
> On Mon, Jul 21, 2025 at 01:47:55PM +0100, Will Deacon wrote:
> > On Sun, Jul 20, 2025 at 04:10:01PM +1000, Ard Biesheuvel wrote:
> > > On Sat, 19 Jul 2025 at 08:51, Kees Cook <kees@kernel.org> wrote:
> > > > On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> > > > > On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > > > > > When KCOV is enabled all functions get instrumented, unless the
> > > > > > __no_sanitize_coverage attribute is used. To prepare for
> > > > > > __no_sanitize_coverage being applied to __init functions, we have to
> > > > > > handle differences in how GCC's inline optimizations get resolved. For
> > > > > > x86 this means forcing several functions to be inline with
> > > > > > __always_inline.
> > > > > >
> > > > > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > > >
> > > > > ...
> > > > >
> > > > > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > > > > index bb19a2534224..b96746376e17 100644
> > > > > > --- a/include/linux/memblock.h
> > > > > > +++ b/include/linux/memblock.h
> > > > > > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> > > > > >                                       NUMA_NO_NODE);
> > > > > >  }
> > > > > >
> > > > > > -static inline void *memblock_alloc_from(phys_addr_t size,
> > > > > > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> > > > > >                                             phys_addr_t align,
> > > > > >                                             phys_addr_t min_addr)
> > > > >
> > > > > I'm curious why from all memblock_alloc* wrappers this is the only one that
> > > > > needs to be __always_inline?
> > > >
> > > > Thread-merge[1], adding Will Deacon, who was kind of asking the same
> > > > question.
> > > >
> > > > Based on what I can tell, GCC has kind of fragile inlining logic, in the
> > > > sense that it can change whether or not it inlines something based on
> > > > optimizations. It looks like the kcov instrumentation being added (or in
> > > > this case, removed) from a function changes the optimization results,
> > > > and some functions marked "inline" are _not_ inlined. In that case, we end up
> > > > with __init code calling a function not marked __init, and we get the
> > > > build warnings I'm trying to eliminate.
> > 
> > Got it, thanks for the explanation!
> > 
> > > > So, to Will's comment, yes, the problem is somewhat fragile (though
> > > > using either __always_inline or __init will deterministically solve it).
> > > > We've tripped over this before with GCC and the solution has usually
> > > > been to just use __always_inline and move on.
> > > >
> > > 
> > > Given that 'inline' is already a macro in the kernel, could we just
> > > add __attribute__((__always_inline__)) to it when KCOV is enabled?
> > 
> > That sounds like a more robust approach and, by the sounds of it, we
> > could predicate it on GCC too. That would also provide a neat place for
> > a comment describing the problem.
> > 
> > Kees, would that work for you?
> 
> That seems like an extremely large hammer for this problem, IMO. It
> feels like it could cause new strange corner cases. I'd much prefer the
> small fixes I've currently got since it keeps it focused. KCOV is
> already enabled for "allmodconfig", so any new instances would be found
> very quickly, etc. (And GCC's fragility in this regard has already been
> exposed to these cases -- it's just that I changed one of the
> combinations of __init vs inline vs instrumentation.
> 
> I could give it a try, if you really prefer the big hammer approach...

I gave it a try -- it fails spectacularly. ;) Let's stick to my small
fixes instead?

-- 
Kees Cook

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Ard Biesheuvel]

On Tue, 22 Jul 2025 at 06:49, Kees Cook <kees@kernel.org> wrote:
>
> On Mon, Jul 21, 2025 at 01:14:36PM -0700, Kees Cook wrote:
> > On Mon, Jul 21, 2025 at 01:47:55PM +0100, Will Deacon wrote:
> > > On Sun, Jul 20, 2025 at 04:10:01PM +1000, Ard Biesheuvel wrote:
> > > > On Sat, 19 Jul 2025 at 08:51, Kees Cook <kees@kernel.org> wrote:
> > > > > On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> > > > > > On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > > > > > > When KCOV is enabled all functions get instrumented, unless the
> > > > > > > __no_sanitize_coverage attribute is used. To prepare for
> > > > > > > __no_sanitize_coverage being applied to __init functions, we have to
> > > > > > > handle differences in how GCC's inline optimizations get resolved. For
> > > > > > > x86 this means forcing several functions to be inline with
> > > > > > > __always_inline.
> > > > > > >
> > > > > > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > > > >
> > > > > > ...
> > > > > >
> > > > > > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > > > > > index bb19a2534224..b96746376e17 100644
> > > > > > > --- a/include/linux/memblock.h
> > > > > > > +++ b/include/linux/memblock.h
> > > > > > > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> > > > > > >                                       NUMA_NO_NODE);
> > > > > > >  }
> > > > > > >
> > > > > > > -static inline void *memblock_alloc_from(phys_addr_t size,
> > > > > > > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> > > > > > >                                             phys_addr_t align,
> > > > > > >                                             phys_addr_t min_addr)
> > > > > >
> > > > > > I'm curious why from all memblock_alloc* wrappers this is the only one that
> > > > > > needs to be __always_inline?
> > > > >
> > > > > Thread-merge[1], adding Will Deacon, who was kind of asking the same
> > > > > question.
> > > > >
> > > > > Based on what I can tell, GCC has kind of fragile inlining logic, in the
> > > > > sense that it can change whether or not it inlines something based on
> > > > > optimizations. It looks like the kcov instrumentation being added (or in
> > > > > this case, removed) from a function changes the optimization results,
> > > > > and some functions marked "inline" are _not_ inlined. In that case, we end up
> > > > > with __init code calling a function not marked __init, and we get the
> > > > > build warnings I'm trying to eliminate.
> > >
> > > Got it, thanks for the explanation!
> > >
> > > > > So, to Will's comment, yes, the problem is somewhat fragile (though
> > > > > using either __always_inline or __init will deterministically solve it).
> > > > > We've tripped over this before with GCC and the solution has usually
> > > > > been to just use __always_inline and move on.
> > > > >
> > > >
> > > > Given that 'inline' is already a macro in the kernel, could we just
> > > > add __attribute__((__always_inline__)) to it when KCOV is enabled?
> > >
> > > That sounds like a more robust approach and, by the sounds of it, we
> > > could predicate it on GCC too. That would also provide a neat place for
> > > a comment describing the problem.
> > >
> > > Kees, would that work for you?
> >
> > That seems like an extremely large hammer for this problem, IMO. It
> > feels like it could cause new strange corner cases. I'd much prefer the
> > small fixes I've currently got since it keeps it focused. KCOV is
> > already enabled for "allmodconfig", so any new instances would be found
> > very quickly, etc. (And GCC's fragility in this regard has already been
> > exposed to these cases -- it's just that I changed one of the
> > combinations of __init vs inline vs instrumentation.
> >
> > I could give it a try, if you really prefer the big hammer approach...
>
> I gave it a try -- it fails spectacularly. ;) Let's stick to my small
> fixes instead?
>

Fair enough :-)

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Mike Rapoport]

On Fri, Jul 18, 2025 at 03:51:28PM -0700, Kees Cook wrote:
> On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> > Hi Kees,
> > 
> > On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > > When KCOV is enabled all functions get instrumented, unless the
> > > __no_sanitize_coverage attribute is used. To prepare for
> > > __no_sanitize_coverage being applied to __init functions, we have to
> > > handle differences in how GCC's inline optimizations get resolved. For
> > > x86 this means forcing several functions to be inline with
> > > __always_inline.
> > > 
> > > Signed-off-by: Kees Cook <kees@kernel.org>
> > 
> > ...
> > 
> > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > index bb19a2534224..b96746376e17 100644
> > > --- a/include/linux/memblock.h
> > > +++ b/include/linux/memblock.h
> > > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> > >  					  NUMA_NO_NODE);
> > >  }
> > >  
> > > -static inline void *memblock_alloc_from(phys_addr_t size,
> > > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> > >  						phys_addr_t align,
> > >  						phys_addr_t min_addr)
> > 
> > I'm curious why from all memblock_alloc* wrappers this is the only one that
> > needs to be __always_inline?
> 
> Thread-merge[1], adding Will Deacon, who was kind of asking the same
> question.
> 
> Based on what I can tell, GCC has kind of fragile inlining logic, in the
> sense that it can change whether or not it inlines something based on
> optimizations. It looks like the kcov instrumentation being added (or in
> this case, removed) from a function changes the optimization results,
> and some functions marked "inline" are _not_ inlined. In that case, we end up
> with __init code calling a function not marked __init, and we get the
> build warnings I'm trying to eliminate.
> 
> So, to Will's comment, yes, the problem is somewhat fragile (though
> using either __always_inline or __init will deterministically solve it).
> We've tripped over this before with GCC and the solution has usually
> been to just use __always_inline and move on.
> 
> For memblock_alloc*, it appears to be that the heuristic GCC uses
> resulted in only memblock_alloc_from() being a problem in this case. I
> can certainly mark them all as __always_inline if that is preferred.

We had a few of those already converted to __always_inline, so I'm ok with
continuing to fix them one at at time. Gives a feeling of job security ;-)

> -- 
> Kees Cook

-- 
Sincerely yours,
Mike.

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

[Will Deacon]

On Tue, Jul 22, 2025 at 04:55:47PM +1000, Ard Biesheuvel wrote:
> On Tue, 22 Jul 2025 at 06:49, Kees Cook <kees@kernel.org> wrote:
> >
> > On Mon, Jul 21, 2025 at 01:14:36PM -0700, Kees Cook wrote:
> > > On Mon, Jul 21, 2025 at 01:47:55PM +0100, Will Deacon wrote:
> > > > On Sun, Jul 20, 2025 at 04:10:01PM +1000, Ard Biesheuvel wrote:
> > > > > On Sat, 19 Jul 2025 at 08:51, Kees Cook <kees@kernel.org> wrote:
> > > > > > On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> > > > > > > On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > > > > > > > When KCOV is enabled all functions get instrumented, unless the
> > > > > > > > __no_sanitize_coverage attribute is used. To prepare for
> > > > > > > > __no_sanitize_coverage being applied to __init functions, we have to
> > > > > > > > handle differences in how GCC's inline optimizations get resolved. For
> > > > > > > > x86 this means forcing several functions to be inline with
> > > > > > > > __always_inline.
> > > > > > > >
> > > > > > > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > > > > >
> > > > > > > ...
> > > > > > >
> > > > > > > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > > > > > > index bb19a2534224..b96746376e17 100644
> > > > > > > > --- a/include/linux/memblock.h
> > > > > > > > +++ b/include/linux/memblock.h
> > > > > > > > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> > > > > > > >                                       NUMA_NO_NODE);
> > > > > > > >  }
> > > > > > > >
> > > > > > > > -static inline void *memblock_alloc_from(phys_addr_t size,
> > > > > > > > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> > > > > > > >                                             phys_addr_t align,
> > > > > > > >                                             phys_addr_t min_addr)
> > > > > > >
> > > > > > > I'm curious why from all memblock_alloc* wrappers this is the only one that
> > > > > > > needs to be __always_inline?
> > > > > >
> > > > > > Thread-merge[1], adding Will Deacon, who was kind of asking the same
> > > > > > question.
> > > > > >
> > > > > > Based on what I can tell, GCC has kind of fragile inlining logic, in the
> > > > > > sense that it can change whether or not it inlines something based on
> > > > > > optimizations. It looks like the kcov instrumentation being added (or in
> > > > > > this case, removed) from a function changes the optimization results,
> > > > > > and some functions marked "inline" are _not_ inlined. In that case, we end up
> > > > > > with __init code calling a function not marked __init, and we get the
> > > > > > build warnings I'm trying to eliminate.
> > > >
> > > > Got it, thanks for the explanation!
> > > >
> > > > > > So, to Will's comment, yes, the problem is somewhat fragile (though
> > > > > > using either __always_inline or __init will deterministically solve it).
> > > > > > We've tripped over this before with GCC and the solution has usually
> > > > > > been to just use __always_inline and move on.
> > > > > >
> > > > >
> > > > > Given that 'inline' is already a macro in the kernel, could we just
> > > > > add __attribute__((__always_inline__)) to it when KCOV is enabled?
> > > >
> > > > That sounds like a more robust approach and, by the sounds of it, we
> > > > could predicate it on GCC too. That would also provide a neat place for
> > > > a comment describing the problem.
> > > >
> > > > Kees, would that work for you?
> > >
> > > That seems like an extremely large hammer for this problem, IMO. It
> > > feels like it could cause new strange corner cases. I'd much prefer the
> > > small fixes I've currently got since it keeps it focused. KCOV is
> > > already enabled for "allmodconfig", so any new instances would be found
> > > very quickly, etc. (And GCC's fragility in this regard has already been
> > > exposed to these cases -- it's just that I changed one of the
> > > combinations of __init vs inline vs instrumentation.
> > >
> > > I could give it a try, if you really prefer the big hammer approach...
> >
> > I gave it a try -- it fails spectacularly. ;) Let's stick to my small
> > fixes instead?
> >
> 
> Fair enough :-)

(but please add the helpful explanation you provided to the commit message!)

Will