2008-01-11 06:07:21

by Roland McGrath

[permalink] [raw]
Subject: [PATCH] x86_64 early_idt_handler improvements

It's not too pretty, but I found this made the "PANIC: early exception"
messages become much more reliably useful: 1. print the vector number,
2. print the %cs value, 3. handle error-code-pushing vs non-pushing vectors.

Signed-off-by: Roland McGrath <[email protected]>
---
arch/x86/kernel/head64.c | 6 ++++--
arch/x86/kernel/head_64.S | 34 ++++++++++++++++++++++++++++++----
2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 4a1c135..0000000 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -48,6 +48,8 @@ static void __init copy_bootdata(char *r
}
}

+extern const char early_idt_handlers[IDT_ENTRIES][10];
+
void __init x86_64_start_kernel(char * real_mode_data)
{
int i;
@@ -59,7 +61,7 @@ void __init x86_64_start_kernel(char * r
zap_identity_mappings();

for (i = 0; i < IDT_ENTRIES; i++)
- set_intr_gate(i, early_idt_handler);
+ set_intr_gate(i, &early_idt_handlers[i]);
load_idt((const struct desc_ptr *)&idt_descr);

early_printk("Kernel alive\n");
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index c31b1c9..0000000 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -267,14 +267,40 @@ init_rsp:
bad_address:
jmp bad_address

+.macro early_idt_tramp first, last
+ .ifgt \last-\first
+ early_idt_tramp \first, \last-1
+ .endif
+ movl $\last,%esi
+ jmp early_idt_handler
+.endm
+
+ .globl early_idt_handlers
+early_idt_handlers:
+ early_idt_tramp 0, 63
+ early_idt_tramp 64, 127
+ early_idt_tramp 128, 191
+ early_idt_tramp 192, 255
+
ENTRY(early_idt_handler)
cmpl $2,early_recursion_flag(%rip)
jz 1f
incl early_recursion_flag(%rip)
- xorl %eax,%eax
- movq 8(%rsp),%rsi # get rip
- movq (%rsp),%rdx
GET_CR2_INTO_RCX
+ movq %rcx,%r9
+ xorl %r8d,%r8d # zero for error code
+ movl %esi,%ecx # get vector number
+ # Test %ecx against mask of vectors that push error code.
+ cmpl $31,%ecx
+ ja 0f
+ movl $1,%eax
+ salq %cl,%rax
+ testl $0x27d00,%eax
+ je 0f
+ popq %r8 # get error code
+0: movq 0(%rsp),%rcx # get ip
+ movq 8(%rsp),%rdx # get cs
+ xorl %eax,%eax
leaq early_idt_msg(%rip),%rdi
call early_printk
cmpl $2,early_recursion_flag(%rip)
@@ -291,7 +317,7 @@ early_recursion_flag:
.long 0

early_idt_msg:
- .asciz "PANIC: early exception rip %lx error %lx cr2 %lx\n"
+ .asciz "PANIC: early exception %02lx rip %lx:%lx error %lx cr2 %lx\n"
early_idt_ripmsg:
.asciz "RIP %s\n"


2008-01-11 06:27:18

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86_64 early_idt_handler improvements

Roland McGrath <[email protected]> writes:

> It's not too pretty, but I found this made the "PANIC: early exception"
> messages become much more reliably useful: 1. print the vector number,
> 2. print the %cs value, 3. handle error-code-pushing vs non-pushing vectors.

For what do you need cs? It should be always the same for early boot.

>
> Signed-off-by: Roland McGrath <[email protected]>
> ---
> arch/x86/kernel/head64.c | 6 ++++--
> arch/x86/kernel/head_64.S | 34 ++++++++++++++++++++++++++++++----
> 2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 4a1c135..0000000 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -48,6 +48,8 @@ static void __init copy_bootdata(char *r
> }
> }
>
> +extern const char early_idt_handlers[IDT_ENTRIES][10];

I don't think that minor improvement is worth that much memory
(several KB for the array and some more for all the handlers)

Also in my experience it is not that difficult to work out from
which vector the exception came from. There are not that many variants
anyways; it's near always #GP or #PF.

-Andi

2008-01-11 07:07:33

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] x86_64 early_idt_handler improvements

Probably true that it's not worth the space. I couldn't figure a way to do
it that doesn't use at least 8 bytes per vector. Of course, the vectors
>= 32 are never interesting, so 256 bytes of dispatch table could cover it.

Anyway, I think it's fine just to have the patch on hand to enable for a
particular debugging session, which is what I wrote it for. Maybe it could
be a config option.


Thanks,
Roland

2008-01-11 07:39:19

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86_64 early_idt_handler improvements


* Roland McGrath <[email protected]> wrote:

> It's not too pretty, but I found this made the "PANIC: early
> exception" messages become much more reliably useful: 1. print the
> vector number, 2. print the %cs value, 3. handle error-code-pushing vs
> non-pushing vectors.

thanks, applied.

since these printouts are only useful if we have early-printk
capabilities enabled, i made it dependent on CONFIG_EARLY_PRINTK.

I also made EARLY_PRINTK selectable on 64-bit as well. (for some unknown
reason it was unconditionally built into 64-bit (unlike 32-bit where it
is selectable on EMBEDDED))

Ingo