Re: [Qemu-devel] [PATCH] x86: Reboot CPU on triple fault - Version 6

[Anthony Liguori <anthony@codemonkey.ws>]

Kevin Wolf wrote:
> Quite a few rounds for such a short patch. But surely a CPU dump without
> any explanation isn't optimal. We should put a "CPU Reset" line in the
> log before the dump.
>
> Signed-off-by: Kevin Wolf <kwolf@suse.de>
>   
> Index: dyngen-exec.h
> ===================================================================
> --- dyngen-exec.h	(Revision 4215)
> +++ dyngen-exec.h	(Arbeitskopie)
> @@ -86,6 +86,7 @@
>  extern int fprintf(FILE *, const char *, ...);
>  extern int fputs(const char *, FILE *);
>  extern int printf(const char *, ...);
> +extern FILE *stderr;
>   

This is unnecessary.

>  #undef NULL
>  #define NULL 0
>  
> Index: exec.c
> ===================================================================
> --- exec.c	(Revision 4215)
> +++ exec.c	(Arbeitskopie)
> @@ -1259,6 +1259,8 @@
>  #ifdef TARGET_I386
>      { CPU_LOG_PCALL, "pcall",
>        "show protected mode far calls/returns/exceptions" },
> +    { CPU_LOG_RESET, "cpu_reset",
> +      "show CPU state before CPU resets" },
>  #endif
>  #ifdef DEBUG_IOPORT
>      { CPU_LOG_IOPORT, "ioport",
> Index: target-i386/helper.c
> ===================================================================
> --- target-i386/helper.c	(Revision 4215)
> +++ target-i386/helper.c	(Arbeitskopie)
> @@ -1231,6 +1231,9 @@
>      }
>  }
>  
> +/* This should come from sysemu.h - if we could include it here... */
> +void qemu_system_reset_request(void);
> +
>  /*
>   * Check nested exceptions and change to double or triple fault if
>   * needed. It should only be called, if this is not an interrupt.
> @@ -1248,9 +1251,20 @@
>          fprintf(logfile, "check_exception old: %x new %x\n",
>                  env->old_exception, intno);
>  
> -    if (env->old_exception == EXCP08_DBLE)
> -        cpu_abort(env, "triple fault");
> +    if (env->old_exception == EXCP08_DBLE) {
> +        if(env->intercept & INTERCEPT_SVM_MASK) {
> +            /* most probably the virtual machine should not
> +               be shut down but rather caught by the VMM */
> +            vmexit(SVM_EXIT_SHUTDOWN, 0);
> +        }
>  
> +        if (loglevel & CPU_LOG_RESET)
> +            fprintf(logfile, "Triple fault\n");
> +
> +        qemu_system_reset_request();
>   

This isn't the right function to use here.  If we supported ACPI 
shutdown, this would generate an ACPI shutdown request.  You probably 
want to just do:

cpu_interrupt(env, CPU_INTERRUPT_EXIT);

Regards,

Anthony Liguori

> +        return EXCP_HLT;
> +    }
> +
>      if ((first_contributory && second_contributory)
>          || (env->old_exception == EXCP0E_PAGE &&
>              (second_contributory || (intno == EXCP0E_PAGE)))) {
> Index: target-i386/helper2.c
> ===================================================================
> --- target-i386/helper2.c	(Revision 4215)
> +++ target-i386/helper2.c	(Arbeitskopie)
> @@ -362,6 +362,11 @@
>  void cpu_reset(CPUX86State *env)
>  {
>      int i;
> +        
> +    if (loglevel & CPU_LOG_RESET) {
> +        fprintf(logfile, "CPU Reset (CPU %d)\n", env->cpu_index);
> +        cpu_dump_state(env, logfile, fprintf, X86_DUMP_FPU | X86_DUMP_CCOP);
> +    }
>  
>      memset(env, 0, offsetof(CPUX86State, breakpoints));
>  
> Index: cpu-all.h
> ===================================================================
> --- cpu-all.h	(Revision 4215)
> +++ cpu-all.h	(Arbeitskopie)
> @@ -779,6 +779,7 @@
>  #define CPU_LOG_PCALL      (1 << 6)
>  #define CPU_LOG_IOPORT     (1 << 7)
>  #define CPU_LOG_TB_CPU     (1 << 8)
> +#define CPU_LOG_RESET      (1 << 9)
>  
>  /* define log items */
>  typedef struct CPULogItem {
>