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]>
---
diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
index 011b7a4..ae9df0d 100644
--- a/arch/i386/boot/bootsect.S
+++ b/arch/i386/boot/bootsect.S
@@ -44,7 +44,7 @@ #endif
_start:
# Normalize the start address
- jmpl $BOOTSEG, $start2
+ jmpw $BOOTSEG, $start2
start2:
movw %cs, %ax
Acked-by: H. Peter Anvin <[email protected]>
Alexander van Heukelum wrote:
> 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]>
>
> ---
>
> diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
> index 011b7a4..ae9df0d 100644
> --- a/arch/i386/boot/bootsect.S
> +++ b/arch/i386/boot/bootsect.S
> @@ -44,7 +44,7 @@ #endif
> _start:
>
> # Normalize the start address
> - jmpl $BOOTSEG, $start2
> + jmpw $BOOTSEG, $start2
>
> start2:
> movw %cs, %ax
On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum <[email protected]> wrote:
> 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]>
>
> ---
>
> diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
> index 011b7a4..ae9df0d 100644
> --- a/arch/i386/boot/bootsect.S
> +++ b/arch/i386/boot/bootsect.S
> @@ -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
Andrew Morton wrote:
>>
>> # 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
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
On Tuesday 08 May 2007 13:25, H. Peter Anvin wrote:
> Andrew Morton wrote:
> >> # 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
>
> 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?
> [*] 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
Andi Kleen wrote:
>
> 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
On Tue, 08 May 2007 04:25:00 -0700 "H. Peter Anvin" <[email protected]> wrote:
> Andrew Morton wrote:
>
> >>
> >> # 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
>
> 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.
> 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.
>
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.
Andrew Morton wrote:
>
> 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.
>
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
On Tue, May 08, 2007 at 03:28:17AM -0700, Andrew Morton wrote:
> On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum <[email protected]> wrote:
> > --- a/arch/i386/boot/bootsect.S
> > +++ b/arch/i386/boot/bootsect.S
> > @@ -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
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]>
---
diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
index 8143c95..8e404cb 100644
--- a/arch/i386/boot/video.S
+++ b/arch/i386/boot/video.S
@@ -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
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index b02308e..0a2e892 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -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 */
};
Alexander van Heukelum wrote:
>
> 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
On Tue, May 08, 2007 at 11:45:47AM -0700, H. Peter Anvin wrote:
> Alexander van Heukelum wrote:
> >
> > 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]>
---
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index b02308e..4a7c24b 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -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;
On Tue, 8 May 2007 20:32:32 +0200
Alexander van Heukelum <[email protected]> wrote:
> On Tue, May 08, 2007 at 03:28:17AM -0700, Andrew Morton wrote:
> > On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum <[email protected]> wrote:
> > > --- a/arch/i386/boot/bootsect.S
> > > +++ b/arch/i386/boot/bootsect.S
> > > @@ -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
>
> Sigh... sounds impossible.
>
> I assume this is repeatable?
100%
> And this patch changes behaviour?
Yup.
> What do you mean by immediate?
Basically the last thing I see is grub.
> Before or after decompression?
Not sure - things happen pretty quickly and I suspect info is getting lost
when the display is changing modes.
> 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.
> Does vga=ask show up?
no.
> *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.
>
> ---
>
> 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]>
>
> ---
>
> diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
> diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
> index 8143c95..8e404cb 100644
> --- a/arch/i386/boot/video.S
> +++ b/arch/i386/boot/video.S
> @@ -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
> diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
> index b02308e..0a2e892 100644
> --- a/include/linux/screen_info.h
> +++ b/include/linux/screen_info.h
> @@ -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 */
> };
>
Alexander van Heukelum wrote:
> On Tue, May 08, 2007 at 11:45:47AM -0700, H. Peter Anvin wrote:
>> Alexander van Heukelum wrote:
>>> 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
On Tue, 2007-05-08 at 20:32 +0200, Alexander van Heukelum wrote:
> On Tue, May 08, 2007 at 03:28:17AM -0700, Andrew Morton wrote:
> > On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum <[email protected]> wrote:
> > > --- a/arch/i386/boot/bootsect.S
> > > +++ b/arch/i386/boot/bootsect.S
> > > @@ -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
>
> 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]>
>
> ---
>
> diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
> diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
> index 8143c95..8e404cb 100644
> --- a/arch/i386/boot/video.S
> +++ b/arch/i386/boot/video.S
> @@ -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
> diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
> index b02308e..0a2e892 100644
> --- a/include/linux/screen_info.h
> +++ b/include/linux/screen_info.h
> @@ -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
diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
index 5e2280c..2d637d1 100644
--- a/arch/i386/boot/video.S
+++ b/arch/i386/boot/video.S
@@ -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
On Wed, May 09, 2007 at 08:04:07AM +0800, Antonino A. Daplas wrote:
> On Tue, 2007-05-08 at 20:32 +0200, Alexander van Heukelum wrote:
> > diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
> > diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
> > index 8143c95..8e404cb 100644
> > --- a/arch/i386/boot/video.S
> > +++ b/arch/i386/boot/video.S
> > @@ -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
> > diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
> > index b02308e..0a2e892 100644
> > --- a/include/linux/screen_info.h
> > +++ b/include/linux/screen_info.h
> > @@ -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
>
> diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
> index 5e2280c..2d637d1 100644
> --- a/arch/i386/boot/video.S
> +++ b/arch/i386/boot/video.S
> @@ -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
On Wed, 2007-05-09 at 10:54 +0200, Alexander van Heukelum wrote:
> On Wed, May 09, 2007 at 08:04:07AM +0800, Antonino A. Daplas wrote:
> > On Tue, 2007-05-08 at 20:32 +0200, Alexander van Heukelum wrote:
> > > diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
> > > diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
> > > index 8143c95..8e404cb 100644
> > > --- a/arch/i386/boot/video.S
> > > +++ b/arch/i386/boot/video.S
> > > @@ -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
> > > diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
> > > index b02308e..0a2e892 100644
> > > --- a/include/linux/screen_info.h
> > > +++ b/include/linux/screen_info.h
> > > @@ -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
> >
> > diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
> > index 5e2280c..2d637d1 100644
> > --- a/arch/i386/boot/video.S
> > +++ b/arch/i386/boot/video.S
> > @@ -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?
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
On Wed, May 09, 2007 at 08:04:07AM +0800, Antonino A. Daplas wrote:
> On Tue, 2007-05-08 at 20:32 +0200, Alexander van Heukelum wrote:
> > On Tue, May 08, 2007 at 03:28:17AM -0700, Andrew Morton wrote:
> > > On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum <[email protected]> wrote:
> > > > --- a/arch/i386/boot/bootsect.S
> > > > +++ b/arch/i386/boot/bootsect.S
> > > > @@ -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
> >
> > 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]>
> >
> > ---
> >
> > diff --git a/arch/i386/boot/bootsect.S b/arch/i386/boot/bootsect.S
> > diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
> > index 8143c95..8e404cb 100644
> > --- a/arch/i386/boot/video.S
> > +++ b/arch/i386/boot/video.S
> > @@ -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?
> > /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
> > #ifdef CONFIG_VIDEO_RETAIN
> > diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
> > index b02308e..0a2e892 100644
> > --- a/include/linux/screen_info.h
> > +++ b/include/linux/screen_info.h
> > @@ -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
>
> diff --git a/arch/i386/boot/video.S b/arch/i386/boot/video.S
> index 5e2280c..2d637d1 100644
> --- a/arch/i386/boot/video.S
> +++ b/arch/i386/boot/video.S
> @@ -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
On Wed, 2007-05-09 at 10:30 -0400, Lennart Sorensen wrote:
> On Wed, May 09, 2007 at 08:04:07AM +0800, Antonino A. Daplas wrote:
> > On Tue, 2007-05-08 at 20:32 +0200, Alexander van Heukelum wrote:
> > > On Tue, May 08, 2007 at 03:28:17AM -0700, Andrew Morton wrote:
> > > > On Sat, 5 May 2007 12:44:52 +0200 Alexander van Heukelum <[email protected]> wrote:
> > > @@ -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
Alexander van Heukelum wrote:
>
> 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
On Wed, May 09, 2007 at 09:51:36AM -0700, H. Peter Anvin wrote:
> Alexander van Heukelum wrote:
> >
> > 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
Alexander van Heukelum wrote:
> On Wed, May 09, 2007 at 09:51:36AM -0700, H. Peter Anvin wrote:
>> Alexander van Heukelum wrote:
>>> 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
>
Absolutely.
-hpa