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

[Fabrice Bellard <fabrice@bellard.org>]

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