[RFC PATCH] x86/boot: Get rid of linux/init.h include

[RFC PATCH] x86/boot: Get rid of linux/init.h include

[Borislav Petkov (AMD)]

Hi,

this is what I think we should do (just a first patch) to decouple the
decompressor from kernel proper headers namespace so that there's no
collisions and ugly ifdeffery when those kernel proper headers get shared.

And if we want to share things, we will use asm/shared/ to put such shared
definitions there.

Thoughts?

---

Get rid of the linux/init.h kernel proper namespace include and add
a KERNEL_PROPER_HEADER header guard to protect any future inclusions
into the decompressor.

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
 arch/x86/boot/compressed/error.h   | 2 --
 arch/x86/boot/compressed/head_32.S | 7 +++++--
 arch/x86/boot/compressed/head_64.S | 7 +++++--
 arch/x86/include/asm/init.h        | 2 +-
 include/linux/init.h               | 2 ++
 5 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/x86/boot/compressed/error.h b/arch/x86/boot/compressed/error.h
index 31f9e080d61a..938f6f1f1589 100644
--- a/arch/x86/boot/compressed/error.h
+++ b/arch/x86/boot/compressed/error.h
@@ -2,8 +2,6 @@
 #ifndef BOOT_COMPRESSED_ERROR_H
 #define BOOT_COMPRESSED_ERROR_H
 
-#include <linux/compiler.h>
-
 void warn(const char *m);
 void error(char *m) __noreturn;
 void panic(const char *fmt, ...) __noreturn __cold;
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 1cfe9802a42f..6d7728582215 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -24,7 +24,6 @@
  */
 	.text
 
-#include <linux/init.h>
 #include <linux/linkage.h>
 #include <asm/segment.h>
 #include <asm/page_types.h>
@@ -32,6 +31,10 @@
 #include <asm/asm-offsets.h>
 #include <asm/bootparam.h>
 
+#ifdef KERNEL_PROPER_HEADER
+#error Do not include kernel proper namespace headers
+#endif
+
 /*
  * These symbols needed to be marked as .hidden to prevent the BFD linker from
  * generating R_386_32 (rather than R_386_RELATIVE) relocations for them when
@@ -42,7 +45,7 @@
 	.hidden _ebss
 	.hidden _end
 
-	__HEAD
+	.section ".head.text","ax"
 SYM_FUNC_START(startup_32)
 	cld
 	cli
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 1dcb794c5479..75ea0c8e4116 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -25,7 +25,6 @@
 	.code32
 	.text
 
-#include <linux/init.h>
 #include <linux/linkage.h>
 #include <asm/segment.h>
 #include <asm/boot.h>
@@ -37,6 +36,10 @@
 #include <asm/trapnr.h>
 #include "pgtable.h"
 
+#ifdef KERNEL_PROPER_HEADER
+#error Do not include kernel proper namespace headers
+#endif
+
 /*
  * Fix alignment at 16 bytes. Following CONFIG_FUNCTION_ALIGNMENT will result
  * in assembly errors due to trying to move .org backward due to the excessive
@@ -52,7 +55,7 @@
 	.hidden _ebss
 	.hidden _end
 
-	__HEAD
+	.section ".head.text","ax"
 
 /*
  * This macro gives the relative virtual address of X, i.e. the offset of X
diff --git a/arch/x86/include/asm/init.h b/arch/x86/include/asm/init.h
index 14d72727d7ee..6c47c84a3731 100644
--- a/arch/x86/include/asm/init.h
+++ b/arch/x86/include/asm/init.h
@@ -2,7 +2,7 @@
 #ifndef _ASM_X86_INIT_H
 #define _ASM_X86_INIT_H
 
-#define __head	__section(".head.text")
+#define __head __section(".head.text")
 
 struct x86_mapping_info {
 	void *(*alloc_pgt_page)(void *); /* allocate buf for page table */
diff --git a/include/linux/init.h b/include/linux/init.h
index ee1309473bc6..21e636abf3c4 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -2,6 +2,8 @@
 #ifndef _LINUX_INIT_H
 #define _LINUX_INIT_H
 
+#define KERNEL_PROPER_HEADER
+
 #include <linux/build_bug.h>
 #include <linux/compiler.h>
 #include <linux/stringify.h>
-- 
2.43.0


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [RFC PATCH] x86/boot: Get rid of linux/init.h include

[Ingo Molnar]

* Borislav Petkov (AMD) <bp@alien8.de> wrote:

> 
> this is what I think we should do (just a first patch) to decouple the
> decompressor from kernel proper headers namespace so that there's no
> collisions and ugly ifdeffery when those kernel proper headers get shared.
> 
> And if we want to share things, we will use asm/shared/ to put such shared
> definitions there.
> 
> Thoughts?

Sounds good.


> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@ -24,7 +24,6 @@
>   */
>  	.text
>  
> -#include <linux/init.h>
>  #include <linux/linkage.h>
>  #include <asm/segment.h>
>  #include <asm/page_types.h>
> @@ -32,6 +31,10 @@
>  #include <asm/asm-offsets.h>
>  #include <asm/bootparam.h>
>  
> +#ifdef KERNEL_PROPER_HEADER
> +#error Do not include kernel proper namespace headers
> +#endif

