[PATCH] fix i486 boot failure due to stale %ds

[PATCH] fix i486 boot failure due to stale %ds

[Mikael Pettersson]

On Mon, 29 Oct 2007 01:05:58 +0100 (MET), Mikael Pettersson wrote:
>My old 486 fails to boot with the 2.6.24-rc1 kernel.
>Grub loads it, 4 lines of text appear but not the kernel's
>"Linux version <blah> greet", and the machine reboots.
>Double-checked with a serial console: nothing appears
>before it reboots.
>
>All 2.6 kernels up to 2.6.23 worked fine on this machine.

Problem identified and solved.

The last good kernel was 2.6.23-git17. The change in 2.6.23-git18
that broke my 486 was a24e785111a32ccb7cebafd24b1b1cb474ea8e5d,
which included the following:

>--- a/arch/x86/boot/compressed/head_32.S
>+++ b/arch/x86/boot/compressed/head_32.S
>@@ -27,13 +27,22 @@
> #include <asm/segment.h>
> #include <asm/page.h>
> #include <asm/boot.h>
>+#include <asm/asm-offsets.h>
> 
> .section ".text.head","ax",@progbits
>        .globl startup_32
> 
> startup_32:
>-       cld
>-       cli
>+       /* check to see if KEEP_SEGMENTS flag is meaningful */
>+       cmpw $0x207, BP_version(%esi)
>+       jb 1f
>+
>+       /* test KEEP_SEGMENTS flag to see if the bootloader is asking
>+        * us to not reload segments */
>+       testb $(1<<6), BP_loadflags(%esi)
>+       jnz 2f
>+
>+1:     cli
>        movl $(__BOOT_DS),%eax
>        movl %eax,%ds
>        movl %eax,%es

That is, it added loads from %ds:offset(%esi) before the code
that sanitises %ds et al. Commenting out these loads and tests
made the kernel boot again.

Adding debugging code to head_32.S and decompress_kernel(),
I found that startup_32 actually is invoked with a good %ds
(== 0x18 == __BOOT_DS) and a flat 4GB data segment in the
corresponding GDT entry.

Since data accesses work after %ds is (re)loaded with __BOOT_DS,
it's clear that on this machine, the hidden registers behind %ds
are stale on entry to startup_32. After adding a patch to reload
%ds with itself (not __BOOT_DS) at startup_32 the kernel doesn't
crash and reboot any more on this machine.

The machine in question is a ca 1993 vintage Siemens 486 with
a Quadtel S3 / Phoenix BIOS from 1994, booting via grub-0.95-13
from Fedora Core 4.

Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
---
 arch/x86/boot/compressed/head_32.S |    5 +++++
 1 files changed, 5 insertions(+)

--- linux-2.6.24-rc1-git13/arch/x86/boot/compressed/head_32.S.~1~	2007-11-04 16:34:33.000000000 +0100
+++ linux-2.6.24-rc1-git13/arch/x86/boot/compressed/head_32.S	2007-11-04 16:44:15.000000000 +0100
@@ -33,6 +33,11 @@
 	.globl startup_32
 
 startup_32:
+	/* workaround for BIOSen or boot loaders that don't reload %ds
+	   after changing the GDT (insane but unfortunately true) */
+	movl %ds,%eax
+	movl %eax,%ds
+
 	cld
 	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
 	 * us to not reload segments */

Re: [PATCH] fix i486 boot failure due to stale %ds

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]

Mikael Pettersson wrote:
> 
> The machine in question is a ca 1993 vintage Siemens 486 with
> a Quadtel S3 / Phoenix BIOS from 1994, booting via grub-0.95-13
> from Fedora Core 4.
> 
> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
> ---
>  arch/x86/boot/compressed/head_32.S |    5 +++++
>  1 files changed, 5 insertions(+)
> 
> --- linux-2.6.24-rc1-git13/arch/x86/boot/compressed/head_32.S.~1~	2007-11-04 16:34:33.000000000 +0100
> +++ linux-2.6.24-rc1-git13/arch/x86/boot/compressed/head_32.S	2007-11-04 16:44:15.000000000 +0100
> @@ -33,6 +33,11 @@
>  	.globl startup_32
>  
>  startup_32:
> +	/* workaround for BIOSen or boot loaders that don't reload %ds
> +	   after changing the GDT (insane but unfortunately true) */
> +	movl %ds,%eax
> +	movl %eax,%ds
> +
>  	cld
>  	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
>  	 * us to not reload segments */

