qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] workaround qemu guest SIGSEGVs with cmpxchg8b insn
@ 2007-03-05 10:27 Juergen Keil
  2007-03-05 23:24 ` Fabrice Bellard
  0 siblings, 1 reply; 3+ messages in thread
From: Juergen Keil @ 2007-03-05 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-discuss

[-- Attachment #1: Type: TEXT/plain, Size: 3243 bytes --]

Current "Solaris x86 Developer Express" doesn't install any more as qemu 
guest

- qemu 0.9.0 + cvs (32bit), 768 mbyte memory (or more) allocated for guest
- kqemu *not* used


I doesn't install because the java virtual machine (used for the installer)
crashes with a SIGSEGV.


  
========================================================================

The following test program reproduces the problem.  The second cmpxchg8b
instruction after the fork results in a copy-on-write page fault and
the process terminates with a core dump.


Same program runs on bare metal just fine.


The problem can be reproduced both with an opensolaris b55 guest or a
linux ubuntu guest, running inside an i386-softmmu qemu, and running it
with -no-kqemu.

========================================================================

/*
 * cc -o cmpxchg cmpxchg.c
 */
 
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>


#if 0
static int
cmpxchg64(int64_t *mem, int64_t *expected_old, int64_t new)
{
	if (*mem == *expected_old) {
		*mem = new;
		return 1;
	} else {
		*expected_old = *mem;
		return 0;
	}
}
#else
static int
cmpxchg64(int64_t *mem, int64_t *expected_old, int64_t new)
{
	asm("pushl %ebx");
	asm("pushl %esi");
	asm("pushl %edi");

	asm("movl 12(%ebp), %esi");
	asm("movl (%esi), %eax");
	asm("movl 4(%esi), %edx");
	asm("movl 16(%ebp), %ebx");
	asm("movl 20(%ebp), %ecx");
	asm("movl 8(%ebp), %edi");
	asm("cmpxchg8b (%edi)");
	asm("movl %eax, (%esi)");
	asm("movl %edx, 4(%esi)");
	
	asm("sete %al");
	asm("movsbl %al, %eax");

	asm("popl %edi");
	asm("popl %esi");
	asm("popl %ebx");
}
#endif

int64_t *val;

int
main(int argc, char **argv)
{
	int64_t old, update;

	val = calloc(2*4096, 1);
	val = (void*)(((long)val + 0xfff) & ~0xfff);
	printf("%lld\n", *val);

	old = 0;
	update = 1;
	cmpxchg64(val, &old, update);

	printf("%lld\n", *val);

	switch (fork()) {
	case -1:
		perror("fork");
		exit(1);
	case 0:
		sleep(1);
		_exit(0);
		break;
	default:
		break;
	}
	
	old = update;
	update++;
	cmpxchg64(val, &old, update);

	printf("%lld\n", *val);
}
========================================================================

Workaround: move the code from inside the helper_cmpxchg8b() function
into the op_cmpxchg8b() insn template[*].

A patch for this workaround is attached.  With this workaround applied,
the above test program doesn't SIGSEGV any more, and the java installer
from  "Solaris x86 Developer Express" doesn't crash with SIGSEGV any more
either.



[*] so that softmmu_template.h function __stq_mmu() ...

``void REGPARM(2) glue(glue(__st, SUFFIX), MMUSUFFIX)(target_ulong addr, 
                                                    DATA_TYPE val,
                                                    int is_user)''

... passes a PC that is is inside translated code to tlb_fill().

Without the workaround, the return address passed into tlb_fill()
is inside helper_cmpxchg8b() and tlb_fill() is unable to pass
the correct virtual PC for the page fault to the kernel exception -
the PC that gets passed is the PC that was last saved to the "env"
register file, and this could point to a location that was executed
a few translations blocks before we reached the cmpxchg8b instruction.




[-- Attachment #2: cmpxchg8b.patch --]
[-- Type: TEXT/plain, Size: 1358 bytes --]

diff -ru qemu-cvs-orig/target-i386/helper.c qemu-cvs/target-i386/helper.c
--- qemu-cvs-orig/target-i386/helper.c	2007-02-09 22:10:08.000000000 +0100
+++ qemu-cvs/target-i386/helper.c	2007-03-04 21:46:41.971808493 +0100
@@ -1573,24 +1573,6 @@
     EDX = (uint32_t)r;
 }
 
-void helper_cmpxchg8b(void)
-{
-    uint64_t d;
-    int eflags;
-
-    eflags = cc_table[CC_OP].compute_all();
-    d = ldq(A0);
-    if (d == (((uint64_t)EDX << 32) | EAX)) {
-        stq(A0, ((uint64_t)ECX << 32) | EBX);
-        eflags |= CC_Z;
-    } else {
-        EDX = d >> 32;
-        EAX = d;
-        eflags &= ~CC_Z;
-    }
-    CC_SRC = eflags;
-}
-
 void helper_cpuid(void)
 {
     uint32_t index;
diff -ru qemu-cvs-orig/target-i386/op.c qemu-cvs/target-i386/op.c
--- qemu-cvs-orig/target-i386/op.c	2007-02-09 22:10:08.000000000 +0100
+++ qemu-cvs/target-i386/op.c	2007-03-04 21:46:46.396171993 +0100
@@ -727,7 +727,21 @@
 
 void OPPROTO op_cmpxchg8b(void)
 {
-    helper_cmpxchg8b();
+    uint64_t d;
+    int eflags;
+
+    eflags = cc_table[CC_OP].compute_all();
+    d = ldq(A0);
+    if (d == (((uint64_t)EDX << 32) | EAX)) {
+        stq(A0, ((uint64_t)ECX << 32) | EBX);
+        eflags |= CC_Z;
+    } else {
+        EDX = d >> 32;
+        EAX = d;
+        eflags &= ~CC_Z;
+    }
+    CC_SRC = eflags;
+    FORCE_RET();
 }
 
 void OPPROTO op_movl_T0_0(void)

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] workaround qemu guest SIGSEGVs with cmpxchg8b insn
@ 2007-03-12 11:03 Juergen Keil
  0 siblings, 0 replies; 3+ messages in thread
From: Juergen Keil @ 2007-03-12 11:03 UTC (permalink / raw)
  To: fabrice; +Cc: qemu-devel, qemu-discuss

So a better fix would be something like this?


Index: target-i386/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/translate.c,v
retrieving revision 1.62
diff -u -B -C5 -r1.62 translate.c
*** target-i386/translate.c     16 Jan 2007 19:28:58 -0000      1.62
--- target-i386/translate.c     12 Mar 2007 10:46:21 -0000
***************
*** 3795,3804 ****
--- 3795,3805 ----
      case 0x1c7: /* cmpxchg8b */
          modrm = ldub_code(s->pc++);
          mod = (modrm >> 6) & 3;
          if (mod == 3)
              goto illegal_op;
+       gen_jmp_im(pc_start - s->cs_base);
          if (s->cc_op != CC_OP_DYNAMIC)
              gen_op_set_cc_op(s->cc_op);
          gen_lea_modrm(s, modrm, &reg_addr, &offset_addr);
          gen_op_cmpxchg8b();
          s->cc_op = CC_OP_EFLAGS;




> OK for the bug. The proper patch is to set the EIP before executing the 
> instruction, as it is done for the other helpers which can generate 
> exceptions. I'll try to make a fix ASAP.
> 
> Regards,
> 
> Fabrice.
> 
> Juergen Keil wrote:
> > Current "Solaris x86 Developer Express" doesn't install any more as qemu 
> > guest
> > 
> > - qemu 0.9.0 + cvs (32bit), 768 mbyte memory (or more) allocated for guest
> > - kqemu *not* used
> > 
> > 
> > I doesn't install because the java virtual machine (used for the installer)
> > crashes with a SIGSEGV.
> > 
> > 
> >   
> > ========================================================================
> > 
> > The following test program reproduces the problem.  The second cmpxchg8b
> > instruction after the fork results in a copy-on-write page fault and
> > the process terminates with a core dump.
> > 
> > 
> > Same program runs on bare metal just fine.
> > 
> > 
> > The problem can be reproduced both with an opensolaris b55 guest or a
> > linux ubuntu guest, running inside an i386-softmmu qemu, and running it
> > with -no-kqemu.
> > 
> > ========================================================================
> > 
> > /*
> >  * cc -o cmpxchg cmpxchg.c
> >  */
> >  
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <unistd.h>
> > #include <sys/types.h>
> > 
> > 
> > #if 0
> > static int
> > cmpxchg64(int64_t *mem, int64_t *expected_old, int64_t new)
> > {
> > 	if (*mem == *expected_old) {
> > 		*mem = new;
> > 		return 1;
> > 	} else {
> > 		*expected_old = *mem;
> > 		return 0;
> > 	}
> > }
> > #else
> > static int
> > cmpxchg64(int64_t *mem, int64_t *expected_old, int64_t new)
> > {
> > 	asm("pushl %ebx");
> > 	asm("pushl %esi");
> > 	asm("pushl %edi");
> > 
> > 	asm("movl 12(%ebp), %esi");
> > 	asm("movl (%esi), %eax");
> > 	asm("movl 4(%esi), %edx");
> > 	asm("movl 16(%ebp), %ebx");
> > 	asm("movl 20(%ebp), %ecx");
> > 	asm("movl 8(%ebp), %edi");
> > 	asm("cmpxchg8b (%edi)");
> > 	asm("movl %eax, (%esi)");
> > 	asm("movl %edx, 4(%esi)");
> > 	
> > 	asm("sete %al");
> > 	asm("movsbl %al, %eax");
> > 
> > 	asm("popl %edi");
> > 	asm("popl %esi");
> > 	asm("popl %ebx");
> > }
> > #endif
> > 
> > int64_t *val;
> > 
> > int
> > main(int argc, char **argv)
> > {
> > 	int64_t old, update;
> > 
> > 	val = calloc(2*4096, 1);
> > 	val = (void*)(((long)val + 0xfff) & ~0xfff);
> > 	printf("%lld\n", *val);
> > 
> > 	old = 0;
> > 	update = 1;
> > 	cmpxchg64(val, &old, update);
> > 
> > 	printf("%lld\n", *val);
> > 
> > 	switch (fork()) {
> > 	case -1:
> > 		perror("fork");
> > 		exit(1);
> > 	case 0:
> > 		sleep(1);
> > 		_exit(0);
> > 		break;
> > 	default:
> > 		break;
> > 	}
> > 	
> > 	old = update;
> > 	update++;
> > 	cmpxchg64(val, &old, update);
> > 
> > 	printf("%lld\n", *val);
> > }
> > ========================================================================
> > 
> > Workaround: move the code from inside the helper_cmpxchg8b() function
> > into the op_cmpxchg8b() insn template[*].
> > 
> > A patch for this workaround is attached.  With this workaround applied,
> > the above test program doesn't SIGSEGV any more, and the java installer
> > from  "Solaris x86 Developer Express" doesn't crash with SIGSEGV any more
> > either.
> > 
> > 
> > 
> > [*] so that softmmu_template.h function __stq_mmu() ...
> > 
> > ``void REGPARM(2) glue(glue(__st, SUFFIX), MMUSUFFIX)(target_ulong addr, 
> >                                                     DATA_TYPE val,
> >                                                     int is_user)''
> > 
> > ... passes a PC that is is inside translated code to tlb_fill().
> > 
> > Without the workaround, the return address passed into tlb_fill()
> > is inside helper_cmpxchg8b() and tlb_fill() is unable to pass
> > the correct virtual PC for the page fault to the kernel exception -
> > the PC that gets passed is the PC that was last saved to the "env"
> > register file, and this could point to a location that was executed
> > a few translations blocks before we reached the cmpxchg8b instruction.
> > 
> > 
> > 
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > diff -ru qemu-cvs-orig/target-i386/helper.c qemu-cvs/target-i386/helper.c
> > --- qemu-cvs-orig/target-i386/helper.c	2007-02-09 22:10:08.000000000 
+0100
> > +++ qemu-cvs/target-i386/helper.c	2007-03-04 21:46:41.971808493 +0100
> > @@ -1573,24 +1573,6 @@
> >      EDX = (uint32_t)r;
> >  }
> >  
> > -void helper_cmpxchg8b(void)
> > -{
> > -    uint64_t d;
> > -    int eflags;
> > -
> > -    eflags = cc_table[CC_OP].compute_all();
> > -    d = ldq(A0);
> > -    if (d == (((uint64_t)EDX << 32) | EAX)) {
> > -        stq(A0, ((uint64_t)ECX << 32) | EBX);
> > -        eflags |= CC_Z;
> > -    } else {
> > -        EDX = d >> 32;
> > -        EAX = d;
> > -        eflags &= ~CC_Z;
> > -    }
> > -    CC_SRC = eflags;
> > -}
> > -
> >  void helper_cpuid(void)
> >  {
> >      uint32_t index;
> > diff -ru qemu-cvs-orig/target-i386/op.c qemu-cvs/target-i386/op.c
> > --- qemu-cvs-orig/target-i386/op.c	2007-02-09 22:10:08.000000000 +0100
> > +++ qemu-cvs/target-i386/op.c	2007-03-04 21:46:46.396171993 +0100
> > @@ -727,7 +727,21 @@
> >  
> >  void OPPROTO op_cmpxchg8b(void)
> >  {
> > -    helper_cmpxchg8b();
> > +    uint64_t d;
> > +    int eflags;
> > +
> > +    eflags = cc_table[CC_OP].compute_all();
> > +    d = ldq(A0);
> > +    if (d == (((uint64_t)EDX << 32) | EAX)) {
> > +        stq(A0, ((uint64_t)ECX << 32) | EBX);
> > +        eflags |= CC_Z;
> > +    } else {
> > +        EDX = d >> 32;
> > +        EAX = d;
> > +        eflags &= ~CC_Z;
> > +    }
> > +    CC_SRC = eflags;
> > +    FORCE_RET();
> >  }
> >  
> >  void OPPROTO op_movl_T0_0(void)
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > _______________________________________________
> > Qemu-devel mailing list
> > Qemu-devel@nongnu.org
> > http://lists.nongnu.org/mailman/listinfo/qemu-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-03-12 11:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-05 10:27 [Qemu-devel] [PATCH] workaround qemu guest SIGSEGVs with cmpxchg8b insn Juergen Keil
2007-03-05 23:24 ` Fabrice Bellard
  -- strict thread matches above, loose matches on Subject: below --
2007-03-12 11:03 Juergen Keil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).