The canonical solution in such cases is to use the existing header 
guard, ie:

 #ifdef _LINUX_INIT_H
 # error Do not include kernel proper namespace headers
 #endif

Then we can skip defining KERNEL_PROPER_HEADER as well, and this change 
will be purely to x86 code.

Thanks,

	Ingo

Re: [RFC PATCH] x86/boot: Get rid of linux/init.h include

[Borislav Petkov]

On Fri, Nov 22, 2024 at 05:55:52PM +0100, Ingo Molnar wrote:
> > --- a/arch/x86/boot/compressed/head_32.S
> > +++ b/arch/x86/boot/compressed/head_32.S
> > @@ -24,7 +24,6 @@
> >   */
> >  	.text
> >  
> > -#include <linux/init.h>
> >  #include <linux/linkage.h>
> >  #include <asm/segment.h>
> >  #include <asm/page_types.h>
> > @@ -32,6 +31,10 @@
> >  #include <asm/asm-offsets.h>
> >  #include <asm/bootparam.h>
> >  
> > +#ifdef KERNEL_PROPER_HEADER
> > +#error Do not include kernel proper namespace headers
> > +#endif
> 
> The canonical solution in such cases is to use the existing header 
> guard, ie:
> 
>  #ifdef _LINUX_INIT_H
>  # error Do not include kernel proper namespace headers
>  #endif
> 
> Then we can skip defining KERNEL_PROPER_HEADER as well, and this change 
> will be purely to x86 code.

Yap, I know, thought about it.

However, if we have to protect against every header, then we will have to do
a big

if defined...

which doesn't really work.

For the above example:

#if defined(_LINUX_INIT_H) || defined(_LINUX_LINKAGE_H)

and that would protect against the two headers which are included here.

If someone includes another one, it won't fire.

So we need a generic way to identify a kernel proper header. Either with
a define like KERNEL_PROPER_HEADER or some other magic (I don't know if there
is something like that), perhaps some preprocessor hackery which can figure
out whether some of the include paths have a include/linux/ in them and then
error out if so...

Maybe I should talk to toolchain folks...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [RFC PATCH] x86/boot: Get rid of linux/init.h include

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Fri, Nov 22, 2024 at 05:55:52PM +0100, Ingo Molnar wrote:
> > > --- a/arch/x86/boot/compressed/head_32.S
> > > +++ b/arch/x86/boot/compressed/head_32.S
> > > @@ -24,7 +24,6 @@
> > >   */
> > >  	.text
> > >  
> > > -#include <linux/init.h>
> > >  #include <linux/linkage.h>
> > >  #include <asm/segment.h>
> > >  #include <asm/page_types.h>
> > > @@ -32,6 +31,10 @@
> > >  #include <asm/asm-offsets.h>
> > >  #include <asm/bootparam.h>
> > >  
> > > +#ifdef KERNEL_PROPER_HEADER
> > > +#error Do not include kernel proper namespace headers
> > > +#endif
> > 
> > The canonical solution in such cases is to use the existing header 
> > guard, ie:
> > 
> >  #ifdef _LINUX_INIT_H
> >  # error Do not include kernel proper namespace headers
> >  #endif
> > 
> > Then we can skip defining KERNEL_PROPER_HEADER as well, and this change 
> > will be purely to x86 code.
> 
> Yap, I know, thought about it.
> 
> However, if we have to protect against every header, then we will have to do
> a big
> 
> if defined...
> 
> which doesn't really work.
> 
> For the above example:
> 
> #if defined(_LINUX_INIT_H) || defined(_LINUX_LINKAGE_H)
> 
> and that would protect against the two headers which are included here.
> 
> If someone includes another one, it won't fire.

And if someone doesn't add the ugly KERNEL_PROPER_HEADER defines to a 
new header that somehow gets included into the decompressor build 
virally, it won't fire either. I think it's better to concentrate the 
uglies in the 'weird' code, ie. the decompressor.

Also, what's the root problem being solved? The changelog says:

   > no collisions and ugly ifdeffery when those kernel proper headers 
   > get shared.

But that's pretty vague - is there some recent build regression this is 
responding to? Which kernel headers collided with which headers used by 
the decompressor build?

Thanks,

	Ingo

Re: [RFC PATCH] x86/boot: Get rid of linux/init.h include

[Borislav Petkov]

On Mon, Nov 25, 2024 at 09:24:28AM +0100, Ingo Molnar wrote:
> And if someone doesn't add the ugly KERNEL_PROPER_HEADER defines to a 
> new header that somehow gets included into the decompressor build 
> virally, it won't fire either. I think it's better to concentrate the 
> uglies in the 'weird' code, ie. the decompressor.

Yes, I'd need to think of something slicker...
 
> Also, what's the root problem being solved? The changelog says:
> 
>    > no collisions and ugly ifdeffery when those kernel proper headers 
>    > get shared.
> 
> But that's pretty vague - is there some recent build regression this is 
> responding to? Which kernel headers collided with which headers used by 
> the decompressor build?

