Skip to contentSkip to conversation

exception processing in early boot

RFC PATCH In review Willy Tarreau 5 participants 19 messages Aug 30, 2006 4 messages not in the archive

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 exception processing in early boot

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,

#1 lore raw
Quoted 10 lines · Willy Tarreau

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.

Quoted 2 lines

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 ...]

Quoted 3 lines

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

#2 lore raw
Quoted 12 lines · Andi Kleen
Quoted 10 lines · Willy Tarreau

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 1 line

-Andi

Regards,
Willy

#3 lore raw
Quoted 8 lines

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.

Quoted 5 lines

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

#4 lore raw
Quoted 11 lines · Andi Kleen
Quoted 8 lines

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.

Quoted 7 lines
Quoted 5 lines

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

#5 lore raw
Quoted 4 lines · 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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw
Quoted 8 lines · Sean
Quoted 4 lines · 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.

Quoted 6 lines

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.

Quoted 1 line

Sean

Cheers,
Willy

A message that is not in the archive (sent off-list or not yet received): [email protected]

#9 lore raw
Quoted 10 lines · On Wednesday 30 August 2006 18:25, [email protected] wrote:
Quoted 7 lines · Andi Kleen
Quoted 5 lines

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.)

Quoted 33 lines

--- 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

(quoted diff) +5 -10
@@ -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

Review rp_sidt: · line 401

Quoted 10 lines
(quoted diff) +1 -7
 #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

#10 lore raw
Quoted 66 lines · Andi Kleen
Quoted 10 lines · On Wednesday 30 August 2006 18:25, [email protected] wrote:
Quoted 7 lines · Andi Kleen
Quoted 5 lines

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.)

Quoted 33 lines

--- 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

(quoted diff) +5 -10
@@ -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

Review rp_sidt: · line 401

Quoted 10 lines
(quoted diff) +1 -7
 #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.

Quoted 1 line

-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.

#11 lore raw
Quoted 4 lines

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

#12 lore raw
Quoted 6 lines · Andi Kleen
Quoted 4 lines

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.

Quoted 8 lines

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.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#14 lore raw
Quoted 5 lines · On Wednesday 30 August 2006 19:33, [email protected] wrote:
Quoted 3 lines

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#16 lore raw
Quoted 11 lines · On Wednesday 30 August 2006 20:21, [email protected] wrote:
Quoted 8 lines · Andi Kleen
Quoted 4 lines · On Wednesday 30 August 2006 19:33, [email protected] wrote:
Quoted 2 lines

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.

Quoted 8 lines
Quoted 5 lines

-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.

Quoted 1 line

ps: sorry but [email protected] bounces for me, removed from cc.

For me too.

-Andi

#17 lore raw
Quoted 7 lines · Andi Kleen
Quoted 6 lines · On Wednesday 30 August 2006 20:21, [email protected] wrote:
Quoted 5 lines · Andi Kleen
Quoted 3 lines · On Wednesday 30 August 2006 19:33, [email protected] wrote:
Quoted 2 lines

But I went with the simpler patch with some changes now (added PANIC to the message etc.)

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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

#18 lore raw
Quoted 4 lines

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

#19 lore raw
Quoted 7 lines · Andi Kleen
Quoted 4 lines

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

#20 lore raw
Quoted 11 lines · Willy Tarreau
Quoted 7 lines · Andi Kleen
Quoted 4 lines

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

#21 lore raw
Quoted 7 lines · Andi Kleen
Quoted 6 lines · Willy Tarreau
Quoted 5 lines · Andi Kleen
Quoted 4 lines

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

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

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

#22 lore raw
In reply to Willy Tarreau

Ar Mer, 2006-08-30 am 22:03 +0200, ysgrifennodd Willy Tarreau:

Quoted 3 lines

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