Make bootsector stub 16-bit-only (i386)

PATCH In review Alexander van Heukelum 6 participants 21 messages May 5, 2007 – May 9, 2007

lore

Share & tools

lore

Alexander van Heukelum

author

2007-05-05 17:00 UTC #0 lore raw

PATCH 1/1 Make bootsector stub 16-bit-only (i386)

Hi!

The x86 bzImage contains a stub to inform people that it is not possible any more to run a Linux kernel by catting it to a floppy and then booting from it. This was meant to be all 16-bit code. The first instruction, however, ended up as being coded as a 16:32-bit far jump. I assume the intention was a 16:16-bit far jump.

This patch changes only i386.

Greetings,
Alexander

Signed-off-by Alexander van Heukelum <[email protected]>

---

arch/i386/boot/bootsect.S +1 -1

index 011b7a4..ae9df0d 100644
@@ -44,7 +44,7 @@ #endif
 _start:
 
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
 
 start2:
 	movw	%cs, %ax

H. Peter Anvin

Acked-by on v1 1/1

2007-05-05 18:08 UTC #1 lore raw

Acked-by H. Peter Anvin <[email protected]>

Quoted 30 lines · Alexander van Heukelum

Hi!
The x86 bzImage contains a stub to inform people that it is not possible any more to run a Linux kernel by catting it to a floppy and then booting from it. This was meant to be all 16-bit code. The first instruction, however, ended up as being coded as a 16:32-bit far jump. I assume the intention was a 16:16-bit far jump.
This patch changes only i386.
Greetings,
Alexander
Signed-off-by Alexander van Heukelum <[email protected]>
---
arch/i386/boot/bootsect.S +1 -1
index 011b7a4..ae9df0d 100644
@@ -44,7 +44,7 @@ #endif
 _start:
 
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
 
 start2:
 	movw	%cs, %ax

Andrew Morton

2007-05-08 10:29 UTC #2 lore raw

In reply to Alexander van Heukelum

Quoted 20 lines · On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum…

The x86 bzImage contains a stub to inform people that it is not possible any more to run a Linux kernel by catting it to a floppy and then booting from it. This was meant to be all 16-bit code. The first instruction, however, ended up as being coded as a 16:32-bit far jump. I assume the intention was a 16:16-bit far jump.
This patch changes only i386.
Greetings,
Alexander
Signed-off-by Alexander van Heukelum <[email protected]>
---
arch/i386/boot/bootsect.S +0 -0
index 011b7a4..ae9df0d 100644

arch/i386/boot/bootsect.S #endif · line 47

Quoted 5 lines

arch/i386/boot/bootsect.S +1 -1

@@ -44,7 +44,7 @@ #endif
 _start:
 
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2

Sigh, another blow struck in the ongoing struggle between my Vaio and the rest of the world.

Stone-cold black-screen lockup immediately upon boot.

Stock FC5 install, config at
http://userweb.kernel.org/~akpm/config-sony.txt

H. Peter Anvin

2007-05-08 11:25 UTC #3 lore raw

Quoted 8 lines · Andrew Morton

arch/i386/boot/bootsect.S #endif · line 47
Quoted 4 lines
arch/i386/boot/bootsect.S +1 -1
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
Sigh, another blow struck in the ongoing struggle between my Vaio and the rest of the world.

Quoted 4 lines

Stone-cold black-screen lockup immediately upon boot.
Stock FC5 install, config at
http://userweb.kernel.org/~akpm/config-sony.txt

Andrew, I'm seriously starting to think there is something fundamentally wrong with that test setup.

The bootsect code in question is never executed. AT ALL. The only raison d'être for it at all is to print an error message if someone writes the kernel to a raw floppy disk. Nor does it change the alignment of the header or anything else to that effect -- the assembly code downstream has an explicit ".org" directive. For what it's worth, just to make sure I'm not crazy, I just re-tested both booting the kernel and booting the raw disk image, in simulation and on real hardware, and it doesn't change anything. I used your configuration file (yes '' | make oldconfig) minus Bluetooth (which is broken in current top of Linus) against top of tree Linus + the jmpw patch. I obviously don't have your Vaio, but I do have my own share of quirky hardware. FWIW, I netbooted the hardware using pxelinux 3.50-pre7 as the bootloader.[*]

