Re: [PATCH] x86/traps: Remove redundant trapnr parameter from fatal_trap()

[Don Slutz <dslutz@verizon.com>]

On 09/01/14 06:06, Andrew Cooper wrote:
> It is always available via regs->entry_vector.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Jan Beulich <JBeulich@suse.com>
> ---
>   xen/arch/x86/cpu/mcheck/mce.c   |    2 +-
>   xen/arch/x86/nmi.c              |    2 +-
>   xen/arch/x86/traps.c            |   13 +++++++------
>   xen/arch/x86/x86_64/entry.S     |    3 +--
>   xen/include/asm-x86/processor.h |    2 +-
>   5 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
> index 812daf6..05a86fb 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -74,7 +74,7 @@ static void unexpected_machine_check(const struct cpu_user_regs *regs)
>   {
>       console_force_unlock();
>       printk("Unexpected Machine Check Exception\n");
> -    fatal_trap(TRAP_machine_check, regs);
> +    fatal_trap(regs);
>   }
>   
>   
> diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
> index 7d15d5b..055f4ef 100644
> --- a/xen/arch/x86/nmi.c
> +++ b/xen/arch/x86/nmi.c
> @@ -472,7 +472,7 @@ bool_t nmi_watchdog_tick(const struct cpu_user_regs *regs)
>               console_force_unlock();
>               printk("Watchdog timer detects that CPU%d is stuck!\n",
>                      smp_processor_id());
> -            fatal_trap(TRAP_nmi, regs);
> +            fatal_trap(regs);
>           }
>       }
>       else
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 7f5306f..10fc2ca 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -394,9 +394,10 @@ static const char *trapstr(unsigned int trapnr)
>    * are disabled). In such situations we can't do much that is safe. We try to
>    * print out some tracing and then we just spin.
>    */
> -void fatal_trap(int trapnr, const struct cpu_user_regs *regs)
> +void fatal_trap(const struct cpu_user_regs *regs)
>   {
>       static DEFINE_PER_CPU(char, depth);
> +    unsigned int trapnr = regs->entry_vector;

Should this be:

unsigned int trapnr = regs->entry_vector & ~(TRAP_regs_partial|TRAP_syscall);


To get rid of the extra bits?

   -Don Slutz

>   
>       /* Set AC to reduce chance of further SMAP faults */
>       stac();
> @@ -1427,7 +1428,7 @@ void do_page_fault(struct cpu_user_regs *regs)
>           {
>               console_start_sync();
>               printk("Xen SM%cP violation\n", (pf_type == smep_fault) ? 'E' : 'A');
> -            fatal_trap(TRAP_page_fault, regs);
> +            fatal_trap(regs);
>           }
>   
>           if ( pf_type != real_fault )
> @@ -1498,7 +1499,7 @@ void __init do_early_page_fault(struct cpu_user_regs *regs)
>           console_start_sync();
>           printk("Early fatal page fault at %04x:%p (cr2=%p, ec=%04x)\n",
>                  regs->cs, _p(regs->eip), _p(cr2), regs->error_code);
> -        fatal_trap(TRAP_page_fault, regs);
> +        fatal_trap(regs);
>       }
>   }
>   
> @@ -3256,7 +3257,7 @@ static void pci_serr_error(const struct cpu_user_regs *regs)
>       default:  /* 'fatal' */
>           console_force_unlock();
>           printk("\n\nNMI - PCI system error (SERR)\n");
> -        fatal_trap(TRAP_nmi, regs);
> +        fatal_trap(regs);
>       }
>   }
>   
> @@ -3271,7 +3272,7 @@ static void io_check_error(const struct cpu_user_regs *regs)
>       default:  /* 'fatal' */
>           console_force_unlock();
>           printk("\n\nNMI - I/O ERROR\n");
> -        fatal_trap(TRAP_nmi, regs);
> +        fatal_trap(regs);
>       }
>   
>       outb((inb(0x61) & 0x0f) | 0x08, 0x61); /* clear-and-disable IOCK */
> @@ -3291,7 +3292,7 @@ static void unknown_nmi_error(const struct cpu_user_regs *regs, unsigned char re
>           console_force_unlock();
>           printk("Uhhuh. NMI received for unknown reason %02x.\n", reason);
>           printk("Do you have a strange power saving mode enabled?\n");
> -        fatal_trap(TRAP_nmi, regs);
> +        fatal_trap(regs);
>       }
>   }
>   
> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
> index a3ed216..42835d0 100644
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -536,8 +536,7 @@ exception_with_ints_disabled:
>   
>   /* No special register assumptions. */
>   FATAL_exception_with_ints_disabled:
> -        movzbl UREGS_entry_vector(%rsp),%edi
> -        movq  %rsp,%rsi
> +        movq  %rsp,%rdi
>           call  fatal_trap
>           ud2
>   
> diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
> index a156e01..9e1f210 100644
> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -481,7 +481,7 @@ void show_registers(const struct cpu_user_regs *regs);
>   void show_execution_state(const struct cpu_user_regs *regs);
>   #define dump_execution_state() run_in_exception_handler(show_execution_state)
>   void show_page_walk(unsigned long addr);
> -void noreturn fatal_trap(int trapnr, const struct cpu_user_regs *regs);
> +void noreturn fatal_trap(const struct cpu_user_regs *regs);
>   
>   void compat_show_guest_stack(struct vcpu *v,
>                                const struct cpu_user_regs *regs, int lines);