[Qemu-devel] Patch: generate a DBF when a GPF could not be delivered on x86

[Qemu-devel] Patch: generate a DBF when a GPF could not be delivered on x86

[Bernhard Kauer]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]

Qemu does not generate a double fault (DBF) on x86, if a general protection
fault could not be delivered. Instead it hangs in a loop.

The patch fix this bug by checking whether we are already in a GPF exception.


    Bernhard Kauer
 

[-- Attachment #2: qemu_dbf.diff --]
[-- Type: text/plain, Size: 6286 bytes --]

Index: helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-i386/helper.c,v
retrieving revision 1.74
diff -u -r1.74 helper.c
--- helper.c	1 Feb 2007 22:12:19 -0000	1.74
+++ helper.c	27 Mar 2007 11:33:34 -0000
@@ -592,6 +592,16 @@
     sp += 4;\
 }
 
+void
+raise_gpf(int intno, int is_int, int is_hw, int error)
+{
+  if (!is_int && !is_hw && intno == EXCP0D_GPF)
+    raise_exception_err(EXCP08_DBLE, 0);
+  else
+    raise_exception_err(EXCP0D_GPF, error >= 0 ? error : intno * TARGET_LONG_SIZE + 2);
+}
+
+
 /* protected mode interrupt */
 static void do_interrupt_protected(int intno, int is_int, int error_code,
                                    unsigned int next_eip, int is_hw)
@@ -624,7 +634,7 @@
 
     dt = &env->idt;
     if (intno * 8 + 7 > dt->limit)
-        raise_exception_err(EXCP0D_GPF, intno * 8 + 2);
+        raise_gpf(intno, is_int, is_hw, -1);
     ptr = dt->base + intno * 8;
     e1 = ldl_kernel(ptr);
     e2 = ldl_kernel(ptr + 4);
@@ -661,29 +671,30 @@
     case 15: /* 386 trap gate */
         break;
     default:
-        raise_exception_err(EXCP0D_GPF, intno * 8 + 2);
+        raise_gpf(intno, is_int, is_hw, -1);
         break;
     }
     dpl = (e2 >> DESC_DPL_SHIFT) & 3;
     cpl = env->hflags & HF_CPL_MASK;
     /* check privledge if software int */
     if (is_int && dpl < cpl)
-        raise_exception_err(EXCP0D_GPF, intno * 8 + 2);
+        raise_gpf(intno, is_int, is_hw, -1);
+
     /* check valid bit */
     if (!(e2 & DESC_P_MASK))
         raise_exception_err(EXCP0B_NOSEG, intno * 8 + 2);
     selector = e1 >> 16;
     offset = (e2 & 0xffff0000) | (e1 & 0x0000ffff);
     if ((selector & 0xfffc) == 0)
-        raise_exception_err(EXCP0D_GPF, 0);
+        raise_gpf(intno, is_int, is_hw, 0);
 
     if (load_segment(&e1, &e2, selector) != 0)
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
     if (!(e2 & DESC_S_MASK) || !(e2 & (DESC_CS_MASK)))
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
     dpl = (e2 >> DESC_DPL_SHIFT) & 3;
     if (dpl > cpl)
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
     if (!(e2 & DESC_P_MASK))
         raise_exception_err(EXCP0B_NOSEG, selector & 0xfffc);
     if (!(e2 & DESC_C_MASK) && dpl < cpl) {
@@ -710,14 +721,14 @@
     } else if ((e2 & DESC_C_MASK) || dpl == cpl) {
         /* to same priviledge */
         if (env->eflags & VM_MASK)
-            raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+	    raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
         new_stack = 0;
         sp_mask = get_sp_mask(env->segs[R_SS].flags);
         ssp = env->segs[R_SS].base;
         esp = ESP;
         dpl = cpl;
     } else {
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
         new_stack = 0; /* avoid warning */
         sp_mask = 0; /* avoid warning */
         ssp = 0; /* avoid warning */
@@ -860,7 +871,7 @@
 
     dt = &env->idt;
     if (intno * 16 + 15 > dt->limit)
-        raise_exception_err(EXCP0D_GPF, intno * 16 + 2);
+        raise_gpf(intno, is_int, is_hw, -1);
     ptr = dt->base + intno * 16;
     e1 = ldl_kernel(ptr);
     e2 = ldl_kernel(ptr + 4);
@@ -872,14 +883,13 @@
     case 15: /* 386 trap gate */
         break;
     default:
-        raise_exception_err(EXCP0D_GPF, intno * 16 + 2);
-        break;
+        raise_gpf(intno, is_int, is_hw, -1);
     }
     dpl = (e2 >> DESC_DPL_SHIFT) & 3;
     cpl = env->hflags & HF_CPL_MASK;
     /* check privledge if software int */
     if (is_int && dpl < cpl)
