Re: [Qemu-devel] [PATCH] workaround qemu guest SIGSEGVs with cmpxchg8b insn

[Juergen Keil <jk@tools.de>]

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