[Qemu-devel] [patch] testandset

[Piotr Krysik]

Hi,

Some weeks ago I described problem with testandset 
function on x86 architecture:
http://lists.gnu.org/archive/html/qemu-devel/2004-06/msg00022.html
and Richard Zidlicky confirmed that m68k behaves 
similar:
http://lists.gnu.org/archive/html/qemu-devel/2004-06/msg00045.html
Later I checked (under emulator) that powerpc is not 
affected by the problem.

Now I'm attaching a patch (tas.diff) to fix 
testandset on x86, x86_64 and m68k (m68k was not 
tested). The patch should significantly improve 
IRQ latency.

I'd like to ask people using qemu on architectures 
other that x86, x86_64 and powerpc to do simple test 
to verify assumptions on testandset function. After 
unpacking qemu 0.6, please apply attached patch 
tas-verify.diff:
  # cd qemu-0.6.0
  # patch -p1 < [full pathname of tas-verify.diff]
and then build and run qemu as usual. If qemu exits 
with "assertion ... failed" message, testandset 
function needs to be fixed for your host architecture.

Please notify the mailing list on success or failure.


Regards,

Piotrek



                
__________________________________
Do you Yahoo!?
New and Improved Yahoo! Mail - Send 10MB messages!
http://promotions.yahoo.com/new_mail

diff -ru qemu-0.6.0/exec-all.h qemu-0.6.0-tas/exec-all.h
--- qemu-0.6.0/exec-all.h       2004-07-10 20:20:09.000000000 +0200
+++ qemu-0.6.0-tas/exec-all.h   2004-08-04 22:26:15.000000000 +0200
@@ -386,6 +386,12 @@
 extern CPUReadMemoryFunc *io_mem_read[IO_MEM_NB_ENTRIES][4];
 extern void *io_mem_opaque[IO_MEM_NB_ENTRIES];
 
+/*
+ * The testandset function should atomically check value of given variable
+ * and set it to non-zero. If old value of variable was 0, function returns 0.
+ * Otherwise non-zero is returned.
+ */
+
 #ifdef __powerpc__
 static inline int testandset (int *p)
 {
@@ -407,13 +413,11 @@
 #ifdef __i386__
 static inline int testandset (int *p)
 {
-    char ret;
-    long int readval;
-    
-    __asm__ __volatile__ ("lock; cmpxchgl %3, %1; sete %0"
-                          : "=q" (ret), "=m" (*p), "=a" (readval)
-                          : "r" (1), "m" (*p), "a" (0)
-                          : "memory");
+    int ret;
+    __asm__ __volatile__ ("lock; cmpxchgl %2, %3"
+                          : "=a" (ret)
+                          : "a" (0), "r" (1), "m" (*p)
+                          : "cc", "memory");
     return ret;
 }
 #endif
@@ -421,13 +425,11 @@
 #ifdef __x86_64__
 static inline int testandset (int *p)
 {
-    char ret;
-    int readval;
-    
-    __asm__ __volatile__ ("lock; cmpxchgl %3, %1; sete %0"
-                          : "=q" (ret), "=m" (*p), "=a" (readval)
-                          : "r" (1), "m" (*p), "a" (0)
-                          : "memory");
+    int ret;
+    __asm__ __volatile__ ("lock; cmpxchgl %2, %3"
+                          : "=a" (ret)
+                          : "a" (0), "r" (1), "m" (*p)
+                          : "cc", "memory");
     return ret;
 }
 #endif
@@ -499,7 +501,7 @@
                          : "=r" (ret)
                          : "m" (p)
                          : "cc","memory");
-    return ret == 0;
+    return ret;
 }
 #endif
 
diff -ru qemu-0.6.0/exec.c qemu-0.6.0-tas/exec.c
--- qemu-0.6.0/exec.c   2004-07-10 20:20:09.000000000 +0200
+++ qemu-0.6.0-tas/exec.c       2004-08-04 21:58:48.000000000 +0200
@@ -1138,7 +1138,7 @@
 void cpu_interrupt(CPUState *env, int mask)
 {
     TranslationBlock *tb;
-    static int interrupt_lock;
+    static int interrupt_lock = 0;
 
     env->interrupt_request |= mask;
     /* if the cpu is currently executing code, we must unlink it and

diff -ru qemu-0.6.0/vl.c qemu-0.6.0-tas/vl.c
--- qemu-0.6.0/vl.c     2004-07-10 20:20:09.000000000 +0200
+++ qemu-0.6.0-tas/vl.c 2004-08-04 16:55:01.000000000 +0200
@@ -23,6 +23,7 @@
  */
 #include "vl.h"
 
+#include <assert.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <signal.h>
@@ -2251,6 +2252,15 @@
 
 #endif
 
+void verify_testandset()
+{
+    int lock = 0;
+    assert(testandset(&lock) == 0);
+    assert(lock != 0);
+    assert(testandset(&lock) != 0);
+    assert(lock != 0);
+}
+
 #define NET_IF_TUN   0
 #define NET_IF_USER  1
 #define NET_IF_DUMMY 2
@@ -2274,6 +2284,7 @@
     int optind;
     const char *r, *optarg;
 
+    verify_testandset();
 #if !defined(CONFIG_SOFTMMU)
     /* we never want that malloc() uses mmap() */
     mallopt(M_MMAP_THRESHOLD, 4096 * 1024);