-        raise_exception_err(EXCP0D_GPF, intno * 16 + 2);
+        raise_gpf(intno, is_int, is_hw, -1);
     /* check valid bit */
     if (!(e2 & DESC_P_MASK))
         raise_exception_err(EXCP0B_NOSEG, intno * 16 + 2);
@@ -887,19 +897,19 @@
     offset = ((target_ulong)e3 << 32) | (e2 & 0xffff0000) | (e1 & 0x0000ffff);
     ist = e2 & 7;
     if ((selector & 0xfffc) == 0)
-        raise_exception_err(EXCP0D_GPF, 0);
+        raise_gpf(intno, is_int, is_hw, 0);
 
     if (load_segment(&e1, &e2, selector) != 0)
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
     if (!(e2 & DESC_S_MASK) || !(e2 & (DESC_CS_MASK)))
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
     dpl = (e2 >> DESC_DPL_SHIFT) & 3;
     if (dpl > cpl)
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
     if (!(e2 & DESC_P_MASK))
         raise_exception_err(EXCP0B_NOSEG, selector & 0xfffc);
     if (!(e2 & DESC_L_MASK) || (e2 & DESC_B_MASK))
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
     if ((!(e2 & DESC_C_MASK) && dpl < cpl) || ist != 0) {
         /* to inner priviledge */
         if (ist != 0)
@@ -912,7 +922,7 @@
     } else if ((e2 & DESC_C_MASK) || dpl == cpl) {
         /* to same priviledge */
         if (env->eflags & VM_MASK)
-            raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+	    raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
         new_stack = 0;
         if (ist != 0)
             esp = get_rsp_from_tss(ist + 3);
@@ -921,7 +931,7 @@
         esp &= ~0xfLL; /* align stack */
         dpl = cpl;
     } else {
-        raise_exception_err(EXCP0D_GPF, selector & 0xfffc);
+        raise_gpf(intno, is_int, is_hw, selector & 0xfffc);
         new_stack = 0; /* avoid warning */
         esp = 0; /* avoid warning */
     }
@@ -1089,7 +1099,12 @@
     /* real mode (simpler !) */
     dt = &env->idt;
     if (intno * 4 + 3 > dt->limit)
-        raise_exception_err(EXCP0D_GPF, intno * 8 + 2);
+      {
+        if (!is_int && intno == EXCP0D_GPF)
+	    raise_exception_err(EXCP08_DBLE, 0);
+	else
+	    raise_exception_err(EXCP0D_GPF, intno * 4 + 2);
+      }
     ptr = dt->base + intno * 4;
     offset = lduw_kernel(ptr);
     selector = lduw_kernel(ptr + 2);

Re: [Qemu-devel] Patch: generate a DBF when a GPF could not be delivered on x86

[Sebastian Kaliszewski]

Bernhard Kauer wrote:
> Qemu does not generate a double fault (DBF) on x86, if a general protection
> fault could not be delivered. Instead it hangs in a loop.
> 
> The patch fix this bug by checking whether we are already in a GPF exception.

If you're at it, maybe add triple fault handling (ie exception if DBF 
handler) -- it should reset CPU.


rgds
-- 
Sebastian Kaliszewski

Re: [Qemu-devel] Patch: generate a DBF when a GPF could not be delivered on x86

[Bernhard Kauer]

On Wed, Mar 28, 2007 at 10:13:49AM +0200, Sebastian Kaliszewski wrote:
> Bernhard Kauer wrote:
> >Qemu does not generate a double fault (DBF) on x86, if a general protection
> >fault could not be delivered. Instead it hangs in a loop.
> >
> >The patch fix this bug by checking whether we are already in a GPF 
> >exception.
> 
> If you're at it, maybe add triple fault handling (ie exception if DBF 
> handler) -- it should reset CPU.

There are many things missing in the x86 exception handling. For example
the case PF -> PF is also not handled.


    Bernhard Kauer

[Qemu-devel] Patch: generate double and triple faults

