[Qemu-devel] Re: [RfC PATCH 08/11] spice: add qxl device

[Gerd Hoffmann <kraxel@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

On 04/15/10 01:08, Paolo Bonzini wrote:
> On 04/14/2010 06:52 PM, Blue Swirl wrote:
>> On 4/14/10, Gerd Hoffmann<kraxel@redhat.com> wrote:
>>> +static inline void atomic_or(uint32_t *var, uint32_t add)
>>> +{
>>> + __asm__ __volatile__ ("lock; orl %1, %0" : "+m" (*var) : "r" (add)
>>> : "memory");
>>> +}
>>
>> This will break on non-x86 hosts.
>
> I'd just use __sync_fetch_and_or here.

Good idea.  I think we can zap the memory barrier and fix a small race 
while being at it, see the incremental fix below.

cheers,
   Gerd

[-- Attachment #2: fix --]
[-- Type: text/plain, Size: 1138 bytes --]

diff --git a/hw/qxl.c b/hw/qxl.c
index 7ac06f6..8cbd9a3 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -6,7 +6,6 @@
 #include <pthread.h>
 
 #include "qemu-common.h"
-#include "qemu-barrier.h"
 #include "qemu-spice.h"
 #include "qemu-timer.h"
 #include "qemu-queue.h"
@@ -176,11 +175,6 @@ static inline uint32_t msb_mask(uint32_t val)
     return mask;
 }
 
-static inline void atomic_or(uint32_t *var, uint32_t add)
-{
-   __asm__ __volatile__ ("lock; orl %1, %0" : "+m" (*var) : "r" (add) : "memory");
-}
-
 static ram_addr_t qxl_rom_size(void)
 {
     uint32_t rom_size = sizeof(QXLRom) + sizeof(QXLModes) + sizeof(qxl_modes);
@@ -892,12 +886,13 @@ static void pipe_read(void *opaque)
 
 static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
 {
+    uint32_t old_pending;
+
     assert(d->ssd.running);
-    smp_wmb();
-    if ((d->ram->int_pending & events) == events) {
+    old_pending = __sync_fetch_and_or(&d->ram->int_pending, events);
+    if ((old_pending & events) == events) {
         return;
     }
-    atomic_or(&d->ram->int_pending, events);
     if (pthread_self() == d->main) {
         qxl_set_irq(d);
     } else {