Double bogus flag here.

It's not an issue of the BIOS or the boot loader (in your case, Grub) 
unless you have a boot loader that does 32-bit entry (e.g. kexec or ELILO.)

Second, the "workaround" you have here effectively disables the meaning 
of the KEEP_SEGMENTS flag, so it's unacceptable.

If one couldn't rely on %ds, then we could load the loadflags with a 
%cs: override, but there is something much more bizarre going on here. 
Since you're doing a normal 16-bit entry (unless your Grub is 
configured/patched to do something extremely weird, not that that would 
be anything out of the ordinary for Grub) the code that should have been 
executed immediately before this point is this code from 
arch/x86/boot/pm_jump.S:

protected_mode_jump:
         xorl    %ebx, %ebx              # Flag to indicate this is a boot
         movl    %edx, %esi              # Pointer to boot_params table
         movl    %eax, 2f                # Patch ljmpl instruction
         jmp     1f                      # Short jump to flush 
instruction q.

1:
         movw    $__BOOT_DS, %cx

         movl    %cr0, %edx
         orb     $1, %dl                 # Protected mode (PE) bit
         movl    %edx, %cr0

         movw    %cx, %ds
         movw    %cx, %es
         movw    %cx, %fs
         movw    %cx, %gs
         movw    %cx, %ss

         # Jump to the 32-bit entrypoint
         .byte   0x66, 0xea              # ljmpl opcode
2:      .long   0                       # offset
         .word   __BOOT_CS               # segment

         .size   protected_mode_jump, .-protected_mode_jump


As you can see, all the segments should have been properly set up.  I'm 
somewhat wondering if you have found one lone CPU revision in the entire 
x86 menagerie which doesn't properly serialize on mov to %cr0, which 
would casue the mov to %ds immediately after to be misexecuted.  If 
that's the case, I believe I owe Eric Biederman a drink of choice.

Could you send me your /proc/cpuinfo?

Also, I would be very interested if you could try out this patch:


[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 902 bytes --]

diff --git a/arch/x86/boot/pmjump.S b/arch/x86/boot/pmjump.S
index 2e55923..97cf407 100644
--- a/arch/x86/boot/pmjump.S
+++ b/arch/x86/boot/pmjump.S
@@ -30,7 +30,6 @@
 protected_mode_jump:
 	xorl	%ebx, %ebx		# Flag to indicate this is a boot
 	movl	%edx, %esi		# Pointer to boot_params table
-	movl	%eax, 2f		# Patch ljmpl instruction
 	jmp	1f			# Short jump to flush instruction q.
 
 1:
@@ -39,16 +38,17 @@ protected_mode_jump:
 	movl	%cr0, %edx
 	orb	$1, %dl			# Protected mode (PE) bit
 	movl	%edx, %cr0
+	ljmpw	__BOOT_CS, 2f
 
-	movw	%cx, %ds
-	movw	%cx, %es
-	movw	%cx, %fs
-	movw	%cx, %gs
-	movw	%cx, %ss
+	.code32
+2:
+	movl	%ecx, %ds
+	movl	%ecx, %es
+	movl	%ecx, %fs
+	movl	%ecx, %gs
+	movl	%ecx, %ss
 
 	# Jump to the 32-bit entrypoint
-	.byte	0x66, 0xea		# ljmpl opcode
-2:	.long	0			# offset
-	.word	__BOOT_CS		# segment
-
+	jmpl	%eax
+	
 	.size	protected_mode_jump, .-protected_mode_jump

Re: [PATCH] fix i486 boot failure due to stale %ds

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 55 bytes --]

Urk, -ENOTAWAKEYET.  Try *THIS* patch, please.

	-hpa


[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 1222 bytes --]

diff --git a/arch/x86/boot/pmjump.S b/arch/x86/boot/pmjump.S
index 2e55923..17e6dec 100644
--- a/arch/x86/boot/pmjump.S
+++ b/arch/x86/boot/pmjump.S
@@ -28,27 +28,37 @@
  * void protected_mode_jump(u32 entrypoint, u32 bootparams);
  */
 protected_mode_jump:
-	xorl	%ebx, %ebx		# Flag to indicate this is a boot
 	movl	%edx, %esi		# Pointer to boot_params table