The sharing of headers has always been a PITA. Because the decompressor is
different from kernel proper, the moment you start including kernel proper
headers for functionality, you need to exempt or add ifdeffery or do some
other weird dance to be able to share those headers.

Things like below are only some examples.

So I'd like to separate the two namespaces and only share common functionality
through asm/shared/ and avoid all that ugly ifdeffery and workarounds we're
doing. Because each time we have to touch the decompressor - and we get to
touch it a lot with the confidential computing stuff recently - it is like
a house of cards.

I hope that makes sense.

/* Use the static base for this part of the boot process */
#undef __PAGE_OFFSET
#define __PAGE_OFFSET __PAGE_OFFSET_BASE
#include "../../mm/ident_map.c"

or 

#define _SETUP
#include <asm/setup.h>	/* For COMMAND_LINE_SIZE */
#undef _SETUP

/* No MITIGATION_PAGE_TABLE_ISOLATION support needed either: */
#undef CONFIG_MITIGATION_PAGE_TABLE_ISOLATION

or

#define KASLR_COMPRESSED_BOOT
#include "../../lib/kaslr.c"

or

#ifdef CONFIG_X86_5LEVEL
#ifdef USE_EARLY_PGTABLE_L5
/*
 * cpu_feature_enabled() is not available in early boot code.
 * Use variable instead.
 */
static inline bool pgtable_l5_enabled(void)
{
	return __pgtable_l5_enabled;
}



-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [RFC PATCH] x86/boot: Get rid of linux/init.h include

[Brian Gerst]

On Mon, Nov 25, 2024 at 6:08 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Nov 25, 2024 at 09:24:28AM +0100, Ingo Molnar wrote:
> > And if someone doesn't add the ugly KERNEL_PROPER_HEADER defines to a
> > new header that somehow gets included into the decompressor build
> > virally, it won't fire either. I think it's better to concentrate the
> > uglies in the 'weird' code, ie. the decompressor.
>
> Yes, I'd need to think of something slicker...
>
> > Also, what's the root problem being solved? The changelog says:
> >
> >    > no collisions and ugly ifdeffery when those kernel proper headers
> >    > get shared.
> >
> > But that's pretty vague - is there some recent build regression this is
> > responding to? Which kernel headers collided with which headers used by
> > the decompressor build?
>
> The sharing of headers has always been a PITA. Because the decompressor is
> different from kernel proper, the moment you start including kernel proper
> headers for functionality, you need to exempt or add ifdeffery or do some
> other weird dance to be able to share those headers.
>
> Things like below are only some examples.
>
> So I'd like to separate the two namespaces and only share common functionality
> through asm/shared/ and avoid all that ugly ifdeffery and workarounds we're
> doing. Because each time we have to touch the decompressor - and we get to
> touch it a lot with the confidential computing stuff recently - it is like
> a house of cards.
>
> I hope that makes sense.

How about removing the kernel headers that you don't want from the
include path?  This is a part of a broader issue where different parts
of the kernel need different compiler flags (main kernel, VDSO, boot,
etc.) and the current makefile structure doesn't handle that very
well.


Brian Gerst

Re: [RFC PATCH] x86/boot: Get rid of linux/init.h include

[Borislav Petkov]

On Mon, Nov 25, 2024 at 11:57:37AM -0500, Brian Gerst wrote:
> How about removing the kernel headers that you don't want from the
> include path?  This is a part of a broader issue where different parts
> of the kernel need different compiler flags (main kernel, VDSO, boot,
> etc.) and the current makefile structure doesn't handle that very
> well.

Right, the idea is to remove *all* include/linux/ headers from the
decompressor and have the build break if someone includes new ones, forcing
her/him to properly split such header and use asm/shared for the common stuff.

There are examples in asm/shared/ for that already.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [RFC PATCH] x86/boot: Get rid of linux/init.h include

[Brian Gerst]

On Mon, Nov 25, 2024 at 12:09 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Nov 25, 2024 at 11:57:37AM -0500, Brian Gerst wrote:
> > How about removing the kernel headers that you don't want from the
> > include path?  This is a part of a broader issue where different parts
> > of the kernel need different compiler flags (main kernel, VDSO, boot,
> > etc.) and the current makefile structure doesn't handle that very
> > well.
>
> Right, the idea is to remove *all* include/linux/ headers from the
> decompressor and have the build break if someone includes new ones, forcing
> her/him to properly split such header and use asm/shared for the common stuff.
>
> There are examples in asm/shared/ for that already.

To clarify, what I meant was to remove the -Iinclude/linux flag from
the compiler command line, instead of messing around with ifdefs.


Brian Gerst

Re: [RFC PATCH] x86/boot: Get rid of linux/init.h include

[Borislav Petkov]

On Mon, Nov 25, 2024 at 12:31:05PM -0500, Brian Gerst wrote:
> To clarify, what I meant was to remove the -Iinclude/linux flag from
> the compiler command line, instead of messing around with ifdefs.

Ha, that's a cool idea, thanks!

That will be the last patch in this series when I get to it eventually.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette