[PATCH] x86: Fix boot protocol KEEP_SEGMENTS check.

[PATCH] x86: Fix boot protocol KEEP_SEGMENTS check.

[Eric W. Biederman]

The kernel only ever supports 1 version of the boot protocol
so there is no need to check the boot protocol revision to
see if a feature is supported.

Both x86 and x86_64 support the same boot protocol so we need
to implement the KEEP_SEGMENTS on x86_64 as well.  It isn't
just paravirt bootloaders that could use this functionality.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Vivek Goyal <vgoyal@in.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Zachary Amsden <zach@vmware.com>
Cc: Andi Kleen <ak@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/boot/compressed/head_32.S |   12 ++++--------
 arch/x86/boot/compressed/head_64.S |    7 +++++++
 arch/x86/kernel/asm-offsets_64.c   |   10 ++++++++++
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index a0ae2e7..036e635 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -33,24 +33,20 @@
 	.globl startup_32
 
 startup_32:
-	/* check to see if KEEP_SEGMENTS flag is meaningful */
-	cmpw $0x207, BP_version(%esi)
-	jb 1f
-
+	cld
 	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
 	 * us to not reload segments */
 	testb $(1<<6), BP_loadflags(%esi)
-	jnz 2f
+	jnz 1f
 
-1:	cli
+	cli
 	movl $(__BOOT_DS),%eax
 	movl %eax,%ds
 	movl %eax,%es
 	movl %eax,%fs
 	movl %eax,%gs
 	movl %eax,%ss
-
-2:	cld
+1:
 
 /* Calculate the delta between where we were compiled to run
  * at and where we were actually loaded at.  This can only be done
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 4946764..1ccb38a 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -29,6 +29,7 @@
 #include <asm/pgtable.h>
 #include <asm/page.h>
 #include <asm/msr.h>
+#include <asm/asm-offsets.h>
 
 .section ".text.head"
 	.code32
@@ -36,11 +37,17 @@
 
 startup_32:
 	cld
+	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
+	 * us to not reload segments */
+	testb $(1<<6), BP_loadflags(%esi)
+	jnz 1f
+
 	cli
 	movl	$(__KERNEL_DS), %eax
 	movl	%eax, %ds
 	movl	%eax, %es
 	movl	%eax, %ss