-	movl	%eax, 2f		# Patch ljmpl instruction
+
+	xorl	%edx, %edx
+	movw	%cs, %dx
+	shll	$4, %edx		# Patch ljmpl instruction
+	addl	%edx, 2f
 	jmp	1f			# Short jump to flush instruction q.
 
 1:
 	movw	$__BOOT_DS, %cx
+	xorl	%ebx, %ebx		# Per protocol
+	xorl	%ebp, %ebp		# Per protocol
+	xorl	%edi, %edi		# Per protocol
 
 	movl	%cr0, %edx
 	orb	$1, %dl			# Protected mode (PE) bit
 	movl	%edx, %cr0
+	
+	.byte	0x66, 0xea		# ljmpl opcode
+2:	.long	3f			# Offset
+	.word	__BOOT_CS		# Segment
 
-	movw	%cx, %ds
-	movw	%cx, %es
-	movw	%cx, %fs
-	movw	%cx, %gs
-	movw	%cx, %ss
+	.code32
+3:
+	movl	%ecx, %ds
+	movl	%ecx, %es
+	movl	%ecx, %fs
+	movl	%ecx, %gs
+	movl	%ecx, %ss
 
 	# Jump to the 32-bit entrypoint
-	.byte	0x66, 0xea		# ljmpl opcode
-2:	.long	0			# offset
-	.word	__BOOT_CS		# segment
-
+	jmpl	*%eax
+	
 	.size	protected_mode_jump, .-protected_mode_jump

Re: [PATCH] fix i486 boot failure due to stale %ds

[Mikael Pettersson]

On Sun, 04 Nov 2007 10:29:34 -0800, H. Peter Anvin wrote:
>Could you send me your /proc/cpuinfo?

Sure. It's a 100Mhz Intel 486 DX4:

processor	: 0
vendor_id	: GenuineIntel
cpu family	: 4
model		: 8
model name	: 486 DX/4
stepping	: 0
cache size	: 0 KB
fdiv_bug	: no
hlt_bug		: no
f00f_bug	: no
coma_bug	: no
fpu		: yes
fpu_exception	: yes
cpuid level	: 1
wp		: yes
flags		: fpu vme
bogomips	: 49.66
clflush size	: 32

>Also, I would be very interested if you could try out this patch:
...
>Urk, -ENOTAWAKEYET.  Try *THIS* patch, please.

First patch didn't build. Second patch builds and boots Ok.

So this means the 486 DX4 has a buggy mov to %cr0?

/Mikael

Re: [PATCH] fix i486 boot failure due to stale %ds

[H. Peter Anvin]

Mikael Pettersson wrote:
> 
> First patch didn't build. Second patch builds and boots Ok.
> 
> So this means the 486 DX4 has a buggy mov to %cr0?
> 

Apparently.

	-hpa

Re: [PATCH] fix i486 boot failure due to stale %ds

[Mikael Pettersson]

On Sun, 04 Nov 2007 11:41:58 -0800, H. Peter Anvin wrote:
> Mikael Pettersson wrote:
> > 
> > First patch didn't build. Second patch builds and boots Ok.
> > 
> > So this means the 486 DX4 has a buggy mov to %cr0?
> > 
> 
> Apparently.

Maybe not. I had a look in Intel's SDM Vol3, and the
section "switching to protected mode" specifies that
a move to %cr0 that sets PE should immediately be
followed by a far jmp or call. They write that "random
failures can occur if other instructions exist between
[the move to %cr0] and [the far jmp/call]". The current
version of pmjump.S does exactly that: it executes
a bunch of moves to segment registers in that window.

(Section 9.9.1 in the Sept. 2005 revision I have in
front of me.)

Similarly, section "serializing instructions" writes
that a move to %cr0 that enables or disables paging
should be followed by a jump. They write that this isn't
required in P4 or P6 family processors, but is required
for compatibility with other ia32 processors. Reading
between the lines, they imply that older ia32 processors
don't treat %cr0 writes as completely serializing.

(Section 7.4 in the Sept. 2005 revision.)

/Mikael

Re: [PATCH] fix i486 boot failure due to stale %ds

[H. Peter Anvin]