I'm not writing this to give you a hard time, far from it. I'm suggesting that there might be something wrong with that rig that's giving you false testing failures. I don't particularly care about the patch itself -- all it does is save 3 bytes which are currently unused anyway, (although it might help Vivek's work.) However, I'm very concerned that you might be getting false failures, for obvious reasons.

-hpa

[*] On the other hand, as I discovered in the process, arch/i386/kernel/cpu/transmeta.c apparently gets miscompiled on my development system for top-of-Linus. This is a Linux bug and not gcc's fault. The following code:

	rdmsr(0x80860004, cap_mask, uk);
	wrmsr(0x80860004, ~0, uk);
	c->x86_capability[0] = cpuid_edx(0x00000001);
	wrmsr(0x80860004, cap_mask, uk);

... gets turned into ...

c0430f2d:       b9 04 00 86 80          mov    $0x80860004,%ecx
c0430f32:       0f 32                   rdmsr
c0430f34:       89 c6                   mov    %eax,%esi
c0430f36:       83 c8 ff                or     $0xffffffff,%eax
c0430f39:       89 d7                   mov    %edx,%edi
c0430f3b:       89 c2                   mov    %eax,%edx	## WTF!!
c0430f3d:       0f 30                   wrmsr
c0430f3f:       31 c9                   xor    %ecx,%ecx
c0430f41:       b8 01 00 00 00          mov    $0x1,%eax
c0430f46:       0f a2                   cpuid
c0430f48:       8b 45 8c                mov    0xffffff8c(%ebp),%eax
c0430f4b:       b9 04 00 86 80          mov    $0x80860004,%ecx
c0430f50:       89 50 0c                mov    %edx,0xc(%eax)
c0430f53:       b8 00 00 00 00          mov    $0x0,%eax
c0430f58:       89 fa                   mov    %edi,%edx
c0430f5a:       09 f0                   or     %esi,%eax
c0430f5c:       0f 30                   wrmsr

... with an immediate crash as a result, since -1 is not legal to write into the high part (%edx) of MSR 0x80860004.

This is due to native_write_msr() being incorrectly implemented as a macro. The preprocessed code expands to:
do { unsigned long long __val = native_read_msr(0x80860004); cap_mask =
__val; uk = __val >> 32; } while(0);
native_write_msr(0x80860004, ((unsigned long long)uk << 32) | ~0); c->x86_capability[0] = cpuid_edx(0x00000001); native_write_msr(0x80860004, ((unsigned long long)uk << 32) | cap_mask);

Spot the looney? "((unsigned long long)uk << 32) | ~0)" is *exactly* the same thing as "(unsigned long long)~0" which is exactly 0xffffffffffffffffULL. Without casting the lower argument to u32 before ORing, it is not guarded against sign extension as a result of promotion.

-hpa

Andi Kleen

2007-05-08 12:27 UTC #4 lore raw

Quoted 8 lines · H. Peter Anvin

Quoted 7 lines · Andrew Morton
arch/i386/boot/bootsect.S #endif · line 47
Quoted 3 lines
arch/i386/boot/bootsect.S +1 -1
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
Sigh, another blow struck in the ongoing struggle between my Vaio and the rest of the world.

Quoted 7 lines

Quoted 4 lines
Stone-cold black-screen lockup immediately upon boot.
Stock FC5 install, config at
http://userweb.kernel.org/~akpm/config-sony.txt
Andrew, I'm seriously starting to think there is something fundamentally wrong with that test setup.

I agree. The patch above does not change any code in the running kernel and is not even used in a normal kernel boot. Andrew, can you please double check that?

Quoted 4 lines

[*] On the other hand, as I discovered in the process, arch/i386/kernel/cpu/transmeta.c apparently gets miscompiled on my development system for top-of-Linus. This is a Linux bug and not gcc's fault. The following code:

You should have put that into a different mail (is there a electron shortage somewhere now? @) Ok, thanks for the well researched bug report. Will fix in my tree.

-Andi

H. Peter Anvin

2007-05-08 17:13 UTC #5 lore raw

Quoted 5 lines · Andi Kleen

You should have put that into a different mail (is there a electron shortage somewhere now? @) Ok, thanks for the well researched bug report. Will fix in my tree.

Nah. Just a brain shortage at 5 in the morning.

-hpa

Andrew Morton

2007-05-08 18:12 UTC #6 lore raw

In reply to H. Peter Anvin

Quoted 10 lines · On Tue, 08 May 2007 04:25:00 -0700 "H. Peter Anvin" <[email protected]> wrote:

Quoted 8 lines · Andrew Morton
arch/i386/boot/bootsect.S #endif · line 47
Quoted 4 lines
arch/i386/boot/bootsect.S +1 -1
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
Sigh, another blow struck in the ongoing struggle between my Vaio and the rest of the world.

Quoted 7 lines

Quoted 4 lines
Stone-cold black-screen lockup immediately upon boot.
Stock FC5 install, config at
http://userweb.kernel.org/~akpm/config-sony.txt
Andrew, I'm seriously starting to think there is something fundamentally wrong with that test setup.

heh. All the other bugs have been real oh-yeah-youre-right bugs. This is the only mystery bug which I recall.

Quoted 14 lines

The bootsect code in question is never executed. AT ALL. The only raison d'être for it at all is to print an error message if someone writes the kernel to a raw floppy disk. Nor does it change the alignment of the header or anything else to that effect -- the assembly code downstream has an explicit ".org" directive. For what it's worth, just to make sure I'm not crazy, I just re-tested both booting the kernel and booting the raw disk image, in simulation and on real hardware, and it doesn't change anything. I used your configuration file (yes '' | make oldconfig) minus Bluetooth (which is broken in current top of Linus) against top of tree Linus + the jmpw patch. I obviously don't have your Vaio, but I do have my own share of quirky hardware. FWIW, I netbooted the hardware using pxelinux 3.50-pre7 as the bootloader.[*]

Quoted 7 lines

I'm not writing this to give you a hard time, far from it. I'm suggesting that there might be something wrong with that rig that's giving you false testing failures. I don't particularly care about the patch itself -- all it does is save 3 bytes which are currently unused anyway, (although it might help Vivek's work.) However, I'm very concerned that you might be getting false failures, for obvious reasons.

I just retested bare 2.6.21 with that patch. Same hang.

Maybe the assembler or linker screwed something up.

Without patch:

(gdb) x/20i _start

0x0 <_start>:   ljmpw  $0x0,$0x8
0x6 <_start+6>: rolb   $0x8c,(%edi)
0x9 <start2+1>: enter  $0xd88e,$0x8e
0xd <start2+5>: rorb   $0xfb,0x7c00bcd0(%esi)
0x14 <start2+12>:       cld    
0x15 <start2+13>:       mov    $0x20ac0031,%esi
0x1a <msg_loop+2>:      (bad)  
0x1b <msg_loop+3>:      je     0x26 <die>
0x1d <msg_loop+5>:      mov    $0xe,%ah
0x1f <msg_loop+7>:      mov    $0x10cd0007,%ebx
0x24 <msg_loop+12>:     jmp    0x18 <msg_loop>
0x26 <die>:     xor    %eax,%eax
0x28 <die+2>:   int    $0x16

With patch:

(gdb) x/20i _start

0x0 <_start>:   ljmp   $0xc88c,$0x7c00005
0x7 <start2+2>: mov    %eax,%ds
0x9 <start2+4>: mov    %eax,%es
0xb <start2+6>: mov    %eax,%ss
0xd <start2+8>: mov    $0xfcfb7c00,%esp
0x12 <start2+13>:       mov    $0x20ac002e,%esi
0x17 <msg_loop+2>:      (bad)  
0x18 <msg_loop+3>:      je     0x23 <die>
0x1a <msg_loop+5>:      mov    $0xe,%ah
0x1c <msg_loop+7>:      mov    $0x10cd0007,%ebx
0x21 <msg_loop+12>:     jmp    0x15 <msg_loop>

not sure what's going on there. We seem to have confused gdb.

H. Peter Anvin

2007-05-08 18:27 UTC #7 lore raw

Quoted 33 lines · Andrew Morton

Without patch:

(gdb) x/20i _start

0x0 <_start>:   ljmpw  $0x0,$0x8
0x6 <_start+6>: rolb   $0x8c,(%edi)
0x9 <start2+1>: enter  $0xd88e,$0x8e
0xd <start2+5>: rorb   $0xfb,0x7c00bcd0(%esi)
0x14 <start2+12>:       cld    
0x15 <start2+13>:       mov    $0x20ac0031,%esi
0x1a <msg_loop+2>:      (bad)  
0x1b <msg_loop+3>:      je     0x26 <die>
0x1d <msg_loop+5>:      mov    $0xe,%ah
0x1f <msg_loop+7>:      mov    $0x10cd0007,%ebx
0x24 <msg_loop+12>:     jmp    0x18 <msg_loop>
0x26 <die>:     xor    %eax,%eax
0x28 <die+2>:   int    $0x16

With patch:

(gdb) x/20i _start

0x0 <_start>:   ljmp   $0xc88c,$0x7c00005
0x7 <start2+2>: mov    %eax,%ds
0x9 <start2+4>: mov    %eax,%es
0xb <start2+6>: mov    %eax,%ss
0xd <start2+8>: mov    $0xfcfb7c00,%esp
0x12 <start2+13>:       mov    $0x20ac002e,%esi
0x17 <msg_loop+2>:      (bad)  
0x18 <msg_loop+3>:      je     0x23 <die>
0x1a <msg_loop+5>:      mov    $0xe,%ah
0x1c <msg_loop+7>:      mov    $0x10cd0007,%ebx
0x21 <msg_loop+12>:     jmp    0x15 <msg_loop>

Quoted 2 lines

not sure what's going on there. We seem to have confused gdb.

You're trying to disassemble 16-bit code as if it was 32-bit code. The easiest way to disassemble is to have NASM installed on your machine and just do:

ndisasm bzImage

My setup code rewrite creates an intermediate .elf file to retain debugging information and make 16-bit disassembly ("objdump -d -m i8086") easier.

-hpa

Alexander van Heukelum

author

2007-05-08 18:36 UTC #8 lore raw

In reply to Andrew Morton

Quoted 13 lines · Andrew Morton

arch/i386/boot/bootsect.S #endif · line 47
Quoted 8 lines · On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum…
arch/i386/boot/bootsect.S +1 -1
@@ -44,7 +44,7 @@ #endif
 _start:
 
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
Sigh, another blow struck in the ongoing struggle between my Vaio and the rest of the world.

Quoted 4 lines

Stone-cold black-screen lockup immediately upon boot.
Stock FC5 install, config at
http://userweb.kernel.org/~akpm/config-sony.txt

Sigh... sounds impossible.

I assume this is repeatable? And this patch changes behaviour? What do you mean by immediate? Before or after decompression?

Fishy... As Andi and Peter have mentioned, this code is not executed. There is only one user of the same memory area this early... video.S uses it to store some collected data, but as far as I can see only the 'CONFIG_VIDEO_RETAIN' code also reads it back. If it happens before decompression, could you see if changing

	#define CONFIG_VIDEO_RETAIN -> #undef CONFIG_VIDEO_RETAIN

in arch/i386/boot/video.S changes anything?

Do you select a non-standard textmode? Does vga=ask show up?

*looks around some*

Oh! A padding hole in a struct! That could be a problem. If the freeze is after decompression, could you test if this makes it work again?

Greetings,
Alexander

---

Commit 89ec4c238e7a3d7e660291f3f1a8181381baad77 introduced a discrepancy between the struct screen_info which is used by the C code, and the PARAM_* offsets which are used in the real-mode kernel. This is an attempt to rectify the situation, but I have no way to test it.

Signed-off-by Alexander van Heukelum <[email protected]>

---

arch/i386/boot/bootsect.S +0 -0

arch/i386/boot/video.S +2 -1

index 8143c95..8e404cb 100644
@@ -95,7 +95,8 @@ #define PARAM_VESAPM_SEG	0x2e
 #define PARAM_VESAPM_OFF	0x30
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
-#define PARAM_CAPABILITIES	0x36
+#define PARAM_VESA_PAD		0x36
+#define PARAM_CAPABILITIES	0x38
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN

include/linux/screen_info.h +2 -2

index b02308e..0a2e892 100644
@@ -41,8 +41,8 @@ struct screen_info {
 	u16 vesapm_off;		/* 0x30 */
 	u16 pages;		/* 0x32 */
 	u16 vesa_attributes;	/* 0x34 */
-	u32 capabilities;       /* 0x36 */
-				/* 0x3a -- 0x3b reserved for future expansion */
+	u16 vesa_pad;		/* 0x36 */
+	u32 capabilities;       /* 0x38 */
 				/* 0x3c -- 0x3f micro stack for relocatable kernels */
 };

H. Peter Anvin

2007-05-08 18:46 UTC #9 lore raw

Quoted 4 lines · Alexander van Heukelum

Oh! A padding hole in a struct! That could be a problem. If the freeze is after decompression, could you test if this makes it work again?

The correct fix is to apply __attribute__((packed)) to this structure. Of course, changing the boot sector will cause different random values to be poked into this field...

I actually caught this during my setup code rewrite, but I forgot that in the old setup code the parameter structure overlays the boot sector (it doesn't in my code -- it starts out with nice clean zero-filled memory.)

-hpa

Alexander van Heukelum

author

2007-05-08 19:21 UTC #10 lore raw

Quoted 7 lines · H. Peter Anvin

Quoted 4 lines · Alexander van Heukelum
Oh! A padding hole in a struct! That could be a problem. If the freeze is after decompression, could you test if this makes it work again?
The correct fix is to apply __attribute__((packed)) to this structure.

Yeah, I thought about that possibility too, but the struct didn't need that (in my opinion ugly) annotation before. Oh well, here you go.

Greetings,
Alexander

---

Commit 89ec4c238e7a3d7e660291f3f1a8181381baad77 introduced a discrepancy between the struct screen_info which is used by the C code, and the PARAM_* offsets which are used in the real-mode kernel. As hpa suggests, adding __attribute__((packed)) to the struct fixes it.

Signed-off-by Alexander van Heukelum <[email protected]>

---

include/linux/screen_info.h +1 -1

index b02308e..4a7c24b 100644
@@ -44,7 +44,7 @@ struct screen_info {
 	u32 capabilities;       /* 0x36 */
 				/* 0x3a -- 0x3b reserved for future expansion */
 				/* 0x3c -- 0x3f micro stack for relocatable kernels */
-};
+} __attribute__((packed));
 
 extern struct screen_info screen_info;

H. Peter Anvin

2007-05-08 22:27 UTC #11 lore raw

Quoted 10 lines · Alexander van Heukelum

Quoted 5 lines · H. Peter Anvin
Quoted 3 lines · Alexander van Heukelum
Oh! A padding hole in a struct! That could be a problem. If the freeze is after decompression, could you test if this makes it work again?
The correct fix is to apply __attribute__((packed)) to this structure.
Yeah, I thought about that possibility too, but the struct didn't need that (in my opinion ugly) annotation before. Oh well, here you go.

It did need it, however, it just "happened to work" anyway.

Yes, it's ugly, but the damage is already done.

-hpa

Andrew Morton

2007-05-08 20:01 UTC #12 lore raw

In reply to Alexander van Heukelum

Quoted 14 lines · On Tue, 8 May 2007 20:32:32 +0200 Alexander van Heukelum…

Quoted 13 lines · Andrew Morton
arch/i386/boot/bootsect.S #endif · line 47
Quoted 8 lines · On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum…
arch/i386/boot/bootsect.S +1 -1
@@ -44,7 +44,7 @@ #endif
 _start:
 
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
Sigh, another blow struck in the ongoing struggle between my Vaio and the rest of the world.

Quoted 8 lines

Quoted 4 lines
Stone-cold black-screen lockup immediately upon boot.
Stock FC5 install, config at
http://userweb.kernel.org/~akpm/config-sony.txt
Sigh... sounds impossible.
I assume this is repeatable?

100%

Quoted 1 line

And this patch changes behaviour?

Yup.

Quoted 1 line

What do you mean by immediate?

Basically the last thing I see is grub.

Quoted 1 line

Before or after decompression?

Not sure - things happen pretty quickly and I suspect info is getting lost when the display is changing modes.

Quoted 9 lines

Fishy... As Andi and Peter have mentioned, this code is not executed. There is only one user of the same memory area this early... video.S uses it to store some collected data, but as far as I can see only the 'CONFIG_VIDEO_RETAIN' code also reads it back. If it happens before decompression, could you see if changing
	#define CONFIG_VIDEO_RETAIN -> #undef CONFIG_VIDEO_RETAIN
in arch/i386/boot/video.S changes anything?
Do you select a non-standard textmode?

yep.

Quoted 1 line

Does vga=ask show up?

no.

Quoted 4 lines

*looks around some*
Oh! A padding hole in a struct! That could be a problem. If the freeze is after decompression, could you test if this makes it work again?

I shall try that later in the day.

Quoted 42 lines

---
Commit 89ec4c238e7a3d7e660291f3f1a8181381baad77 introduced a discrepancy between the struct screen_info which is used by the C code, and the PARAM_* offsets which are used in the real-mode kernel. This is an attempt to rectify the situation, but I have no way to test it.
Signed-off-by Alexander van Heukelum <[email protected]>
---
arch/i386/boot/bootsect.S +0 -0
arch/i386/boot/video.S +2 -1
index 8143c95..8e404cb 100644
@@ -95,7 +95,8 @@ #define PARAM_VESAPM_SEG	0x2e
 #define PARAM_VESAPM_OFF	0x30
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
-#define PARAM_CAPABILITIES	0x36
+#define PARAM_VESA_PAD		0x36
+#define PARAM_CAPABILITIES	0x38
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN
include/linux/screen_info.h +2 -2
index b02308e..0a2e892 100644
@@ -41,8 +41,8 @@ struct screen_info {
 	u16 vesapm_off;		/* 0x30 */
 	u16 pages;		/* 0x32 */
 	u16 vesa_attributes;	/* 0x34 */
-	u32 capabilities;       /* 0x36 */
-				/* 0x3a -- 0x3b reserved for future expansion */
+	u16 vesa_pad;		/* 0x36 */
+	u32 capabilities;       /* 0x38 */
 				/* 0x3c -- 0x3f micro stack for relocatable kernels */
 };
 

Antonino A. Daplas

2007-05-09 00:04 UTC #13 lore raw

In reply to Alexander van Heukelum

Quoted 73 lines · Alexander van Heukelum

Quoted 13 lines · Andrew Morton
arch/i386/boot/bootsect.S #endif · line 47
Quoted 8 lines · On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum…
arch/i386/boot/bootsect.S +1 -1
@@ -44,7 +44,7 @@ #endif
 _start:
 
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
Sigh, another blow struck in the ongoing struggle between my Vaio and the rest of the world.
Quoted 4 lines
Stone-cold black-screen lockup immediately upon boot.
Stock FC5 install, config at
http://userweb.kernel.org/~akpm/config-sony.txt
Sigh... sounds impossible.
I assume this is repeatable? And this patch changes behaviour? What do you mean by immediate? Before or after decompression?
Fishy... As Andi and Peter have mentioned, this code is not executed. There is only one user of the same memory area this early... video.S uses it to store some collected data, but as far as I can see only the 'CONFIG_VIDEO_RETAIN' code also reads it back. If it happens before decompression, could you see if changing
	#define CONFIG_VIDEO_RETAIN -> #undef CONFIG_VIDEO_RETAIN
in arch/i386/boot/video.S changes anything?
Do you select a non-standard textmode? Does vga=ask show up?
*looks around some*
Oh! A padding hole in a struct! That could be a problem. If the freeze is after decompression, could you test if this makes it work again?
Greetings,
Alexander
---
Commit 89ec4c238e7a3d7e660291f3f1a8181381baad77 introduced a discrepancy between the struct screen_info which is used by the C code, and the PARAM_* offsets which are used in the real-mode kernel. This is an attempt to rectify the situation, but I have no way to test it.
Signed-off-by Alexander van Heukelum <[email protected]>
---
arch/i386/boot/bootsect.S +0 -0
arch/i386/boot/video.S +2 -1
index 8143c95..8e404cb 100644
@@ -95,7 +95,8 @@ #define PARAM_VESAPM_SEG	0x2e
 #define PARAM_VESAPM_OFF	0x30
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
-#define PARAM_CAPABILITIES	0x36
+#define PARAM_VESA_PAD		0x36
+#define PARAM_CAPABILITIES	0x38
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN
include/linux/screen_info.h +0 -0
index b02308e..0a2e892 100644

include/linux/screen_info.h struct screen_info { · line 48

Quoted 10 lines

include/linux/screen_info.h +2 -2

@@ -41,8 +41,8 @@ struct screen_info {
 	u16 vesapm_off;		/* 0x30 */
 	u16 pages;		/* 0x32 */
 	u16 vesa_attributes;	/* 0x34 */
-	u32 capabilities;       /* 0x36 */
-				/* 0x3a -- 0x3b reserved for future expansion */
+	u16 vesa_pad;		/* 0x36 */
+	u32 capabilities;       /* 0x38 */
 				/* 0x3c -- 0x3f micro stack for relocatable kernels */
 };

You would also need this minor change.

Tony

arch/i386/boot/video.S +1 -1

index 5e2280c..2d637d1 100644
@@ -95,7 +95,7 @@ #define PARAM_VESAPM_SEG      0x2e
 #define PARAM_VESAPM_OFF       0x30
 #define PARAM_LFB_PAGES                0x32
 #define PARAM_VESA_ATTRIB      0x34
-#define PARAM_CAPABILITIES     0x36
+#define PARAM_CAPABILITIES     0x38
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN

Alexander van Heukelum

author

2007-05-09 08:57 UTC #14 lore raw

Quoted 41 lines · Antonino A. Daplas

Quoted 20 lines · Alexander van Heukelum

arch/i386/boot/bootsect.S +0 -0

arch/i386/boot/video.S +2 -1

index 8143c95..8e404cb 100644
@@ -95,7 +95,8 @@ #define PARAM_VESAPM_SEG	0x2e
 #define PARAM_VESAPM_OFF	0x30
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
-#define PARAM_CAPABILITIES	0x36
+#define PARAM_VESA_PAD		0x36
+#define PARAM_CAPABILITIES	0x38
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN

include/linux/screen_info.h +0 -0

index b02308e..0a2e892 100644

include/linux/screen_info.h struct screen_info { · line 48

Quoted 10 lines

include/linux/screen_info.h +2 -2

@@ -41,8 +41,8 @@ struct screen_info {
 	u16 vesapm_off;		/* 0x30 */
 	u16 pages;		/* 0x32 */
 	u16 vesa_attributes;	/* 0x34 */
-	u32 capabilities;       /* 0x36 */
-				/* 0x3a -- 0x3b reserved for future expansion */
+	u16 vesa_pad;		/* 0x36 */
+	u32 capabilities;       /* 0x38 */
 				/* 0x3c -- 0x3f micro stack for relocatable kernels */
 };

You would also need this minor change.

Tony

arch/i386/boot/video.S +0 -0

index 5e2280c..2d637d1 100644

arch/i386/boot/video.S #define PARAM_VESAPM_SEG 0x2e · line 101

Quoted 8 lines

arch/i386/boot/video.S +1 -1

@@ -95,7 +95,7 @@ #define PARAM_VESAPM_SEG      0x2e
 #define PARAM_VESAPM_OFF       0x30
 #define PARAM_LFB_PAGES                0x32
 #define PARAM_VESA_ATTRIB      0x34
-#define PARAM_CAPABILITIES     0x36
+#define PARAM_CAPABILITIES     0x38
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN

Hi!

I'm making coffee now. I just don't see what I missed? Maybe you were led astray by the new PARAM_VESA_PAD I added?

Greetings,
Alexander

Antonino A. Daplas

2007-05-09 13:45 UTC #15 lore raw

Quoted 48 lines · Alexander van Heukelum

Quoted 47 lines · Antonino A. Daplas

Quoted 20 lines · Alexander van Heukelum

arch/i386/boot/bootsect.S +0 -0

arch/i386/boot/video.S +2 -1

index 8143c95..8e404cb 100644
@@ -95,7 +95,8 @@ #define PARAM_VESAPM_SEG	0x2e
 #define PARAM_VESAPM_OFF	0x30
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
-#define PARAM_CAPABILITIES	0x36
+#define PARAM_VESA_PAD		0x36
+#define PARAM_CAPABILITIES	0x38
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN

include/linux/screen_info.h +0 -0

index b02308e..0a2e892 100644

include/linux/screen_info.h struct screen_info { · line 48

Quoted 10 lines

include/linux/screen_info.h +2 -2

@@ -41,8 +41,8 @@ struct screen_info {
 	u16 vesapm_off;		/* 0x30 */
 	u16 pages;		/* 0x32 */
 	u16 vesa_attributes;	/* 0x34 */
-	u32 capabilities;       /* 0x36 */
-				/* 0x3a -- 0x3b reserved for future expansion */
+	u16 vesa_pad;		/* 0x36 */
+	u32 capabilities;       /* 0x38 */
 				/* 0x3c -- 0x3f micro stack for relocatable kernels */
 };

You would also need this minor change.

Tony

arch/i386/boot/video.S +1 -1

index 5e2280c..2d637d1 100644
@@ -95,7 +95,7 @@ #define PARAM_VESAPM_SEG      0x2e
 #define PARAM_VESAPM_OFF       0x30
 #define PARAM_LFB_PAGES                0x32
 #define PARAM_VESA_ATTRIB      0x34
-#define PARAM_CAPABILITIES     0x36
+#define PARAM_CAPABILITIES     0x38

Quoted 7 lines

arch/i386/boot/video.S #define PARAM_VESAPM_SEG 0x2e · line 101
Quoted 2 lines
arch/i386/boot/video.S +0 -0
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN
Hi!
I'm making coffee now. I just don't see what I missed? Maybe you were led astray by the new PARAM_VESA_PAD I added?

Since you moved screen_info.capabilities forward by 2 bytes, you must also tell video.S where to poke for the new location, otherwise, vesafb (which uses the capabilities field) will get confused.

These are the values for the unadjusted and adjusted PARAM_CAPABILITIES:
(The latter one is correct).

capabilities: 746f0000
capabilities: 1

Tony

H. Peter Anvin

2007-05-09 16:52 UTC #16 lore raw

In reply to Alexander van Heukelum

Quoted 6 lines · Alexander van Heukelum

Hi!
I'm making coffee now. I just don't see what I missed? Maybe you were led astray by the new PARAM_VESA_PAD I added?

Again, I object to changing a documented interface when it's not necessary to do so. Just declare the structure packed.

-hpa

Alexander van Heukelum

author

2007-05-09 18:21 UTC #17 lore raw

Quoted 10 lines · H. Peter Anvin

Quoted 6 lines · Alexander van Heukelum
Hi!
I'm making coffee now. I just don't see what I missed? Maybe you were led astray by the new PARAM_VESA_PAD I added?
Again, I object to changing a documented interface when it's not necessary to do so. Just declare the structure packed.

That would be like this, right?

http://www.ussg.iu.edu/hypermail/linux/kernel/0705.1/0383.html

Greetings,
Alexander

H. Peter Anvin

2007-05-09 18:27 UTC #18 lore raw

Quoted 7 lines · Alexander van Heukelum

Quoted 6 lines · H. Peter Anvin
Quoted 5 lines · Alexander van Heukelum
Hi!
I'm making coffee now. I just don't see what I missed? Maybe you were led astray by the new PARAM_VESA_PAD I added?

Quoted 7 lines

Quoted 2 lines
Again, I object to changing a documented interface when it's not necessary to do so. Just declare the structure packed.
That would be like this, right?
http://www.ussg.iu.edu/hypermail/linux/kernel/0705.1/0383.html

Absolutely.

-hpa

Lennart Sorensen

2007-05-09 14:30 UTC #19 lore raw

In reply to Antonino A. Daplas

Quoted 54 lines · Antonino A. Daplas

Quoted 53 lines · Alexander van Heukelum
Quoted 13 lines · Andrew Morton
arch/i386/boot/bootsect.S #endif · line 47
Quoted 8 lines · On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum…
arch/i386/boot/bootsect.S +1 -1
@@ -44,7 +44,7 @@ #endif
 _start:
 
 	# Normalize the start address
-	jmpl	$BOOTSEG, $start2
+	jmpw	$BOOTSEG, $start2
Sigh, another blow struck in the ongoing struggle between my Vaio and the rest of the world.
Quoted 4 lines
Stone-cold black-screen lockup immediately upon boot.
Stock FC5 install, config at
http://userweb.kernel.org/~akpm/config-sony.txt
Sigh... sounds impossible.
I assume this is repeatable? And this patch changes behaviour? What do you mean by immediate? Before or after decompression?
Fishy... As Andi and Peter have mentioned, this code is not executed. There is only one user of the same memory area this early... video.S uses it to store some collected data, but as far as I can see only the 'CONFIG_VIDEO_RETAIN' code also reads it back. If it happens before decompression, could you see if changing
	#define CONFIG_VIDEO_RETAIN -> #undef CONFIG_VIDEO_RETAIN
in arch/i386/boot/video.S changes anything?
Do you select a non-standard textmode? Does vga=ask show up?
*looks around some*
Oh! A padding hole in a struct! That could be a problem. If the freeze is after decompression, could you test if this makes it work again?
Greetings,
Alexander
---
Commit 89ec4c238e7a3d7e660291f3f1a8181381baad77 introduced a discrepancy between the struct screen_info which is used by the C code, and the PARAM_* offsets which are used in the real-mode kernel. This is an attempt to rectify the situation, but I have no way to test it.
Signed-off-by Alexander van Heukelum <[email protected]>
---

Quoted 12 lines

arch/i386/boot/video.S #define PARAM_VESAPM_SEG 0x2e · line 99
Quoted 12 lines
arch/i386/boot/bootsect.S +0 -0
arch/i386/boot/video.S +2 -1
index 8143c95..8e404cb 100644
@@ -95,7 +95,8 @@ #define PARAM_VESAPM_SEG	0x2e
 #define PARAM_VESAPM_OFF	0x30
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
-#define PARAM_CAPABILITIES	0x36
+#define PARAM_VESA_PAD		0x36
+#define PARAM_CAPABILITIES	0x38

Isn't this ^^^ what you are suggesting below?

Quoted 27 lines

Quoted 7 lines

arch/i386/boot/video.S +0 -0
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN
include/linux/screen_info.h +0 -0
index b02308e..0a2e892 100644

include/linux/screen_info.h struct screen_info { · line 48

Quoted 10 lines

include/linux/screen_info.h +2 -2

@@ -41,8 +41,8 @@ struct screen_info {
 	u16 vesapm_off;		/* 0x30 */
 	u16 pages;		/* 0x32 */
 	u16 vesa_attributes;	/* 0x34 */
-	u32 capabilities;       /* 0x36 */
-				/* 0x3a -- 0x3b reserved for future expansion */
+	u16 vesa_pad;		/* 0x36 */
+	u32 capabilities;       /* 0x38 */
 				/* 0x3c -- 0x3f micro stack for relocatable kernels */
 };

You would also need this minor change.

Tony

arch/i386/boot/video.S +0 -0

index 5e2280c..2d637d1 100644

arch/i386/boot/video.S #define PARAM_VESAPM_SEG 0x2e · line 101

Quoted 8 lines

arch/i386/boot/video.S +1 -1

@@ -95,7 +95,7 @@ #define PARAM_VESAPM_SEG      0x2e
 #define PARAM_VESAPM_OFF       0x30
 #define PARAM_LFB_PAGES                0x32
 #define PARAM_VESA_ATTRIB      0x34
-#define PARAM_CAPABILITIES     0x36
+#define PARAM_CAPABILITIES     0x38
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN

--
Len Sorensen

Antonino A. Daplas

2007-05-09 14:51 UTC #20 lore raw

Quoted 5 lines · Lennart Sorensen

Quoted 3 lines · Antonino A. Daplas
Quoted 2 lines · Alexander van Heukelum
Quoted 1 line · Andrew Morton
On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum <[email protected]> wrote:

Quoted 10 lines

Quoted 7 lines
arch/i386/boot/video.S #define PARAM_VESAPM_SEG 0x2e · line 99
Quoted 7 lines
arch/i386/boot/video.S +2 -1
@@ -95,7 +95,8 @@ #define PARAM_VESAPM_SEG	0x2e
 #define PARAM_VESAPM_OFF	0x30
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
-#define PARAM_CAPABILITIES	0x36
+#define PARAM_VESA_PAD		0x36
+#define PARAM_CAPABILITIES	0x38
Isn't this ^^^ what you are suggesting below?

Yes, sorry, why did I miss that one :-)

Tony