+1:
 
 /* Calculate the delta between where we were compiled to run
  * at and where we were actually loaded at.  This can only be done
diff --git a/arch/x86/kernel/asm-offsets_64.c b/arch/x86/kernel/asm-offsets_64.c
index 7e50bda..d1b6ed9 100644
--- a/arch/x86/kernel/asm-offsets_64.c
+++ b/arch/x86/kernel/asm-offsets_64.c
@@ -15,12 +15,16 @@
 #include <asm/segment.h>
 #include <asm/thread_info.h>
 #include <asm/ia32.h>
+#include <asm/bootparam.h>
 
 #define DEFINE(sym, val) \
         asm volatile("\n->" #sym " %0 " #val : : "i" (val))
 
 #define BLANK() asm volatile("\n->" : : )
 
+#define OFFSET(sym, str, mem) \
+	DEFINE(sym, offsetof(struct str, mem))
+
 #define __NO_STUBS 1
 #undef __SYSCALL
 #undef _ASM_X86_64_UNISTD_H_
@@ -109,5 +113,11 @@ int main(void)
 	DEFINE(crypto_tfm_ctx_offset, offsetof(struct crypto_tfm, __crt_ctx));
 	BLANK();
 	DEFINE(__NR_syscall_max, sizeof(syscalls) - 1);
+
+	BLANK();
+	OFFSET(BP_scratch, boot_params, scratch);
+	OFFSET(BP_loadflags, boot_params, hdr.loadflags);
+	OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
+	OFFSET(BP_version, boot_params, hdr.version);
 	return 0;
 }
-- 
1.5.3.rc6.17.g1911

Re: [PATCH] x86: Fix boot protocol KEEP_SEGMENTS check.

[H. Peter Anvin]

Eric W. Biederman wrote:
> 
> Both x86 and x86_64 support the same boot protocol so we need
> to implement the KEEP_SEGMENTS on x86_64 as well.  It isn't
> just paravirt bootloaders that could use this functionality.
> 

Out of curiousity, what other users do you see?

	-hpa

Re: [PATCH] x86: Fix boot protocol KEEP_SEGMENTS check.

[Eric W. Biederman]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Eric W. Biederman wrote:
>>
>> Both x86 and x86_64 support the same boot protocol so we need
>> to implement the KEEP_SEGMENTS on x86_64 as well.  It isn't
>> just paravirt bootloaders that could use this functionality.
>>
>
> Out of curiousity, what other users do you see?

Simply that is easier to provide segments that are not fit to be
reloaded before the kernel sets up it's own GDT.  Then to ensure you
stick a GDT someplace that won't get stomped.

Especially before we froze which segments numbers the kernel
would load into the boot protocol.  I would have used this in
places where I was doing 32bit bootloader work as it is just easier to
support.

Eric

Re: [PATCH] x86: Fix boot protocol KEEP_SEGMENTS check.

[Jeremy Fitzhardinge]

Eric W. Biederman wrote:
> The kernel only ever supports 1 version of the boot protocol
> so there is no need to check the boot protocol revision to
> see if a feature is supported.
>
> Both x86 and x86_64 support the same boot protocol so we need
> to implement the KEEP_SEGMENTS on x86_64 as well.  It isn't
> just paravirt bootloaders that could use this functionality.
>   

Ack.  I was planning on sending this patch out today...

    J
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Jeremy Fitzhardinge <jeremy@xensource.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Vivek Goyal <vgoyal@in.ibm.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Zachary Amsden <zach@vmware.com>
> Cc: Andi Kleen <ak@suse.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/boot/compressed/head_32.S |   12 ++++--------
>  arch/x86/boot/compressed/head_64.S |    7 +++++++
>  arch/x86/kernel/asm-offsets_64.c   |   10 ++++++++++
>  3 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
> index a0ae2e7..036e635 100644
> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@ -33,24 +33,20 @@
>  	.globl startup_32
>  
>  startup_32:
> -	/* check to see if KEEP_SEGMENTS flag is meaningful */
> -	cmpw $0x207, BP_version(%esi)
> -	jb 1f
> -
> +	cld
>  	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
>  	 * us to not reload segments */
>  	testb $(1<<6), BP_loadflags(%esi)
> -	jnz 2f
> +	jnz 1f
>  
> -1:	cli
> +	cli
>  	movl $(__BOOT_DS),%eax
>  	movl %eax,%ds
>  	movl %eax,%es
>  	movl %eax,%fs
>  	movl %eax,%gs
>  	movl %eax,%ss
> -
> -2:	cld
> +1:
>  
>  /* Calculate the delta between where we were compiled to run
>   * at and where we were actually loaded at.  This can only be done
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 4946764..1ccb38a 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -29,6 +29,7 @@
>  #include <asm/pgtable.h>
>  #include <asm/page.h>
>  #include <asm/msr.h>
> +#include <asm/asm-offsets.h>
>  
>  .section ".text.head"
>  	.code32
> @@ -36,11 +37,17 @@
>  
>  startup_32:
>  	cld
> +	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
> +	 * us to not reload segments */
> +	testb $(1<<6), BP_loadflags(%esi)
> +	jnz 1f
> +
>  	cli
>  	movl	$(__KERNEL_DS), %eax
>  	movl	%eax, %ds
>  	movl	%eax, %es
>  	movl	%eax, %ss
> +1:
>  
>  /* Calculate the delta between where we were compiled to run
>   * at and where we were actually loaded at.  This can only be done
> diff --git a/arch/x86/kernel/asm-offsets_64.c b/arch/x86/kernel/asm-offsets_64.c
> index 7e50bda..d1b6ed9 100644
> --- a/arch/x86/kernel/asm-offsets_64.c
> +++ b/arch/x86/kernel/asm-offsets_64.c
> @@ -15,12 +15,16 @@
>  #include <asm/segment.h>
>  #include <asm/thread_info.h>
>  #include <asm/ia32.h>
> +#include <asm/bootparam.h>
>  
>  #define DEFINE(sym, val) \
>          asm volatile("\n->" #sym " %0 " #val : : "i" (val))
>  
>  #define BLANK() asm volatile("\n->" : : )
>  
> +#define OFFSET(sym, str, mem) \
> +	DEFINE(sym, offsetof(struct str, mem))
> +
>  #define __NO_STUBS 1
>  #undef __SYSCALL
>  #undef _ASM_X86_64_UNISTD_H_
> @@ -109,5 +113,11 @@ int main(void)
>  	DEFINE(crypto_tfm_ctx_offset, offsetof(struct crypto_tfm, __crt_ctx));
>  	BLANK();
>  	DEFINE(__NR_syscall_max, sizeof(syscalls) - 1);
> +
> +	BLANK();
> +	OFFSET(BP_scratch, boot_params, scratch);
> +	OFFSET(BP_loadflags, boot_params, hdr.loadflags);
> +	OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
> +	OFFSET(BP_version, boot_params, hdr.version);
>  	return 0;
>  }
>   

Re: [PATCH] x86: Fix boot protocol KEEP_SEGMENTS check.

[H. Peter Anvin]

Eric W. Biederman wrote:
> The kernel only ever supports 1 version of the boot protocol
> so there is no need to check the boot protocol revision to
> see if a feature is supported.
> 
> Both x86 and x86_64 support the same boot protocol so we need
> to implement the KEEP_SEGMENTS on x86_64 as well.  It isn't
> just paravirt bootloaders that could use this functionality.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Jeremy Fitzhardinge <jeremy@xensource.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Vivek Goyal <vgoyal@in.ibm.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Zachary Amsden <zach@vmware.com>
> Cc: Andi Kleen <ak@suse.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@elte.hu>

Btw, Acked-by: H. Peter Anvin <hpa@zytor.com>

Re: [PATCH] x86: Fix boot protocol KEEP_SEGMENTS check.

[Rusty Russell]

On Saturday 27 October 2007 03:29:04 Eric W. Biederman wrote:
> The kernel only ever supports 1 version of the boot protocol
> so there is no need to check the boot protocol revision to
> see if a feature is supported.

OK, I'm completely confused, so this is probably a v. dumb question.  Doesn't 
this boot header come from the boot loader?

Rusty.

Re: [PATCH] x86: Fix boot protocol KEEP_SEGMENTS check.

[Jeremy Fitzhardinge]

Rusty Russell wrote:
> On Saturday 27 October 2007 03:29:04 Eric W. Biederman wrote:
>   
>> The kernel only ever supports 1 version of the boot protocol
>> so there is no need to check the boot protocol revision to
>> see if a feature is supported.
>>     
>
> OK, I'm completely confused, so this is probably a v. dumb question.  Doesn't 
> this boot header come from the boot loader?

No, other way around.  It the kernel telling the bootloader "I
understand boot protocol X", and the bootloader makes sure it doesn't
fill out more than X allows.  If the bootloader supports <X, then the
kernel's boot_params structure is pre-constructed with reasonable
default values.

    J