Mikael Pettersson wrote:
> 
> Maybe not. I had a look in Intel's SDM Vol3, and the
> section "switching to protected mode" specifies that
> a move to %cr0 that sets PE should immediately be
> followed by a far jmp or call. They write that "random
> failures can occur if other instructions exist between
> [the move to %cr0] and [the far jmp/call]". The current
> version of pmjump.S does exactly that: it executes
> a bunch of moves to segment registers in that window.
> 
> (Section 9.9.1 in the Sept. 2005 revision I have in
> front of me.)
> 
> Similarly, section "serializing instructions" writes
> that a move to %cr0 that enables or disables paging
> should be followed by a jump. They write that this isn't
> required in P4 or P6 family processors, but is required
> for compatibility with other ia32 processors. Reading
> between the lines, they imply that older ia32 processors
> don't treat %cr0 writes as completely serializing.
> 
> (Section 7.4 in the Sept. 2005 revision.)
> 

The problem is that Intel has a tendency to exaggerate in their 
documentation; in particular, they tend not to remove restrictions that 
are long-since obsolete.  However, it sounds like you have actually 
found a CPU for which this restriction is motivated.

	-hpa

Re: [PATCH] fix i486 boot failure due to stale %ds

[Andi Kleen]

Mikael Pettersson <mikpe@it.uu.se> writes:

> Maybe not. I had a look in Intel's SDM Vol3, and the
> section "switching to protected mode" specifies that
> a move to %cr0 that sets PE should immediately be
> followed by a far jmp or call. They write that "random
> failures can occur if other instructions exist between
> [the move to %cr0] and [the far jmp/call]". The current
> version of pmjump.S does exactly that: it executes
> a bunch of moves to segment registers in that window.

iirc various Intel VT hypervisor implementations also have trouble
with not following this restriction. At least in the early Xen
days it was a frequent reason for boot failure.

-Andi

Re: [PATCH] fix i486 boot failure due to stale %ds

[Jeremy Fitzhardinge]

Mikael Pettersson wrote:
> On Sun, 04 Nov 2007 11:41:58 -0800, H. Peter Anvin wrote:
>   
>> Mikael Pettersson wrote:
>>     
>>> First patch didn't build. Second patch builds and boots Ok.
>>>
>>> So this means the 486 DX4 has a buggy mov to %cr0?
>>>
>>>       
>> Apparently.
>>     
>
> Maybe not. I had a look in Intel's SDM Vol3, and the
> section "switching to protected mode" specifies that
> a move to %cr0 that sets PE should immediately be
> followed by a far jmp or call.
>   

Yes, that's what the spec says.  I queried this a few months ago, but
hpa used his convincing voice and said that in practice it isn't
necessary; there are no known cpus which need this, and any that do
would cause other things to break.  But I guess now we have the
counter-example...

    J

Re: [PATCH] fix i486 boot failure due to stale %ds

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
> 
> Yes, that's what the spec says.  I queried this a few months ago, but
> hpa used his convincing voice and said that in practice it isn't
> necessary; there are no known cpus which need this, and any that do
> would cause other things to break.  But I guess now we have the
> counter-example...
> 

Yup.  I owe you a beverage :)

	-hpa

Re: [PATCH] fix i486 boot failure due to stale %ds

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
>> Maybe not. I had a look in Intel's SDM Vol3, and the
>> section "switching to protected mode" specifies that
>> a move to %cr0 that sets PE should immediately be
>> followed by a far jmp or call.
> 
> Yes, that's what the spec says.  I queried this a few months ago, but
> hpa used his convincing voice and said that in practice it isn't
> necessary; there are no known cpus which need this, and any that do
> would cause other things to break.  But I guess now we have the
> counter-example...

Joy.  Apparently the Intel documentation is actually self-inconsistent. 
  Section 9.9.1, page 9-17 does indeed have the "far jump or call" 
injunction, whereas the sample code in section 9.10.1, page 9-27, line 
180 does a near jump!

	-hpa

Re: [PATCH] fix i486 boot failure due to stale %ds

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 59 bytes --]

Mikael, can you try this patch (rev 3) on your 486?

	-hpa

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 1021 bytes --]

diff --git a/arch/x86/boot/pmjump.S b/arch/x86/boot/pmjump.S
index 2e55923..d93a0c2 100644
--- a/arch/x86/boot/pmjump.S
+++ b/arch/x86/boot/pmjump.S
@@ -28,17 +28,21 @@
  * void protected_mode_jump(u32 entrypoint, u32 bootparams);
  */
 protected_mode_jump:
-	xorl	%ebx, %ebx		# Flag to indicate this is a boot
 	movl	%edx, %esi		# Pointer to boot_params table
