Re: [PATCH] x86: optionally show last exception from/to register contents

[Andi Kleen <ak@suse.de>]

On Mon, Aug 13, 2007 at 12:33:05PM +0100, Jan Beulich wrote:


>  
>  	if (cpu_has_ds) {
>  		unsigned int l1;
> --- linux-2.6.23-rc3/arch/i386/kernel/traps.c	2007-08-13 08:59:45.000000000 +0200
> +++ 2.6.23-rc3-x86-ler/arch/i386/kernel/traps.c	2007-08-07 10:42:55.000000000 +0200
> @@ -321,6 +321,13 @@ void show_registers(struct pt_regs *regs
>  		unsigned int code_len = code_bytes;
>  		unsigned char c;
>  
> +		if (__get_cpu_var(ler_msr)) {
> +			u32 from, to, hi;
> +
> +			rdmsr(__get_cpu_var(ler_msr), from, hi);
> +			rdmsr(__get_cpu_var(ler_msr) + 1, to, hi);
> +			printk("LER: %08x -> %08x\n", from, to);
> +		}

This seems racy -- AFAIK the MSR will record the last branch
before an interrupt too, and the trap handlers enable interrupts
before coming here. 

Can't think of a good way to avoid that for page fault at least
without impacting interrupt latency or reading the MSR always.

The other thing is that this should use print_symbol()

>  		printk("\n" KERN_EMERG "Stack: ");
>  		show_stack_log_lvl(NULL, regs, (unsigned long *)esp, KERN_EMERG);
>  
> @@ -360,6 +367,18 @@ int is_valid_bugaddr(unsigned long eip)
>  	return ud2 == 0x0b0f;
>  }
>  
> +DEFINE_PER_CPU(u32, ler_msr);
> +int ler_enabled = 0;
> +
> +void ler_enable(void) {
> +	if (__get_cpu_var(ler_msr)) {
> +		u64 debugctl;
> +
> +		rdmsrl(MSR_IA32_DEBUGCTLMSR, debugctl);
> +		wrmsrl(MSR_IA32_DEBUGCTLMSR, debugctl | 1);

Better use checking_rd/wrmsrl and disable on fail -- often emulators 
fake specific CPUs, but don't implement all debugging features.

> +static int __init ler_setup(char *s)
> +{
> +	ler_enabled = 1;
> +	return 1;
> +}
> +__setup("ler", ler_setup);

I don't think the option is very useful.

BTW I have a patch somewhere to save/report last branch on exception
too, but it never seemed suitable for mainline because of its
high overhead. If someone submitted a full BTS driver I would
be tempted though :)

-Andi