Reboot CPU on triple fault
PATCH 1/1 Reboot CPU on triple fault
Hi,
On i386/x86-64, CPU must reboot when a triple fault is detected. Attached patch implements this behaviour.
Hervé
Index: target-i386/helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-i386/helper.c,v retrieving revision 1.103
helper.c +10 -0
@@ -1276,6 +1276,11 @@ {if (!is_int) {svm_check_intercept_param (SVM_EXIT_EXCP_BASE + intno, error_code); +if (env->old_exception ==EXCP08_DBLE ) { +cpu_loop_exit (); +cpu_reset (env); +return ; + } intno =check_exception (intno, &error_code); }
@@ -1289,6 +1294,11 @@/* same as raise_exception_err, but do not restore global registers */ static void raise_exception_err_norestore (int exception_index,int error_code) { +if (env->old_exception ==EXCP08_DBLE ) { +cpu_loop_exit (); +cpu_reset (env); +return ; + } exception_index =check_exception (exception_index, &error_code); env->exception_index = exception_index;
Quoted 4 lines · Hervé Poussineau
Hi,
On i386/x86-64, CPU must reboot when a triple fault is detected. Attached patch implements this behaviour.
So what exactly does check_exception() in target-i386/helper.c do then? Should the cpu_reset call go in there?
Quoted 10 lines · Alexander Graf
Quoted 4 lines · Hervé Poussineau
Hi,
On i386/x86-64, CPU must reboot when a triple fault is detected. Attached patch implements this behaviour.
So what exactly does
check_exception()intarget-i386/helper.cdo then? Should thecpu_resetcall go in there?
It definitely should. Besides, I'd really like to have the CPU dump on triple faults retained. In most cases, this will be a lot more useful than a silent reboot.
Kevin
Quoted 5 lines · Kevin Wolf
Quoted 4 lines · Alexander Graf
Quoted 2 lines · Hervé Poussineau
Hi,
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
On i386/x86-64, CPU must reboot when a triple fault is detected. Attached patch implements this behaviour.
So what exactly does
check_exception()intarget-i386/helper.cdo then? Should thecpu_resetcall go in there?It definitely should. Besides, I'd really like to have the CPU dump on triple faults retained. In most cases, this will be a lot more useful than a silent reboot.
As the same requirement came up here, I worked out the following patch. I feel a bit uneasy about it because
o I'm unsure if breaking out of the exception loop is OK this way.
o including necessary headers fails, mostly due to stdio redefinitions in dyngen-exec.h.
This patch behaves as it should, but only in one specific test case. Feedback is welcome.
(BTW, the original patch didn't work at all.)
--- dyngen-exec.h | 1 + target-i386/helper.c | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)
dyngen-exec.h +1 -0
@@ -86,6 +86,7 @@ typedef struct FILE FILE;extern int fprintf (FILE *,const char *, ...);extern int fputs (const char *,FILE *);extern int printf (const char *, ...); +extern FILE *stderr;#undef NULL#define NULL 0
target-i386/helper.c +17 -2
@@ -1231,6 +1231,8 @@ void do_interrupt(int intno, int is_int, } } +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,8 +1250,21 @@ static int check_exception(int intno, infprintf (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 ) { +fprintf (stderr,"qemu: warning: triple fault\n" ); +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 ); + } +cpu_dump_state (env, stderr, fprintf,X86_DUMP_FPU |X86_DUMP_CCOP ); +if (logfile) { +fprintf (logfile,"qemu: warning: triple fault\n" ); +cpu_dump_state (env, logfile, fprintf,X86_DUMP_FPU |X86_DUMP_CCOP ); + } +qemu_system_reset_request (); +return 0 ; + } if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&
target-i386/helper.c static int check_exception(int intno, in · line 1254
Quoted 2 lines
target-i386/helper.c +2
-0+if (env->old_exception ==EXCP08_DBLE ) { +fprintf (stderr,"qemu: warning: triple fault\n" );
IMHO There's no reason to print a message to stderr. This is all well defined behavior, and the accepted way of exiting from 286 protected mode.
Paul
Quoted 10 lines · Jan Kiszka
As the same requirement came up here, I worked out the following patch. I feel a bit uneasy about it because
o I'm unsure if breaking out of the exception loop is OK this way.
o including necessary headers fails, mostly due to stdio redefinitions in
dyngen-exec.h.This patch behaves as it should, but only in one specific test case. Feedback is welcome.
This is exactly the behaviour I'd like to have. I still needed a small change to your patch, though: You shouldn't return 0 from check_exception, after all it's not a divide error. I'm not sure if EXCP_HLT is the right thing to return here, but the attached patch works for me. Without this change it kept hanging in a loop forever writing more dumps than I ever wanted. ;-)
dyngen-exec.h +1 -0
@@ -86,6 +86,7 @@extern int fprintf (FILE *,const char *, ...);extern int fputs (const char *,FILE *);extern int printf (const char *, ...); +extern FILE *stderr;#undef NULL#define NULL 0
helper.c +17 -2
@@ -1231,6 +1231,8 @@ } } +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,8 +1250,21 @@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 ) { +fprintf (stderr,"qemu: warning: triple fault\n" ); +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 ); + } +cpu_dump_state (env, stderr, fprintf,X86_DUMP_FPU |X86_DUMP_CCOP ); +if (logfile) { +fprintf (logfile,"qemu: warning: triple fault\n" ); +cpu_dump_state (env, logfile, fprintf,X86_DUMP_FPU |X86_DUMP_CCOP ); + } +qemu_system_reset_request (); +return EXCP_HLT ; + } if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&
Quoted 5 lines · Paul Brook
target-i386/helper.c static int check_exception(int intno, in · line 1254
Quoted 2 lines
target-i386/helper.c +2
-0+if (env->old_exception ==EXCP08_DBLE ) { +fprintf (stderr,"qemu: warning: triple fault\n" );IMHO There's no reason to print a message to stderr. This is all well defined behavior, and the accepted way of exiting from 286 protected mode.
How many users does qemu have who need triple faults to exit from 286 Protected Mode? And how many users does it have who don't use triple faults (yes, it's called a fault, not a PM exiting feature) intentionally but might want the message and dumps for debugging? Personally I suspect that a great majority belongs to the latter case.
And honestly, a message on stderr really shouldn't hurt those 286 folks. They are even getting the reset they want from this patch.
Kevin
Quoted 6 lines · Kevin Wolf
Quoted 3 lines
IMHO There's no reason to print a message to stderr. This is all well defined behavior, and the accepted way of exiting from 286 protected mode.
How many users does qemu have who need triple faults to exit from 286 Protected Mode?
Not many, only those users running old MS-DOS apps / OSes.
Quoted 2 lines
And how many users does it have who don't use triple faults (yes, it's called a fault, not a PM exiting feature)
Faults don't mean errors. Think about page faults.
Quoted 1 line
And honestly, a message on stderr really shouldn't hurt those 286 folks.
For old MS-DOS apps / OSes, I have the impression this can happen hundreds of times per second. It's part of task context switching and BIOS calls.
-- Jamie
Am Mittwoch, 16. April 2008 11.23:40 schrieb Jamie Lokier:
Quoted 8 lines
Quoted 6 lines · Kevin Wolf
Quoted 3 lines
IMHO There's no reason to print a message to stderr. This is all well defined behavior, and the accepted way of exiting from 286 protected mode.
How many users does qemu have who need triple faults to exit from 286 Protected Mode?
Quoted 6 lines
Not many, only those users running old MS-DOS apps / OSes.
Quoted 2 lines
And how many users does it have who don't use triple faults (yes, it's called a fault, not a PM exiting feature)
Faults don't mean errors. Think about page faults.
I agree with you, but in that specific case, its definitively an Error if it isn't catched earlier.
Quoted 5 lines
Quoted 1 line
And honestly, a message on stderr really shouldn't hurt those 286 folks.
For old MS-DOS apps / OSes, I have the impression this can happen hundreds of times per second. It's part of task context switching and BIOS calls.
If this is really a problem of performance, I would suggest to add a commandline switch to disable the error message.
This error message should not be removed imho. Because for those people who use it for debugging (I'm one of them), it's really nice. This message makes it possible for beginners to see, what went wrong, without further debugging.
Quoted 8 lines · Jamie Lokier
Quoted 5 lines · Kevin Wolf
Quoted 3 lines
IMHO There's no reason to print a message to stderr. This is all well defined behavior, and the accepted way of exiting from 286 protected mode.
How many users does qemu have who need triple faults to exit from 286 Protected Mode?
Not many, only those users running old MS-DOS apps / OSes.
MS-DOS is running in Real Mode, you'd additionally need an application which uses 286 PM. Apparently there aren't too many users relying on triple faults as in the past qemu aborted (or in earlier versions even hung) on triple faults and nobody complained.
Quoted 4 lines
Quoted 2 lines
And how many users does it have who don't use triple faults (yes, it's called a fault, not a PM exiting feature)
Faults don't mean errors. Think about page faults.
Ok, you're right on my wording. But double faults (which always occur before a triple fault) are in fact despite their name not faults, but aborts. Just to reword it using the correct terminology. ;-)
Quoted 5 lines
Quoted 1 line
And honestly, a message on stderr really shouldn't hurt those 286 folks.
For old MS-DOS apps / OSes, I have the impression this can happen hundreds of times per second. It's part of task context switching and BIOS calls.
I have to admit that I'm all but an 286 PM expert. What exactly is a triple fault doing there? If it leads to a reset as implemented here, I can hardly imagine that it is of much use in regular context switching.
Kevin
Quoted 7 lines · Kevin Wolf
Quoted 3 lines
For old MS-DOS apps / OSes, I have the impression this can happen hundreds of times per second. It's part of task context switching and BIOS calls.
I have to admit that I'm all but an 286 PM expert. What exactly is a triple fault doing there? If it leads to a reset as implemented here, I can hardly imagine that it is of much use in regular context switching.
It is required due to the "feature" of 286 CPU. Once 286 enters protected mode, it cannot be switched back to the real mode. The only way to perform it is resetting the CPU. A software which tries to go back from protected to real mode sets the warm-reset vector in BIOS memory area, sets the reboot reason variable in CMOS memory and issues a CPU reset.
CPU may be resetted due to a triple fault :)
Signature · 1 line
Michal Schulz
Quoted 10 lines · Kevin Wolf
Quoted 5 lines · Jamie Lokier
Quoted 1 line · Kevin Wolf
And honestly, a message on stderr really shouldn't hurt those 286 folks.
For old MS-DOS apps / OSes, I have the impression this can happen hundreds of times per second. It's part of task context switching and BIOS calls.
I have to admit that I'm all but an 286 PM expert. What exactly is a triple fault doing there? If it leads to a reset as implemented here, I can hardly imagine that it is of much use in regular context switching.
According to my current information, the triple fault will make the processor issue a shutdown cycle on its external bus. This is obviously detected by many PC boards, which then issue a hard reset.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Quoted 11 lines · Michal Schulz
Quoted 3 lines
I have to admit that I'm all but an 286 PM expert. What exactly is a triple fault doing there? If it leads to a reset as implemented here, I can hardly imagine that it is of much use in regular context switching.
It is required due to the "feature" of 286 CPU. Once 286 enters protected mode, it cannot be switched back to the real mode. The only way to perform it is resetting the CPU. A software which tries to go back from protected to real mode sets the warm-reset vector in BIOS memory area, sets the reboot reason variable in CMOS memory and issues a CPU reset.
CPU may be resetted due to a triple fault :)
and some example code using it (with comments):
http://www.x86.org/ftp/source/3fault/reset.asm
Signature · 1 line
Michal Schulz
Quoted 6 lines · Kevin Wolf
Quoted 1 line
Not many, only those users running old MS-DOS apps / OSes.
MS-DOS is running in Real Mode, you'd additionally need an application which uses 286 PM. Apparently there aren't too many users relying on triple faults as in the past qemu aborted (or in earlier versions even hung) on triple faults and nobody complained.
That's true. I think they use Bochs instead.
Quoted 7 lines
Quoted 3 lines
For old MS-DOS apps / OSes, I have the impression this can happen hundreds of times per second. It's part of task context switching and BIOS calls.
I have to admit that I'm all but an 286 PM expert. What exactly is a triple fault doing there? If it leads to a reset as implemented here, I can hardly imagine that it is of much use in regular context switching.
CPU reset is the only way to exit 286 protected mode, and that is needed when context switching between real and protected mode tasks.
So apps set a byte in the CMOS NVRAM, trigger a processor reset, then the BIOS reset code immediately jumps back to the app in real mode, avoiding the normal reboot process.
It's slow and silly, but faster than the other method, asking the keyboard controller to do it.
-- Jamie
Quoted 12 lines · Kevin Wolf
Quoted 10 lines · Jan Kiszka
As the same requirement came up here, I worked out the following patch. I feel a bit uneasy about it because
o I'm unsure if breaking out of the exception loop is OK this way.
o including necessary headers fails, mostly due to stdio redefinitions in
dyngen-exec.h.This patch behaves as it should, but only in one specific test case. Feedback is welcome.
Quoted 6 lines
This is exactly the behaviour I'd like to have. I still needed a small change to your patch, though: You shouldn't return 0 from
check_exception, after all it's not a divide error. I'm not sure ifEXCP_HLTis the right thing to return here, but the attached patch works for me. Without this change it kept hanging in a loop forever writing more dumps than I ever wanted. ;-)
Your version still works for me (and makes more sense than return 0, indeed). So we just need someone to confirm that this is actually a clean way to leave the fault handling.
And we need a consensus regarding fault reporting... What about confining the dump to the logfile, leaving the console alone?
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Quoted 23 lines · Jan Kiszka
Quoted 13 lines · Kevin Wolf
Quoted 11 lines · Jan Kiszka
As the same requirement came up here, I worked out the following patch. I feel a bit uneasy about it because
o I'm unsure if breaking out of the exception loop is OK this way.
o including necessary headers fails, mostly due to stdio redefinitions in
dyngen-exec.h.This patch behaves as it should, but only in one specific test case. Feedback is welcome.
Quoted 7 lines
This is exactly the behaviour I'd like to have. I still needed a small change to your patch, though: You shouldn't return 0 from
check_exception, after all it's not a divide error. I'm not sure ifEXCP_HLTis the right thing to return here, but the attached patch works for me. Without this change it kept hanging in a loop forever writing more dumps than I ever wanted. ;-)
Quoted 7 lines
Your version still works for me (and makes more sense than return 0, indeed). So we just need someone to confirm that this is actually a clean way to leave the fault handling.
And we need a consensus regarding fault reporting... What about confining the dump to the logfile, leaving the console alone?
If logging is really necessary, have it disabled by default and add a switch to enable it. This way ordinary (non-developers) don't need to worry about it, and developers can conveniently see it on stdout.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 3 lines · Michal Schulz
A software which tries to go back from protected to
real mode sets the warm-reset vector in BIOS memory area, sets the reboot reason variable in CMOS memory and issues a CPU reset.
Oh, I see. The modification of the BIOS memory area is the piece I missed. Now this makes much more sense.
Still the questions remains what to do with triple faults wrt logging. I understand that in 286 PM there might be masses of faults which nobody is interested in. On the other hand, I'd really hate to have no output when my code crashes with a triple fault and I haven't enabled -d int.
Maybe we can check if this vector has been modified? If not, the triple fault certainly wouldn't be an intended usage of a "feature".
Kevin
Quoted 2 lines · Jan Kiszka
And we need a consensus regarding fault reporting... What about confining the dump to the logfile, leaving the console alone?
Seems that the message is a big problem now, even if intentional triple-faulting didn't work in the past and caused qemu to abort. So yes, maybe the logfile must be enough.
But we definitely need a new -d option then so that a developer can always run qemu with this option. The existing ones (most notably -d int where this one would belong logically) are producing way too much output to have them always enabled.
Kevin
PATCH 1/1 x86: Reboot CPU on triple fault - Version 3
In reply to Jan KiszkaHere comes a version that may hopefully make everyone happy: :)
Reset on triple fault, but only dump the CPU state to stderr and logfile if -triple-fault was given as command line option.
--- dyngen-exec.h | 1 + target-i386/helper.c | 24 ++++++++++++++++++++++-- vl.c | 9 +++++++++ 3 files changed, 32 insertions(+), 2 deletions(-)
dyngen-exec.h +1 -0
@@ -86,6 +86,7 @@ typedef struct FILE FILE;extern int fprintf (FILE *,const char *, ...);extern int fputs (const char *,FILE *);extern int printf (const char *, ...); +extern FILE *stderr;#undef NULL#define NULL 0
target-i386/helper.c +22 -2
@@ -1231,6 +1231,10 @@ void do_interrupt(int intno, int is_int, } } +/* This should come from sysemu.h - if we could include it here... */ +void qemu_system_reset_request (void ); +extern int warn_on_triple_fault; +/* * Check nested exceptions and change to double or triple fault if * needed. It should only be called, if this is not an interrupt.
@@ -1248,8 +1252,24 @@ static int check_exception(int intno, infprintf (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 (warn_on_triple_fault) { +fprintf (stderr,"qemu: warning: triple fault\n" ); +cpu_dump_state (env, stderr, fprintf,X86_DUMP_FPU |X86_DUMP_CCOP ); +if (logfile) { +fprintf (logfile,"qemu: warning: triple fault\n" ); +cpu_dump_state (env, logfile, fprintf, +X86_DUMP_FPU |X86_DUMP_CCOP ); + } + } +qemu_system_reset_request (); +return EXCP_HLT ; + } if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&
vl.c +9 -0
@@ -200,6 +200,7 @@ CharDriverState *serial_hds[MAX_SERIAL_PCharDriverState *parallel_hds[MAX_PARALLEL_PORTS ];#ifdef TARGET_I386 int win2k_install_hack =0 ; +int warn_on_triple_fault =0 ;#endif int usb_enabled =0 ;static VLANState *first_vlan;
@@ -7730,6 +7731,7 @@ static void help(int exitcode)"-std-vga simulate a standard VGA card with VESA Bochs Extensions\n" " (default is CL-GD5446 PCI VGA)\n" "-no-acpi disable ACPI\n" + "-triple-fault enable CPU state dump on triple fault\n"#endif #ifdef CONFIG_CURSES "-curses use a curses/ncurses interface instead of SDL\n"
@@ -7852,6 +7854,7 @@ enum { QEMU_OPTION_old_param, QEMU_OPTION_clock, QEMU_OPTION_startdate, + QEMU_OPTION_triple_fault, }; typedef struct QEMUOption {
@@ -7964,6 +7967,9 @@ const QEMUOption qemu_options[] = {#endif { "clock", HAS_ARG, QEMU_OPTION_clock }, { "startdate", HAS_ARG, QEMU_OPTION_startdate }, +#if defined(TARGET_I386) + { "triple-fault", 0, QEMU_OPTION_triple_fault }, +#endif { NULL }, };
@@ -8702,6 +8708,9 @@ int main(int argc, char **argv)case QEMU_OPTION_win2k_hack: win2k_install_hack =1 ;break ; +case QEMU_OPTION_triple_fault: + warn_on_triple_fault =1 ; +break ;#endif #ifdef USE_KQEMU case QEMU_OPTION_no_kqemu:
Quoted 4 lines · Kevin Wolf
But we definitely need a new -d option then so that a developer can always run qemu with this option. The existing ones (most notably -d int where this one would belong logically) are producing way too much output to have them always enabled.
How about -d cpu_reset? (As opposed to whole system reset).
These don't happen too often, normally.
-- Jamie
PATCH 1/1 x86: Reboot CPU on triple fault - Version 4
In reply to Jan KiszkaAnd another version which implements the -d cpu_reset suggested by Jamie. Extending -d is probably better than introducing a completely new option.
dyngen-exec.h +1 -0
@@ -86,6 +86,7 @@extern int fprintf (FILE *,const char *, ...);extern int fputs (const char *,FILE *);extern int printf (const char *, ...); +extern FILE *stderr;#undef NULL#define NULL 0
exec.c +2 -0
@@ -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",
helper.c +17 -2
@@ -1231,6 +1231,10 @@ } } +/* This should come from sysemu.h - if we could include it here... */ +void qemu_system_reset_request (void ); +extern int warn_on_triple_fault; +/* * Check nested exceptions and change to double or triple fault if * needed. It should only be called, if this is not an interrupt.
@@ -1248,8 +1252,19 @@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 (stderr,"qemu: warning: triple fault\n" ); +fprintf (logfile,"qemu: warning: triple fault\n" ); + } +qemu_system_reset_request (); +return EXCP_HLT ; + } if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&
helper2.c +4 -0
@@ -362,6 +362,10 @@void cpu_reset (CPUX86State *env) {int i; + +if (loglevel &CPU_LOG_RESET ) { +cpu_dump_state (env, logfile, fprintf,X86_DUMP_FPU |X86_DUMP_CCOP ); + }memset (env,0 , offsetof(CPUX86State , breakpoints));
cpu-all.h +1 -0
@@ -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 {
[Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
Quoted 3 lines · Kevin Wolf
And another version which implements the -d
cpu_resetsuggested by Jamie. Extending -d is probably better than introducing a completely new option.
Sounds good.
Quoted 33 lines
Signed-off-by Kevin Wolf <[email protected]>Index:
dyngen-exec.h
===================================================================dyngen-exec.h +1
-0@@ -86,6 +86,7 @@extern int fprintf (FILE *,const char *, ...);extern int fputs (const char *,FILE *);extern int printf (const char *, ...); +extern FILE *stderr;#undef NULL#define NULL 0Index:
exec.c
===================================================================exec.c +2
-0@@ -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
===================================================================
helper.c line 1236
Quoted 6 lines
helper.c +3
-0@@ -1231,6 +1231,10 @@ } } +/* This should come from sysemu.h - if we could include it here... */ +void qemu_system_reset_request (void ); +extern int warn_on_triple_fault;
The latter is now obsolete.
Quoted 8 lines
helper.c +1
-0+/* * Check nested exceptions and change to double or triple fault if * needed. It should only be called, if this is not an interrupt. @@ -1248,8 +1252,19 @@fprintf (logfile,"check_exception old: %x new %x\n" , env->old_exception, intno);
helper.c line 1262
Quoted 10 lines
helper.c +8
-2-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 (stderr,"qemu: warning: triple fault\n" );
Does 'loglevel' manage dumping to stderr as well? I don't think so. To remain consistent, the line above should probably be removed.
Quoted 28 lines
helper.c +5
-0+fprintf (logfile,"qemu: warning: triple fault\n" ); + } +qemu_system_reset_request (); +return EXCP_HLT ; + } if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&Index:
target-i386/helper2.c
===================================================================helper2.c +4
-0@@ -362,6 +362,10 @@void cpu_reset (CPUX86State *env) {int i; + +if (loglevel &CPU_LOG_RESET ) { +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 line 785
Quoted 7 lines
cpu-all.h +1
-0@@ -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 {
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
In reply to Kevin WolfQuoted 6 lines · Kevin Wolf
And another version which implements the -d
cpu_resetsuggested by Jamie. Extending -d is probably better than introducing a completely new option.Signed-off-by Kevin Wolf <[email protected]>
Why is it so important to log triple faults? Printing something to stderr instead of logfile is just wrong.
What is your use-case? I think you've got the wrong solution to the problem and I can't find an adequate description of the problem your trying to solve in this thread.
Regards,
Anthony Liguori
PATCH 1/1 x86: Reboot CPU on triple fault - Version 5
In reply to Jan KiszkaDidn't you dare posting another patch yourself? ;-)
But you're right with those two points, I've fixed them now. The hopefully final patch is attached.
dyngen-exec.h +1 -0
@@ -86,6 +86,7 @@extern int fprintf (FILE *,const char *, ...);extern int fputs (const char *,FILE *);extern int printf (const char *, ...); +extern FILE *stderr;#undef NULL#define NULL 0
exec.c +2 -0
@@ -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",
helper.c +16 -2
@@ -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 (); +return EXCP_HLT ; + } +if ((first_contributory && second_contributory) || (env->old_exception ==EXCP0E_PAGE && (second_contributory || (intno ==EXCP0E_PAGE )))) {
helper2.c +4 -0
@@ -362,6 +362,10 @@void cpu_reset (CPUX86State *env) {int i; + +if (loglevel &CPU_LOG_RESET ) { +cpu_dump_state (env, logfile, fprintf,X86_DUMP_FPU |X86_DUMP_CCOP ); + }memset (env,0 , offsetof(CPUX86State , breakpoints));
cpu-all.h +1 -0
@@ -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 {
Quoted 6 lines · Paul Brook
target-i386/helper.c static int check_exception(int intno, in · line 1254
Quoted 3 lines
target-i386/helper.c +2
-0+if (env->old_exception ==EXCP08_DBLE ) { +fprintf (stderr,"qemu: warning: triple fault\n" );IMHO There's no reason to print a message to stderr. This is all well defined behavior, and the accepted way of exiting from 286 protected mode.
Moreover, it's a standard means of rebooting in the absence of APM or ACPI. See native_machine_emergency_restart() in arch/x86/kernel/reboot.c in Linux.
Regards,
Anthony Liguori
Quoted 4 lines
Paul
PATCH 1/1 x86: Reboot CPU on triple fault - Version 6
In reply to Kevin WolfQuite 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.
dyngen-exec.h +1 -0
@@ -86,6 +86,7 @@extern int fprintf (FILE *,const char *, ...);extern int fputs (const char *,FILE *);extern int printf (const char *, ...); +extern FILE *stderr;#undef NULL#define NULL 0
exec.c +2 -0
@@ -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",
helper.c +16 -2
@@ -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 (); +return EXCP_HLT ; + } +if ((first_contributory && second_contributory) || (env->old_exception ==EXCP0E_PAGE && (second_contributory || (intno ==EXCP0E_PAGE )))) {
helper2.c +5 -0
@@ -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));
cpu-all.h +1 -0
@@ -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 {
[Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
In reply to Anthony LiguoriQuoted 8 lines · Anthony Liguori
Quoted 6 lines · Kevin Wolf
And another version which implements the -d
cpu_resetsuggested by Jamie. Extending -d is probably better than introducing a completely new option.Signed-off-by Kevin Wolf <[email protected]>
Quoted 6 lines
Why is it so important to log triple faults? Printing something to stderr instead of logfile is just wrong.
What is your use-case? I think you've got the wrong solution to the problem and I can't find an adequate description of the problem your trying to solve in this thread.
You may not always sit next to your VM when it decides to reboot due to a _real_ problem that caused a triple fault.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 6
In reply to Kevin WolfQuoted 6 lines · Kevin Wolf
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 <[email protected]>
(But you should kill the trailing whitespace in helper2.c - quilt can helpful for managing patches and keeping such things clean.)
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
In reply to Jan KiszkaQuoted 5 lines · Jan Kiszka
Quoted 3 lines · Anthony Liguori
Why is it so important to log triple faults? Printing something to stderr instead of logfile is just wrong.
Quoted 8 lines
Quoted 4 lines
What is your use-case? I think you've got the wrong solution to the problem and I can't find an adequate description of the problem your trying to solve in this thread.
You may not always sit next to your VM when it decides to reboot due to a _real_ problem that caused a triple fault.
You can always check within the guest to see if it's rebooted (via uptime for instance).
It's extremely unlikely you'll ever see an OS triple fault in the wild unless you're doing kernel development. Triple faulting requires a bad IDT or a really bad page table both of which are not something an OS is likely to do by accident. If your OS is triple faulting, I highly doubt it's just going to reboot and everything's going to be okay.
Besides, the -no-reboot flag should already satisfy your needs if you're really concerned about it.
Regards,
Anthony Liguori
Quoted 3 lines
Jan
[Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
Quoted 11 lines · Anthony Liguori
Quoted 10 lines · Jan Kiszka
Quoted 7 lines · Anthony Liguori
Why is it so important to log triple faults? Printing something to stderr instead of logfile is just wrong.
What is your use-case? I think you've got the wrong solution to the problem and I can't find an adequate description of the problem your trying to solve in this thread.
Quoted 6 lines
Quoted 3 lines
You may not always sit next to your VM when it decides to reboot due to a _real_ problem that caused a triple fault.
You can always check within the guest to see if it's rebooted (via uptime for instance).
But you won't find the CPU state on triple fault there.
Quoted 6 lines
It's extremely unlikely you'll ever see an OS triple fault in the wild unless you're doing kernel development. Triple faulting requires a bad IDT or a really bad page table both of which are not something an OS is likely to do by accident. If your OS is triple faulting, I highly doubt it's just going to reboot and everything's going to be okay.
There are various OSes out there in the wild. Not all of them conform to common assumptions about how OSes typically look like. And once you start moving things under a different roof (like QEMU), you are better off logging such /potentially/ critical events (specifically if that roof is a bit smaller due to missing segment limit and type checks). That's at least our situation ATM.
Quoted 3 lines
Besides, the -no-reboot flag should already satisfy your needs if you're really concerned about it.
Nope, because we also have the case where a triple fault is intentionally triggered to perform a reboot.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
Quoted 10 lines · Jan Kiszka
Quoted 5 lines · Anthony Liguori
Jan Kiszka wrote:
You can always check within the guest to see if it's rebooted (via uptime for instance).
But you won't find the CPU state on triple fault there.
Nor will you with your patch.
Quoted 7 lines
Quoted 6 lines
It's extremely unlikely you'll ever see an OS triple fault in the wild unless you're doing kernel development. Triple faulting requires a bad IDT or a really bad page table both of which are not something an OS is likely to do by accident. If your OS is triple faulting, I highly doubt it's just going to reboot and everything's going to be okay.
Quoted 7 lines
There are various OSes out there in the wild. Not all of them conform to common assumptions about how OSes typically look like. And once you start moving things under a different roof (like QEMU), you are better off logging such /potentially/ critical events (specifically if that roof is a bit smaller due to missing segment limit and type checks). That's at least our situation ATM.
It's not a question of whether it's logged, it's a question of whether it gets logged *specially*. That's the crux of the discussion here. My argument is that triple faults are not sufficiently special that they warrant *special* logging.
Regards,
Anthony Liguori
[Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
Quoted 8 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka
Quoted 4 lines · Anthony Liguori
Jan Kiszka wrote:
You can always check within the guest to see if it's rebooted (via
uptime for instance).
Quoted 4 lines
Quoted 2 lines
But you won't find the CPU state on triple fault there.
Nor will you with your patch.
Provided "-d cpu_reset" with Kevin's last patch, they are. I tried it, it worked, that's why I acked it.
Quoted 16 lines
Quoted 8 lines
Quoted 7 lines
It's extremely unlikely you'll ever see an OS triple fault in the wild unless you're doing kernel development. Triple faulting requires a bad IDT or a really bad page table both of which are not something an OS is likely to do by accident. If your OS is triple faulting, I highly doubt it's just going to reboot and everything's going to be okay.
Quoted 7 lines
There are various OSes out there in the wild. Not all of them conform to common assumptions about how OSes typically look like. And once you start moving things under a different roof (like QEMU), you are better off logging such /potentially/ critical events (specifically if that roof is a bit smaller due to missing segment limit and type checks). That's at least our situation ATM.
Quoted 4 lines
It's not a question of whether it's logged, it's a question of whether it gets logged *specially*. That's the crux of the discussion here. My argument is that triple faults are not sufficiently special that they warrant *special* logging.
Depends on the POV. But I really think we loose nothing with the latest patch, rather gain a useful feature for certain (granted) corner cases.
Jan
Re: [Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
In reply to Anthony LiguoriQuoted 4 lines · Anthony Liguori
It's not a question of whether it's logged, it's a question of whether it gets logged *specially*. That's the crux of the discussion here. My argument is that triple faults are not sufficiently special that they warrant *special* logging.
I think CPU resets are special enough indeed. Obviously there _are_ use cases for this option and nobody who doesn't turn it on intentionally could ever get hurt by it. So what is your problem with it?
Honestly, it is ridiculous that we keep arguing about log messages while the main thing changed by the patch is the introduction of the correct CPU reset. I suspect we would never have discussed all these loglevel things if the latest patch wouldn't have been submitted as answer on a (not even working) patch which tried to remove the log...
Kevin
Re: [Qemu-devel] [PATCH] x86: Reboot CPU on triple fault - Version 6
In reply to Kevin WolfQuoted 11 lines · Kevin Wolf
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 <[email protected]>Index:
dyngen-exec.h
===================================================================
dyngen-exec.h line 90
Quoted 5 lines
dyngen-exec.h +1
-0@@ -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.
Quoted 38 lines
dyngen-exec.h +0
-0#undef NULL#define NULL 0Index:
exec.c
===================================================================exec.c +2
-0@@ -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
===================================================================helper.c +5
-2@@ -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 ) {
helper.c line 1265
Quoted 10 lines
helper.c +8
-0+/* 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
Quoted 35 lines
helper.c +3
-0+return EXCP_HLT ; + } + if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&(second_contributory || (intno == EXCP0E_PAGE)))) {Index:
target-i386/helper2.c
===================================================================helper2.c +5
-0@@ -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 +1
-0@@ -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 {
Re: [Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
In reply to Kevin WolfQuoted 8 lines · Kevin Wolf
Quoted 5 lines · Anthony Liguori
It's not a question of whether it's logged, it's a question of whether it gets logged *specially*. That's the crux of the discussion here. My argument is that triple faults are not sufficiently special that they warrant *special* logging.
Quoted 4 lines
I think CPU resets are special enough indeed. Obviously there _are_ use cases for this option and nobody who doesn't turn it on intentionally could ever get hurt by it. So what is your problem with it?
You're missing my whole point. v4 spewed stuff to stderr. That's the problem I had.
Quoted 6 lines
Honestly, it is ridiculous that we keep arguing about log messages while the main thing changed by the patch is the introduction of the correct CPU reset. I suspect we would never have discussed all these loglevel things if the latest patch wouldn't have been submitted as answer on a (not even working) patch which tried to remove the log...
The patch still isn't right but it's close. Logging events to logfile is fine. It's logging to stderr that was problematic.
Regards,
Anthony Liguori
Quoted 4 lines
Kevin
Re: [Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 4
Quoted 2 lines · Anthony Liguori
You're missing my whole point. v4 spewed stuff to stderr. That's the problem I had.
Yes, I was missing that you still were talking about v4, not v6. Sorry.
Kevin
Re: [Qemu-devel] [PATCH] x86: Reboot CPU on triple fault - Version 6
In reply to Anthony LiguoriQuoted 4 lines · Anthony Liguori
Quoted 4 lines
Index:
dyngen-exec.h
===================================================================dyngen-exec.h (Arbeitskopie) renamed from dyngen-exec.h (Revision 4215) +0
-0
Quoted 8 lines
dyngen-exec.h (Arbeitskopie) line 90
Quoted 6 lines
dyngen-exec.h (Arbeitskopie) renamed from dyngen-exec.h (Revision 4215) +1
-0@@ -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.
Right. I'll fix this with the next version.
Quoted 9 lines
dyngen-exec.h (Arbeitskopie) line 92
Quoted 3 lines
dyngen-exec.h (Arbeitskopie) renamed from dyngen-exec.h (Revision 4215) +2
-0+ + 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);
Your suggestion doesn't work for me either. qemu keeps hanging producing lots of triple faults, but there is no CPU reset. Is there even a function to correctly request a CPU reset only? Directly calling cpu_reset doesn't work here.
Kevin
[Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 6
To pick this up again, as the problem is still unfixed:
Quoted 13 lines · Kevin Wolf
Quoted 11 lines · Anthony Liguori
dyngen-exec.h (Arbeitskopie) line 90
Quoted 10 lines
Index:
dyngen-exec.h
===================================================================dyngen-exec.h (Arbeitskopie) renamed from dyngen-exec.h (Revision 4215) +1
-0@@ -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.
Quoted 1 line
Right. I'll fix this with the next version.
To accelerate this "a bit" ;), I attached a refreshed version of your patch.
Quoted 8 lines
Quoted 8 lines
dyngen-exec.h (Arbeitskopie) line 92
Quoted 4 lines
dyngen-exec.h (Arbeitskopie) renamed from dyngen-exec.h (Revision 4215) +2
-0+ + 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:
Quoted 6 lines
Quoted 1 line
cpu_interrupt(env, CPU_INTERRUPT_EXIT);Your suggestion doesn't work for me either. qemu keeps hanging producing lots of triple faults, but there is no CPU reset. Is there even a function to correctly request a CPU reset only? Directly calling
cpu_resetdoesn't work here.
IMHO, qemu_system_reset_request is the right way to raise a system-wide hard reset. I actually don't see any relation to ACPI at this lowest level.
Jan
--- cpu-all.h | 1 + exec.c | 2 ++ target-i386/helper.c | 18 ++++++++++++++++-- target-i386/helper2.c | 5 +++++ 4 files changed, 24 insertions(+), 2 deletions(-)
exec.c +2 -0
@@ -1302,6 +1302,8 @@ CPULogItem cpu_log_items[] = {#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",
target-i386/helper.c +16 -2
@@ -1261,6 +1261,9 @@ void do_interrupt(int intno, int is_int, } } +/* 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.
@@ -1278,8 +1281,19 @@ static int check_exception(int intno, infprintf (logfile,"check_exception old: 0x%x new 0x%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 (); +return EXCP_HLT ; + } if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&
target-i386/helper2.c +5 -0
@@ -363,6 +363,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));tlb_flush (env,1 );
cpu-all.h +1 -0
@@ -824,6 +824,7 @@ target_phys_addr_t cpu_get_phys_page_deb#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 {
PATCH 1/1 x86: Reboot CPU on triple fault - Version 8
Quoted 15 lines · Jan Kiszka
To pick this up again, as the problem is still unfixed:
Quoted 12 lines · Kevin Wolf
Quoted 11 lines · Anthony Liguori
dyngen-exec.h (Arbeitskopie) line 90
Quoted 10 lines
Index:
dyngen-exec.h
===================================================================dyngen-exec.h (Arbeitskopie) renamed from dyngen-exec.h (Revision 4215) +1
-0@@ -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.
Quoted 4 lines
Quoted 1 line
Right. I'll fix this with the next version.
To accelerate this "a bit" ;), I attached a refreshed version of your patch.
And here is a version that even compiles (sorry).
I assume that we do not want to let a triple fault in SVM context escalate to the guest's host context, right? So I left the check in (while it was removed meanwhile from cpu_abort).
--- cpu-all.h | 1 + exec.c | 2 ++ target-i386/helper.c | 15 +++++++++++++-- target-i386/helper2.c | 5 +++++ 4 files changed, 21 insertions(+), 2 deletions(-)
exec.c +2 -0
@@ -1302,6 +1302,8 @@ CPULogItem cpu_log_items[] = {#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",
target-i386/helper.c +13 -2
@@ -1261,6 +1261,9 @@ void do_interrupt(int intno, int is_int, } } +/* 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.
@@ -1278,8 +1281,16 @@ static int check_exception(int intno, infprintf (logfile,"check_exception old: 0x%x new 0x%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 ) +helper_vmexit (SVM_EXIT_SHUTDOWN ,0 ); + +if (loglevel &CPU_LOG_RESET ) +fprintf (logfile,"Triple fault\n" ); + +qemu_system_reset_request (); +return EXCP_HLT ; + } if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&
target-i386/helper2.c +5 -0
@@ -363,6 +363,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));tlb_flush (env,1 );
cpu-all.h +1 -0
@@ -824,6 +824,7 @@ target_phys_addr_t cpu_get_phys_page_deb#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 {
[Qemu-devel] Re: [PATCH] x86: Reboot CPU on triple fault - Version 8
Quoted 16 lines · Jan Kiszka
Quoted 15 lines · Jan Kiszka
To pick this up again, as the problem is still unfixed:
Quoted 12 lines · Kevin Wolf
Quoted 11 lines · Anthony Liguori
dyngen-exec.h (Arbeitskopie) line 90
Quoted 10 lines
Index:
dyngen-exec.h
===================================================================dyngen-exec.h (Arbeitskopie) renamed from dyngen-exec.h (Revision 4215) +1
-0@@ -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.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Right. I'll fix this with the next version.
To accelerate this "a bit" ;), I attached a refreshed version of your patch.
And here is a version that even compiles (sorry).
Sigh, almost. linux-user become broken, and I posted faster than it took the compiler to reach that target.
No point in testing for double faults in user space, I guess...
--- cpu-all.h | 1 + exec.c | 2 ++ target-i386/helper.c | 17 +++++++++++++++-- target-i386/helper2.c | 5 +++++ 4 files changed, 23 insertions(+), 2 deletions(-)
exec.c +2 -0
@@ -1302,6 +1302,8 @@ CPULogItem cpu_log_items[] = {#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",
target-i386/helper.c +15 -2
@@ -1261,6 +1261,9 @@ void do_interrupt(int intno, int is_int, } } +/* 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.
@@ -1278,8 +1281,18 @@ static int check_exception(int intno, infprintf (logfile,"check_exception old: 0x%x new 0x%x\n" , env->old_exception, intno); -if (env->old_exception ==EXCP08_DBLE ) -cpu_abort (env,"triple fault" ); +#if !defined(CONFIG_USER_ONLY ) +if (env->old_exception ==EXCP08_DBLE ) { +if (env->intercept &INTERCEPT_SVM_MASK ) +helper_vmexit (SVM_EXIT_SHUTDOWN ,0 ); + +if (loglevel &CPU_LOG_RESET ) +fprintf (logfile,"Triple fault\n" ); + +qemu_system_reset_request (); +return EXCP_HLT ; + } +#endif if ((first_contributory && second_contributory) || (env->old_exception == EXCP0E_PAGE &&
target-i386/helper2.c +5 -0
@@ -363,6 +363,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));tlb_flush (env,1 );
cpu-all.h +1 -0
@@ -824,6 +824,7 @@ target_phys_addr_t cpu_get_phys_page_deb#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 {