[Qemu-devel] [PATCH] clone syscall fix

[Qemu-devel] [PATCH] clone syscall fix

[Stuart Anderson]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 639 bytes --]


Even though clone() and fork() are related, they don't seem to be close
enough to allow a single routine to be used to implement both. With this
patch, the LTP tests for clone now pass.

It may be possible to fold this back into do_fork(), but this just seemed to
be a little bit more straightforward.


                                 Stuart

Stuart R. Anderson                               anderson@netsweng.com
Network & Software Engineering                   http://www.netsweng.com/
1024D/37A79149:                                  0791 D3B8 9A4C 2CDC A31F
                                                  BD03 0A62 E534 37A7 9149

[-- Attachment #2: clone() syscall fix --]
[-- Type: TEXT/x-diff, Size: 2857 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-03-26 11:20:06.000000000 -0400
+++ qemu/linux-user/syscall.c	2007-03-26 11:28:01.000000000 -0400
@@ -2088,6 +2088,75 @@
     return 0;
 }
 
+int do_clone(CPUState *env, unsigned int flags, unsigned long newsp,
+             unsigned long parent_tidptr, unsigned long tls_val,
+             unsigned long child_tidptr, unsigned long regs)
+{
+    int ret;
+    TaskState *ts = NULL;
+    uint8_t *new_stack;
+    CPUState *new_env;
+
+        ts = malloc(sizeof(TaskState) + NEW_STACK_SIZE);
+        memset(ts, 0, sizeof(TaskState));
+        new_stack = ts->stack;
+        ts->used = 1;
+        /* add in task state list */
+        ts->next = first_task_state;
+        first_task_state = ts;
+        /* we create a new CPU instance. */
+        new_env = cpu_copy(env);
+#if defined(TARGET_I386)
+        if (!newsp)
+            newsp = env->regs[R_ESP];
+        new_env->regs[R_ESP] = newsp;
+        new_env->regs[R_EAX] = 0;
+#elif defined(TARGET_ARM)
+        if (!newsp)
+            newsp = env->regs[13];
+        new_env->regs[13] = newsp;
+        new_env->regs[0] = 0;
+#elif defined(TARGET_SPARC)
+        if (!newsp)
+            newsp = env->regwptr[22];
+        new_env->regwptr[22] = newsp;
+        new_env->regwptr[0] = 0;
+	/* XXXXX */
+        printf ("HELPME: %s:%d\n", __FILE__, __LINE__);
+#elif defined(TARGET_M68K)
+        if (!newsp)
+            newsp = env->aregs[7];
+        new_env->aregs[7] = newsp;
+        new_env->dregs[0] = 0;
+        /* ??? is this sufficient?  */
+#elif defined(TARGET_MIPS)
+        printf ("HELPME: %s:%d\n", __FILE__, __LINE__);
+#elif defined(TARGET_PPC)
+        if (!newsp)
+            newsp = env->gpr[1];
+        new_env->gpr[1] = newsp;
+        {
+            int i;
+            for (i = 7; i < 32; i++)
+                new_env->gpr[i] = 0;
+        }
+#elif defined(TARGET_SH4)
+	if (!newsp)
+	  newsp = env->gregs[15];
+	new_env->gregs[15] = newsp;
+	/* XXXXX */
+#else
+#error unsupported target CPU
+#endif
+        new_env->opaque = ts;
+#ifdef __ia64__
+        ret = __clone2(clone_func, new_stack + NEW_STACK_SIZE, flags, new_env);
+#else
+	ret = clone(clone_func, new_stack + NEW_STACK_SIZE, flags, new_env);
+#endif
+    return ret;
+}
+
 int do_fork(CPUState *env, unsigned int flags, unsigned long newsp)
 {
     int ret;
@@ -3529,7 +3598,7 @@
         ret = get_errno(fsync(arg1));
         break;
     case TARGET_NR_clone:
-        ret = get_errno(do_fork(cpu_env, arg1, arg2));
+        ret = get_errno(do_clone(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6));
         break;
 #ifdef __NR_exit_group
         /* new thread calls */

Re: [Qemu-devel] [PATCH] clone syscall fix

[Thiemo Seufer]

Stuart Anderson wrote:
> 
> Even though clone() and fork() are related, they don't seem to be close
> enough to allow a single routine to be used to implement both. With this
> patch, the LTP tests for clone now pass.

But it still does the same, assuming VM_CLONE is set, except for passing
additional arguments to the host call. Passing untranslated regs looks
like a bug to me, I'm unsure about the tls_val.

> It may be possible to fold this back into do_fork(), but this just seemed to
> be a little bit more straightforward.

Since Linux's fork() is just a specialcase of clone() this should be
done eventually.


Thiemo

Re: [Qemu-devel] [PATCH] clone syscall fix

[Stuart Anderson]

On Sat, 31 Mar 2007, Thiemo Seufer wrote:

> Stuart Anderson wrote:
>>
>> Even though clone() and fork() are related, they don't seem to be close
>> enough to allow a single routine to be used to implement both. With this
>> patch, the LTP tests for clone now pass.
>
> But it still does the same, assuming VM_CLONE is set, except for passing
> additional arguments to the host call.

I'm not so sure that the VM_CLONE flag should control wether the new
stack is set up or not. There are tests for newsp == NULL inside that
block anyway. The LTP certainly tests combination for which the
do_fork() code doesn't work.


> Passing untranslated regs looks
> like a bug to me, I'm unsure about the tls_val.

Hmm, could be, but that's the way it is in the current code. I think
more testing on additional combination sof target &host will be needed.

>> It may be possible to fold this back into do_fork(), but this just seemed to
>> be a little bit more straightforward.
>
> Since Linux's fork() is just a specialcase of clone() this should be
> done eventually.

I'll try just dropping do_fork completely, and see if this new do_clone()
works for the fork case also. If so, then that effectively folds the
changes back into do_fork(), and more closely resembles that non-emulated
case of fork() being implemnted on top of clone( anyway.


                                 Stuart

Stuart R. Anderson                               anderson@netsweng.com
Network & Software Engineering                   http://www.netsweng.com/
1024D/37A79149:                                  0791 D3B8 9A4C 2CDC A31F
                                                  BD03 0A62 E534 37A7 9149