[Bernhard Kauer]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On Wed, Mar 28, 2007 at 02:39:31PM +0200, Bernhard Kauer wrote:
> On Wed, Mar 28, 2007 at 10:13:49AM +0200, Sebastian Kaliszewski wrote:
> > Bernhard Kauer wrote:
> > >Qemu does not generate a double fault (DBF) on x86, if a general protection
> > >fault could not be delivered. Instead it hangs in a loop.
> > >
> > >The patch fix this bug by checking whether we are already in a GPF 
> > >exception.
> > 
> > If you're at it, maybe add triple fault handling (ie exception if DBF 
> > handler) -- it should reset CPU.
> 
> There are many things missing in the x86 exception handling. For example
> the case PF -> PF is also not handled.

The attached patch implements double andd triple fault handling and makes
the last patch obsolete. It does not generate a reset on triple fault, but
it aborts qemu. The same happen currently if the TSS is bogous.


Greetings,

    Bernhard Kauer

[-- Attachment #2: qemu_exceptions.diff --]
[-- Type: text/plain, Size: 3227 bytes --]

Index: cpu-exec.c
===================================================================
RCS file: /sources/qemu/qemu/cpu-exec.c,v
retrieving revision 1.97
diff -u -r1.97 cpu-exec.c
--- cpu-exec.c	30 Mar 2007 16:44:53 -0000	1.97
+++ cpu-exec.c	31 Mar 2007 11:59:52 -0000
@@ -360,6 +360,7 @@
                                  env->exception_is_int, 
                                  env->error_code, 
                                  env->exception_next_eip, 0);
+		    env->old_exception = -1;  // successful delivered
 #elif defined(TARGET_PPC)
                     do_interrupt(env);
 #elif defined(TARGET_MIPS)
Index: target-i386/helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-i386/helper.c,v
retrieving revision 1.74
diff -u -r1.74 helper.c
--- target-i386/helper.c	1 Feb 2007 22:12:19 -0000	1.74
+++ target-i386/helper.c	31 Mar 2007 11:59:58 -0000
@@ -1192,6 +1193,37 @@
     }
 }
 
+
+/**
+ * Check nested exceptions and change to double or triple fault if
+ * needed. It should only be called, if this is not an
+ * interrrupt. Returns the new exception number.
+ */
+int check_exception(int intno, int *error_code)
+{
+  if (loglevel & CPU_LOG_INT)
+    fprintf(logfile, "%s()  old: %x new %x\n",__func__,  env->old_exception, intno);
+
+
+ if (env->old_exception == EXCP08_DBLE)
+   cpu_abort(env, "triple fault");
+
+ char  first_contributory  = env->old_exception == 0 || (env->old_exception >= 10 && env->old_exception <= 13);
+ char second_contributory  = intno == 0 || (intno >= 10 && intno <= 13);
+
+ if ((first_contributory && second_contributory) 
+     || (env->old_exception == EXCP0E_PAGE && (second_contributory || (intno == EXCP0E_PAGE))))
+   {
+     intno = EXCP08_DBLE;
+     *error_code = 0;
+   }
+
+ if (second_contributory || (intno == EXCP0E_PAGE) || (intno == EXCP08_DBLE))
+   env->old_exception = intno;
+ return intno;
+}
+
+
 /*
  * Signal an interruption. It is executed in the main CPU loop.
  * is_int is TRUE if coming from the int instruction. next_eip is the
@@ -1201,6 +1233,9 @@
 void raise_interrupt(int intno, int is_int, int error_code, 
                      int next_eip_addend)
 {
+    if (!is_int)
+        intno = check_exception(intno, &error_code);
+
     env->exception_index = intno;
     env->error_code = error_code;
     env->exception_is_int = is_int;
@@ -1211,6 +1246,8 @@
 /* same as raise_exception_err, but do not restore global registers */
 static void raise_exception_err_norestore(int exception_index, int error_code)
 {
+    exception_index = check_exception(exception_index, &error_code);
+
     env->exception_index = exception_index;
     env->error_code = error_code;
     env->exception_is_int = 0;
Index: target-i386/cpu.h
===================================================================
RCS file: /sources/qemu/qemu/target-i386/cpu.h,v
retrieving revision 1.41
diff -u -r1.41 cpu.h
--- target-i386/cpu.h	5 Feb 2007 22:06:27 -0000	1.41
+++ target-i386/cpu.h	31 Mar 2007 11:59:56 -0000
@@ -515,6 +515,7 @@
     uint32_t smbase;
     int interrupt_request; 
     int user_mode_only; /* user mode only simulation */
+    int old_exception;  /* exception in flight */
 
     CPU_COMMON