Juergen Keil
2007-Mar-12 11:03 UTC
[qemu-discuss] Re: [Qemu-devel] [PATCH] workaround qemu guest SIGSEGVs with cmpxchg8b insn
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, ®_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 at nongnu.org
> > http://lists.nongnu.org/mailman/listinfo/qemu-devel