-	movl	%eax, 2f		# Patch ljmpl instruction
+	movl	%eax, 3f		# Patch ljmpl instruction
 	jmp	1f			# Short jump to flush instruction q.
-
 1:
+
 	movw	$__BOOT_DS, %cx
+	xorl	%ebx, %ebx		# Per protocol
+	xorl	%ebp, %ebp		# Per protocol
+	xorl	%edi, %edi		# Per protocol
 
 	movl	%cr0, %edx
 	orb	$1, %dl			# Protected mode (PE) bit
 	movl	%edx, %cr0
+	jmp	2f			# Short jump to serialize
+2:
 
 	movw	%cx, %ds
 	movw	%cx, %es
@@ -48,7 +52,7 @@ protected_mode_jump:
 
 	# Jump to the 32-bit entrypoint
 	.byte	0x66, 0xea		# ljmpl opcode
-2:	.long	0			# offset
+3:	.long	0			# offset
 	.word	__BOOT_CS		# segment
 
 	.size	protected_mode_jump, .-protected_mode_jump

Re: [PATCH] fix i486 boot failure due to stale %ds

[Linus Torvalds]

On Sun, 4 Nov 2007, H. Peter Anvin wrote:
> 
> Joy.  Apparently the Intel documentation is actually self-inconsistent.
> Section 9.9.1, page 9-17 does indeed have the "far jump or call" injunction,
> whereas the sample code in section 9.10.1, page 9-27, line 180 does a near
> jump!

See the older code. There's literally two different issues:

 - flushing the pipeline. Using a regular short jump is not only 
   sufficient, but is a good idea because the byte sequence has no 
   dependency on any modes, so it is guaranteed to flush any pipeline 
   without itself having any behavioural differences.

   This part is a no-op for later CPU's: they simply won't care, since 
   they serialize the pipeline on their own (and the have to, since a 
   correctly predicted branch no longer flushes it anyway).

   So this is purely a i386/i486 thing (and *maybe* Pentium, but 
   definitely not PPro and later)

 - setting up the proper protected mode bits in CS. This obviously changes
   the CS itself, and anything that depends on any shadow state in CS will 
   need this to happen first. In practice, very few things really depend 
   on the CS bits, of course.

Doing them back-to-back is obviously the best situation, and leaves the 
minimum footprint for any dubious undocumented behaviour. That said, while 
the whole thing with shadow segment table is strictly speaking probably 
"undocumented" behaviour, it's certainly a reality, and lots of code has 
depended on the fact that even though you haven't reloaded a segment, the 
segment continues to work across mode switches thanks to the shadow table. 

So it's not like there's any real secret to what happens until the 
longjump has been executed: we continue running with the CS shadow table 
entry being in "real mode", and the effect of that tends to be minimal. 
But in theory, the CS table entry could affect how instructions actually 
act, even though in practice I don't think it really has any real effect 
outside of the actual segment access check (and certainly did not in the 
i386/i486 timeframe).

So I'd suggest having both jumps back-to-back, but realistically, the 
first regular short jump is actually the one that is more important. 
That's the one that really matters on i386/i486 class machines, and later 
CPU's will generally do the right thing even with _neither_ jump there.

			Linus

Re: [PATCH] fix i486 boot failure due to stale %ds

[Linus Torvalds]

On Sun, 4 Nov 2007, Linus Torvalds wrote:
> 
> So I'd suggest having both jumps back-to-back, but realistically, the 
> first regular short jump is actually the one that is more important. 
> That's the one that really matters on i386/i486 class machines, and later 
> CPU's will generally do the right thing even with _neither_ jump there.

That's obviously badly phrased.

The far jump is obviously required on all CPU's in order for us to 
actually finally get to 32-bit protected mode and reload CS, but what I 
*meant* was that we certainly also know that "unreal mode" works and is 
used by various strange DOS extenders, and that not doing the far jump 
isn't really required for having a "working setup" - it's just going to be 
a rather limited mode.

So the short jump is required for the code to *work*. The long jump is 
required only to get us the 32-bit mode we *want* and out of the odd 
"half-way" state. Two different issues.

		Linus

Re: [PATCH] fix i486 boot failure due to stale %ds

[Mikael Pettersson]

On Sun, 04 Nov 2007 15:51:43 -0800, H. Peter Anvin wrote:
> Mikael, can you try this patch (rev 3) on your 486?

It works fine.

/Mikael