Hi,
PaX Team has sent me this patch for inclusion. Basically, during early
boot on x86, the exception handler does not make a special case for
exceptions which push an error code onto the stack, leading to a return
to a wrong address. Two patches were proposed, one which would add a
special case for all exceptions using the return code, and this one. The
former was of no use in its form because the return from the exception
handler would get back to the faulting exception, causing it to loop.
This one should be better as it effectively hangs the system using HLT
to prevent CPU from burning.
If nobody has any objections, I will merge it. In this case, I would also
like someone to check if 2.6 needs it and to port it in this case.
Thanks,
Willy
--
fix the longest existing kernel bug ever (since 0.01 ;-). basically,
the dummy interrupt handler installed for the early boot period does
not work for exceptions that push an error code as well, effectively
making the iret at the end of the handler to trigger another exception,
ad infinitum, or rather, until the kernel stack runs over, trashes all
memory below and eventually causes a CPU reset or a hang. without this
fix the early printk facility in 2.6 is also rather useless.
diff -Nurp linux-2.4.33/arch/i386/kernel/head.S linux-2.4.33-early-
inthandler/arch/i386/kernel/head.S
--- linux-2.4.33/arch/i386/kernel/head.S 2003-11-28 19:26:19.000000000 +0100
+++ linux-2.4.33-early-inthandler/arch/i386/kernel/head.S 2006-08-29
14:19:55.000000000 +0200
@@ -325,27 +325,21 @@ ENTRY(stack_start)
/* This is the default interrupt "handler" :-) */
int_msg:
- .asciz "Unknown interrupt\n"
+ .asciz "Unknown interrupt, stack: %p %p %p %p\n"
ALIGN
ignore_int:
cld
- pushl %eax
- pushl %ecx
- pushl %edx
- pushl %es
- pushl %ds
movl $(__KERNEL_DS),%eax
movl %eax,%ds
movl %eax,%es
+ pushl 12(%esp)
+ pushl 12(%esp)
+ pushl 12(%esp)
+ pushl 12(%esp)
pushl $int_msg
call SYMBOL_NAME(printk)
- popl %eax
- popl %ds
- popl %es
- popl %edx
- popl %ecx
- popl %eax
- iret
+1: hlt
+ jmp 1b
/*
* The interrupt descriptor table has room for 256 idt's,
Willy Tarreau <[email protected]> writes:
> Hi,
>
> PaX Team has sent me this patch for inclusion. Basically, during early
> boot on x86, the exception handler does not make a special case for
> exceptions which push an error code onto the stack, leading to a return
> to a wrong address. Two patches were proposed, one which would add a
> special case for all exceptions using the return code, and this one. The
> former was of no use in its form because the return from the exception
> handler would get back to the faulting exception, causing it to loop.
>
> This one should be better as it effectively hangs the system using HLT
> to prevent CPU from burning.
Looks good.
[I'm glad this particular ward in x86 was fixed in x86-64 ...]
>
> If nobody has any objections, I will merge it. In this case, I would also
> like someone to check if 2.6 needs it and to port it in this case.
I don't think you should merge anything like this before 2.6 does. Otherwise
we just end up with the mad situation again that an old release has
more bugs fixed or more features than the new release.
-Andi
On Wed, Aug 30, 2006 at 11:51:39AM +0200, Andi Kleen wrote:
> Willy Tarreau <[email protected]> writes:
>
> > Hi,
> >
> > PaX Team has sent me this patch for inclusion. Basically, during early
> > boot on x86, the exception handler does not make a special case for
> > exceptions which push an error code onto the stack, leading to a return
> > to a wrong address. Two patches were proposed, one which would add a
> > special case for all exceptions using the return code, and this one. The
> > former was of no use in its form because the return from the exception
> > handler would get back to the faulting exception, causing it to loop.
> >
> > This one should be better as it effectively hangs the system using HLT
> > to prevent CPU from burning.
>
> Looks good.
>
> [I'm glad this particular ward in x86 was fixed in x86-64 ...]
good.
> > If nobody has any objections, I will merge it. In this case, I would also
> > like someone to check if 2.6 needs it and to port it in this case.
>
> I don't think you should merge anything like this before 2.6 does. Otherwise
> we just end up with the mad situation again that an old release has
> more bugs fixed or more features than the new release.
Unfortunately, this situation is even more difficult for me, because it's
getting very hard to track patches that get applied, rejected, modified or
obsoleted, which is even more true when people don't always think about
sending an ACK after the patch finally gets in. I already have a few pending
patches in my queue waiting for an ACK that will have to be tracked if the
persons do not respond, say, within one week. Otherwise I might simply lose
them.
I think that the good method would be to :
- announce the patch
- find a volunteer to port it
- apply it once the volunteer agrees to handle it
This way, no code gets lost because there's always someone to track it.
> -Andi
Regards,
Willy
> Unfortunately, this situation is even more difficult for me, because it's
> getting very hard to track patches that get applied, rejected, modified or
> obsoleted, which is even more true when people don't always think about
> sending an ACK after the patch finally gets in. I already have a few pending
> patches in my queue waiting for an ACK that will have to be tracked if the
> persons do not respond, say, within one week. Otherwise I might simply lose
> them.
It shouldn't be that hard to check gitweb or git output occasionally
for the patches. You can probably even automate that.
> I think that the good method would be to :
> - announce the patch
> - find a volunteer to port it
> - apply it once the volunteer agrees to handle it
> This way, no code gets lost because there's always someone to track it.
I can put that one into my tree for .19
-Andi
On Wed, Aug 30, 2006 at 02:59:14PM +0200, Andi Kleen wrote:
>
> > Unfortunately, this situation is even more difficult for me, because it's
> > getting very hard to track patches that get applied, rejected, modified or
> > obsoleted, which is even more true when people don't always think about
> > sending an ACK after the patch finally gets in. I already have a few pending
> > patches in my queue waiting for an ACK that will have to be tracked if the
> > persons do not respond, say, within one week. Otherwise I might simply lose
> > them.
>
> It shouldn't be that hard to check gitweb or git output occasionally
> for the patches. You can probably even automate that.
That's already what I'm doing, and yes, it is *that* hard. We're literally
speaking about *thousands* of patches. It's as difficult to find one patch
within 2.6 git changes as it is to find a useful mail in the middle of 99%
spam. This is not because of GIT but because of the number of changes.
> > I think that the good method would be to :
> > - announce the patch
> > - find a volunteer to port it
> > - apply it once the volunteer agrees to handle it
> > This way, no code gets lost because there's always someone to track it.
>
> I can put that one into my tree for .19
Thanks for this andi,
Willy
On Wed, 30 Aug 2006 15:16:12 +0200
Willy Tarreau <[email protected]> wrote:
> That's already what I'm doing, and yes, it is *that* hard. We're literally
> speaking about *thousands* of patches. It's as difficult to find one patch
> within 2.6 git changes as it is to find a useful mail in the middle of 99%
> spam. This is not because of GIT but because of the number of changes.
Willy,
The git-cherry command might be useful for you in this situation. It will
show you all the patches in one branch that have been merged in an upstream
branch, and those that haven't. Not sure if it's perfect for your situation,
but may be worth a look.
Sean
On Wed, Aug 30, 2006 at 10:00:15AM -0400, Sean wrote:
> On Wed, 30 Aug 2006 15:16:12 +0200
> Willy Tarreau <[email protected]> wrote:
>
> > That's already what I'm doing, and yes, it is *that* hard. We're literally
> > speaking about *thousands* of patches. It's as difficult to find one patch
> > within 2.6 git changes as it is to find a useful mail in the middle of 99%
> > spam. This is not because of GIT but because of the number of changes.
>
> Willy,
>
> The git-cherry command might be useful for you in this situation. It will
> show you all the patches in one branch that have been merged in an upstream
> branch, and those that haven't. Not sure if it's perfect for your situation,
> but may be worth a look.
I've already used it (it's what I was using when to maintain the 2.4-hf
tree in parallel to Marcelo's mainline). But it's useful when you have
*a few* patches. I'm really speaking about *thousands* of patches that
get merged into 2.6 every few months and this is what makes the job difficult.
Not to mention that they also get merged in 2.6-mm in advance, or that
sometimes they are obsoleted and/or replaced by something else.
Clearly, I'm not going to track all 2.6 patch by patch to maintain 2.4 !
The most scalable workflow is distributed, and should be oriented towards
push and not pull. We just have to ensure that *someone* takes care of
each patch and tracks it up to its merge.
> Sean
Cheers,
Willy
On Wednesday 30 August 2006 18:25, [email protected] wrote:
> On 30 Aug 2006 at 14:59, Andi Kleen wrote:
> > > I think that the good method would be to :
> > > - announce the patch
> > > - find a volunteer to port it
> > > - apply it once the volunteer agrees to handle it
> > > This way, no code gets lost because there's always someone to track it.
> >
> > I can put that one into my tree for .19
>
> here's my quick attempt:
It would be better to separate exceptions from interrupts here.
A spurious interrupt is not necessarily fatal, just an exception is.
But I went with the simpler patch with some changes now
(added PANIC to the message etc.)
>
> --- linux-2.6.18-rc5/arch/i386/kernel/head.S 2006-08-28 11:37:31.000000000
> +0200
> +++ linux-2.6.18-rc5-fix/arch/i386/kernel/head.S 2006-08-30
> 18:22:15.000000000 +0200
> @@ -382,34 +382,25 @@ rp_sidt:
> /* This is the default interrupt "handler" :-) */
> ALIGN
> ignore_int:
> - cld
> #ifdef CONFIG_PRINTK
> - pushl %eax
> - pushl %ecx
> - pushl %edx
> - pushl %es
> - pushl %ds
> + cld
> movl $(__KERNEL_DS),%eax
> movl %eax,%ds
> movl %eax,%es
> - pushl 16(%esp)
> - pushl 24(%esp)
> - pushl 32(%esp)
> - pushl 40(%esp)
> + pushl 12(%esp)
> + pushl 12(%esp)
> + pushl 12(%esp)
> + pushl 12(%esp)
> pushl $int_msg
> #ifdef CONFIG_EARLY_PRINTK
> call early_printk
> #else
> call printk
> #endif
> - addl $(5*4),%esp
> - popl %ds
> - popl %es
> - popl %edx
> - popl %ecx
> - popl %eax
> #endif
> - iret
> +1: hlt
This is wrong because i386 still supports some CPUs that don't support
HLT.
-Andi
On Wed, 30 Aug 2006, Andi Kleen wrote:
> On Wednesday 30 August 2006 18:25, [email protected] wrote:
>> On 30 Aug 2006 at 14:59, Andi Kleen wrote:
>>>> I think that the good method would be to :
>>>> - announce the patch
>>>> - find a volunteer to port it
>>>> - apply it once the volunteer agrees to handle it
>>>> This way, no code gets lost because there's always someone to track it.
>>>
>>> I can put that one into my tree for .19
>>
>> here's my quick attempt:
>
>
> It would be better to separate exceptions from interrupts here.
> A spurious interrupt is not necessarily fatal, just an exception is.
>
> But I went with the simpler patch with some changes now
> (added PANIC to the message etc.)
>
>>
>> --- linux-2.6.18-rc5/arch/i386/kernel/head.S 2006-08-28 11:37:31.000000000
>> +0200
>> +++ linux-2.6.18-rc5-fix/arch/i386/kernel/head.S 2006-08-30
>> 18:22:15.000000000 +0200
>> @@ -382,34 +382,25 @@ rp_sidt:
>> /* This is the default interrupt "handler" :-) */
>> ALIGN
>> ignore_int:
>> - cld
>> #ifdef CONFIG_PRINTK
>> - pushl %eax
>> - pushl %ecx
>> - pushl %edx
>> - pushl %es
>> - pushl %ds
>> + cld
>> movl $(__KERNEL_DS),%eax
>> movl %eax,%ds
>> movl %eax,%es
>> - pushl 16(%esp)
>> - pushl 24(%esp)
>> - pushl 32(%esp)
>> - pushl 40(%esp)
>> + pushl 12(%esp)
>> + pushl 12(%esp)
>> + pushl 12(%esp)
>> + pushl 12(%esp)
>> pushl $int_msg
>> #ifdef CONFIG_EARLY_PRINTK
>> call early_printk
>> #else
>> call printk
>> #endif
>> - addl $(5*4),%esp
>> - popl %ds
>> - popl %es
>> - popl %edx
>> - popl %ecx
>> - popl %eax
>> #endif
>> - iret
>> +1: hlt
>
> This is wrong because i386 still supports some CPUs that don't support
> HLT.
>
> -Andi
Even the i286 and the 8086 support hlt. Is there some Cyrix chip that
you are trying to preserve? I think even those all implimented
hlt as well.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.24 on an i686 machine (5592.62 BogoMips).
New book: http://www.AbominableFirebug.com/
_
****************************************************************
The information transmitted in this message is confidential and may be privileged. Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to [email protected] - and destroy all copies of this information, including any attachments, without reading or disclosing them.
Thank you.
> Even the i286 and the 8086 support hlt. Is there some Cyrix chip that
> you are trying to preserve? I think even those all implimented
> hlt as well.
According to the kernel code it's
char hlt_works_ok; /* Problems on some 486Dx4's and old 386's */
I don't know more details about what these problems were.
-Andi
On Wed, 30 Aug 2006, Andi Kleen wrote:
>
>> Even the i286 and the 8086 support hlt. Is there some Cyrix chip that
>> you are trying to preserve? I think even those all implimented
>> hlt as well.
>
>
> According to the kernel code it's
>
> char hlt_works_ok; /* Problems on some 486Dx4's and old 386's */
>
> I don't know more details about what these problems were.
>
> -Andi
>
Oh yes. There were some buggy chips that would not let an interrupt
take the CPU out of hlt! However, what's wanted here is a hard-stop
which you get even with buggy chips because an interrupt won't
awaken them.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.24 on an i686 machine (5592.62 BogoMips).
New book: http://www.AbominableFirebug.com/
_
****************************************************************
The information transmitted in this message is confidential and may be privileged. Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to [email protected] - and destroy all copies of this information, including any attachments, without reading or disclosing them.
Thank you.
On Wednesday 30 August 2006 19:33, [email protected] wrote:
>
> > But I went with the simpler patch with some changes now
> > (added PANIC to the message etc.)
>
> can you post it please?
ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/i386-early-exception
-Andi
P.S.: If you provide patches in the future again also please provide
a real name and a Signed-off-by line
On Wednesday 30 August 2006 20:21, [email protected] wrote:
> On 30 Aug 2006 at 19:52, Andi Kleen wrote:
> > On Wednesday 30 August 2006 19:33, [email protected] wrote:
>
> > > > But I went with the simpler patch with some changes now
> > > > (added PANIC to the message etc.)
> > >
> > > can you post it please?
> >
> > ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/i386-early-exception
>
> thanks, although i suggest you put back the hlt as Dick Johnson explained it.
Unless someone can confirm there were not other problems on those 386s/486s
in HLT no.
>
> > -Andi
> >
> > P.S.: If you provide patches in the future again also please provide
> > a real name and a Signed-off-by line
>
> no problem for the signed-off-by, but what passes for a real name?
> i mean, how do i prove it?
We trust you if you tell us.
> ps: sorry but [email protected] bounces for me, removed from cc.
For me too.
-Andi
On Wed, Aug 30, 2006 at 08:26:05PM +0200, Andi Kleen wrote:
> On Wednesday 30 August 2006 20:21, [email protected] wrote:
> > On 30 Aug 2006 at 19:52, Andi Kleen wrote:
> > > On Wednesday 30 August 2006 19:33, [email protected] wrote:
> >
> > > > > But I went with the simpler patch with some changes now
> > > > > (added PANIC to the message etc.)
> > > >
> > > > can you post it please?
> > >
> > > ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/i386-early-exception
> >
> > thanks, although i suggest you put back the hlt as Dick Johnson explained it.
>
> Unless someone can confirm there were not other problems on those 386s/486s
> in HLT no.
Andi, if you remove the HLT here, some CPUs will spin at full speed. This
is nasty during boot because some of them might not have enabled their
fans yet for instance and could fry if nobody's looking (eg: live reset
caused by hardware problem). Even if HLT does not work on some CPUs,
the JMP after it will spin around it and the initial goal will be achieved.
Regards,
Willy
> Andi, if you remove the HLT here, some CPUs will spin at full speed. This
> is nasty during boot because some of them might not have enabled their
> fans yet for instance
That would be a severe bug in the platform. Basically always the fans are managed
by SMM code.
-Andi
On Wed, Aug 30, 2006 at 09:36:54PM +0200, Andi Kleen wrote:
>
> > Andi, if you remove the HLT here, some CPUs will spin at full speed. This
> > is nasty during boot because some of them might not have enabled their
> > fans yet for instance
>
> That would be a severe bug in the platform. Basically always the fans are managed
> by SMM code.
It was just an example. Other examples include virtual machines never
stopping because they will see the guest is working and not halted.
Anyway, the "hlt;jmp $" is already used in boot/setup.S, and the
HLT bug you have pointed is, as explained by Richard, a problem
preventing some CPUs from WAKING up from HLT. It causes a problem
when HLT is used in the idle loop. Users who have this problem must
pass the "no-hlt" command line option, otherwise their CPU will
halt during the idle loop (which is the effect we're looking for),
and those without the bug (those who don't have to pass "no-hlt")
will also have the HLT working.
So, to conclude, with or without the bug, "hlt;jmp $" will halt the
CPU as we expect it to. Therefore, I'd like you to restore it in
the patch.
Thanks,
Willy
On Wednesday 30 August 2006 22:03, Willy Tarreau wrote:
> On Wed, Aug 30, 2006 at 09:36:54PM +0200, Andi Kleen wrote:
> >
> > > Andi, if you remove the HLT here, some CPUs will spin at full speed. This
> > > is nasty during boot because some of them might not have enabled their
> > > fans yet for instance
> >
> > That would be a severe bug in the platform. Basically always the fans are managed
> > by SMM code.
>
> It was just an example. Other examples include virtual machines never
> stopping because they will see the guest is working and not halted.
They have to deal with that anyways because the machine can just
crash with a busy loop. And BTW -- take a look at the normal panic.
-Andi
On Wed, Aug 30, 2006 at 10:06:46PM +0200, Andi Kleen wrote:
> On Wednesday 30 August 2006 22:03, Willy Tarreau wrote:
> > On Wed, Aug 30, 2006 at 09:36:54PM +0200, Andi Kleen wrote:
> > >
> > > > Andi, if you remove the HLT here, some CPUs will spin at full speed. This
> > > > is nasty during boot because some of them might not have enabled their
> > > > fans yet for instance
> > >
> > > That would be a severe bug in the platform. Basically always the fans are managed
> > > by SMM code.
> >
> > It was just an example. Other examples include virtual machines never
> > stopping because they will see the guest is working and not halted.
>
> They have to deal with that anyways because the machine can just
> crash with a busy loop. And BTW -- take a look at the normal panic.
OK, generic multi-platform code, blinking keyboard leds, etc...
I don't get your point, what are you trying to demonstrate ?
If you want to explain your reject of the HLT instruction just
because other places in the kernel don't use it, that's rather
counter-evolutive and another approach might be to ask if it
would not be appropriate to add a few others at some places.
But you justified your removal of this instruction by your fear
of a potential problem caused by a but pointed out by some poorly
commented field in one struct, and left open a small window for a
change in case someone else confirmed it. We took time to explain
why this bug was even in our advantage. If you really didn't want
the HLT in the first place for your own reasons, we would have all
saved some time by neither searching info on it nor talking about
it.
I don't really mind whether the CPU really halts or not, it's just
that I find it stupid to make it spin for nothing when we know we
can avoid it. That's all.
Regards,
Willy
Ar Mer, 2006-08-30 am 22:03 +0200, ysgrifennodd Willy Tarreau:
> So, to conclude, with or without the bug, "hlt;jmp $" will halt the
> CPU as we expect it to. Therefore, I'd like you to restore it in
> the patch.
Having had a dig I would concur that we should add the "hlt" back. The
cases I can find are 386/486 "hangs if hlt executed" bugs, mostly
unresolved/chipset problems and a "hlt during IDE DMA hangs the box" bug
we work around on the Cyrix CS5510.
That said for a PIV at least "hlt" really makes little difference to the
power burn
In-Reply-To: <[email protected]>
On Wed, 30 Aug 2006 18:30:40 +0200, Andi Kleen wrote:
> It would be better to separate exceptions from interrupts here.
> A spurious interrupt is not necessarily fatal, just an exception is.
>
> But I went with the simpler patch with some changes now
> (added PANIC to the message etc.)
This is already in -mm:
================================================================================
From: Chuck Ebbert <[email protected]>
Add early i386 fault handlers with debug information for common faults.
Handles:
divide error
invalid opcode
protection fault
page fault
Also adds code to detect early recursive/multiple faults and halt the
system when they happen (taken from x86_64.)
Signed-off-by: Chuck Ebbert <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
arch/i386/kernel/head.S | 67 ++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff -puN arch/i386/kernel/head.S~i386-early-fault-handler arch/i386/kernel/head.S
--- a/arch/i386/kernel/head.S~i386-early-fault-handler
+++ a/arch/i386/kernel/head.S
@@ -371,8 +371,65 @@ rp_sidt:
addl $8,%edi
dec %ecx
jne rp_sidt
+
+.macro set_early_handler handler,trapno
+ lea \handler,%edx
+ movl $(__KERNEL_CS << 16),%eax
+ movw %dx,%ax
+ movw $0x8E00,%dx /* interrupt gate - dpl=0, present */
+ lea idt_table,%edi
+ movl %eax,8*\trapno(%edi)
+ movl %edx,8*\trapno+4(%edi)
+.endm
+
+ set_early_handler handler=early_divide_err,trapno=0
+ set_early_handler handler=early_illegal_opcode,trapno=6
+ set_early_handler handler=early_protection_fault,trapno=13
+ set_early_handler handler=early_page_fault,trapno=14
+
ret
+early_divide_err:
+ xor %edx,%edx
+ pushl $0 /* fake errcode */
+ jmp early_fault
+
+early_illegal_opcode:
+ movl $6,%edx
+ pushl $0 /* fake errcode */
+ jmp early_fault
+
+early_protection_fault:
+ movl $13,%edx
+ jmp early_fault
+
+early_page_fault:
+ movl $14,%edx
+ jmp early_fault
+
+early_fault:
+ cld
+#ifdef CONFIG_PRINTK
+ movl $(__KERNEL_DS),%eax
+ movl %eax,%ds
+ movl %eax,%es
+ cmpl $2,early_recursion_flag
+ je hlt_loop
+ incl early_recursion_flag
+ movl %cr2,%eax
+ pushl %eax
+ pushl %edx /* trapno */
+ pushl $fault_msg
+#ifdef CONFIG_EARLY_PRINTK
+ call early_printk
+#else
+ call printk
+#endif
+#endif
+hlt_loop:
+ hlt
+ jmp hlt_loop
+
/* This is the default interrupt "handler" :-) */
ALIGN
ignore_int:
@@ -386,6 +443,9 @@ ignore_int:
movl $(__KERNEL_DS),%eax
movl %eax,%ds
movl %eax,%es
+ cmpl $2,early_recursion_flag
+ je hlt_loop
+ incl early_recursion_flag
pushl 16(%esp)
pushl 24(%esp)
pushl 32(%esp)
@@ -431,9 +491,16 @@ ENTRY(stack_start)
ready: .byte 0
+early_recursion_flag:
+ .long 0
+
int_msg:
.asciz "Unknown interrupt or fault at EIP %p %p %p\n"
+fault_msg:
+ .ascii "Int %d: CR2 %p err %p EIP %p CS %p flags %p\n"
+ .asciz "Stack: %p %p %p %p %p %p %p %p\n"
+
/*
* The IDT and GDT 'descriptors' are a strange 48-bit object
* only used by the lidt and lgdt instructions. They are not
_
--
Chuck