[Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Laurent Desnogues]

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

Hello,

this patch is based on Vince Weaver patch for locked loads/stores.
It was checked against Alpha architecture manual.

Two fixes were done to Vince's patch:

   - use a comparison to 1 for lock instead of 0 to be closer to the
     Alpha spec
   - fix reading of cpu_lock in gen_qemu_stql_c.

On top of Vince's modifications, a new flag was added to
gen_store_mem to allocate local temps instead of temps;  this flag
should be set when the tcg_gen_qemu_store callback uses brcond
before using the temps or else liveness analysis will get rid of the
temps.

This also adds lock printing in cpu_dump_state which can help
debug.


Laurent

Signed-off-by: Laurent Desnogues <laurent.desnogues@gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: alpha-ld-st-lock.patch --]
[-- Type: text/x-patch; name=alpha-ld-st-lock.patch, Size: 5406 bytes --]

Index: target-alpha/helper.c
===================================================================
--- target-alpha/helper.c	(revision 5643)
+++ target-alpha/helper.c	(working copy)
@@ -434,5 +434,6 @@
         if ((i % 3) == 2)
             cpu_fprintf(f, "\n");
     }
+    cpu_fprintf(f, "\nlock %d\n", env->lock);
 }
 
Index: target-alpha/translate.c
===================================================================
--- target-alpha/translate.c	(revision 5643)
+++ target-alpha/translate.c	(working copy)
@@ -138,13 +138,13 @@
 
 static always_inline void gen_qemu_ldl_l (TCGv t0, TCGv t1, int flags)
 {
-    tcg_gen_mov_i64(cpu_lock, t1);
+    tcg_gen_movi_i64(cpu_lock, 1);
     tcg_gen_qemu_ld32s(t0, t1, flags);
 }
 
 static always_inline void gen_qemu_ldq_l (TCGv t0, TCGv t1, int flags)
 {
-    tcg_gen_mov_i64(cpu_lock, t1);
+    tcg_gen_movi_i64(cpu_lock, 1);
     tcg_gen_qemu_ld64(t0, t1, flags);
 }
 
@@ -201,42 +201,38 @@
 
 static always_inline void gen_qemu_stl_c (TCGv t0, TCGv t1, int flags)
 {
-    int l1, l2;
+    int l1;
 
     l1 = gen_new_label();
-    l2 = gen_new_label();
-    tcg_gen_brcond_i64(TCG_COND_NE, cpu_lock, t1, l1);
+    tcg_gen_brcondi_i64(TCG_COND_NE, cpu_lock, 1, l1);
     tcg_gen_qemu_st32(t0, t1, flags);
-    tcg_gen_movi_i64(t0, 0);
-    tcg_gen_br(l2);
     gen_set_label(l1);
-    tcg_gen_movi_i64(t0, 1);
-    gen_set_label(l2);
-    tcg_gen_movi_i64(cpu_lock, -1);
+    tcg_gen_mov_i64(t0, cpu_lock);
+    tcg_gen_movi_i64(cpu_lock, 0);
 }
 
 static always_inline void gen_qemu_stq_c (TCGv t0, TCGv t1, int flags)
 {
-    int l1, l2;
+    int l1;
 
     l1 = gen_new_label();
-    l2 = gen_new_label();
-    tcg_gen_brcond_i64(TCG_COND_NE, cpu_lock, t1, l1);
+    tcg_gen_brcondi_i64(TCG_COND_NE, cpu_lock, 1, l1);
     tcg_gen_qemu_st64(t0, t1, flags);
-    tcg_gen_movi_i64(t0, 0);
-    tcg_gen_br(l2);
     gen_set_label(l1);
-    tcg_gen_movi_i64(t0, 1);
-    gen_set_label(l2);
-    tcg_gen_movi_i64(cpu_lock, -1);
+    tcg_gen_mov_i64(t0, cpu_lock);
+    tcg_gen_movi_i64(cpu_lock, 0);
 }
 
 static always_inline void gen_store_mem (DisasContext *ctx,
                                          void (*tcg_gen_qemu_store)(TCGv t0, TCGv t1, int flags),
                                          int ra, int rb, int32_t disp16,
-                                         int fp, int clear)
+                                         int fp, int clear, int local)
 {
     TCGv addr = tcg_temp_new(TCG_TYPE_I64);
+    if (local)
+        addr = tcg_temp_local_new(TCG_TYPE_I64);
+    else
+        addr = tcg_temp_new(TCG_TYPE_I64);
     if (rb != 31) {
         tcg_gen_addi_i64(addr, cpu_ir[rb], disp16);
         if (clear)
@@ -252,7 +248,11 @@
         else
             tcg_gen_qemu_store(cpu_ir[ra], addr, ctx->mem_idx);
     } else {
-        TCGv zero = tcg_const_i64(0);
+        TCGv zero;
+        if (local)
+            zero = tcg_const_local_i64(0);
+        else
+            zero = tcg_const_i64(0);
         tcg_gen_qemu_store(zero, addr, ctx->mem_idx);
         tcg_temp_free(zero);
     }
@@ -636,15 +636,15 @@
         break;
     case 0x0D:
         /* STW */
-        gen_store_mem(ctx, &tcg_gen_qemu_st16, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st16, ra, rb, disp16, 0, 0, 0);
         break;
     case 0x0E:
         /* STB */
-        gen_store_mem(ctx, &tcg_gen_qemu_st8, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st8, ra, rb, disp16, 0, 0, 0);
         break;
     case 0x0F:
         /* STQ_U */
-        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 1);
+        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 1, 0);
         break;
     case 0x10:
         switch (fn7) {
@@ -2090,19 +2090,19 @@
         break;
     case 0x24:
         /* STF */
-        gen_store_mem(ctx, &gen_qemu_stf, ra, rb, disp16, 1, 0);
+        gen_store_mem(ctx, &gen_qemu_stf, ra, rb, disp16, 1, 0, 0);
         break;
     case 0x25:
         /* STG */
-        gen_store_mem(ctx, &gen_qemu_stg, ra, rb, disp16, 1, 0);
+        gen_store_mem(ctx, &gen_qemu_stg, ra, rb, disp16, 1, 0, 0);
         break;
     case 0x26:
         /* STS */
-        gen_store_mem(ctx, &gen_qemu_sts, ra, rb, disp16, 1, 0);
+        gen_store_mem(ctx, &gen_qemu_sts, ra, rb, disp16, 1, 0, 0);
         break;
     case 0x27:
         /* STT */
-        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 1, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 1, 0, 0);
         break;
     case 0x28:
         /* LDL */
@@ -2122,19 +2122,19 @@
         break;
     case 0x2C:
         /* STL */
-        gen_store_mem(ctx, &tcg_gen_qemu_st32, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st32, ra, rb, disp16, 0, 0, 0);
         break;
     case 0x2D:
         /* STQ */
-        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 0, 0);
         break;
     case 0x2E:
         /* STL_C */
-        gen_store_mem(ctx, &gen_qemu_stl_c, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &gen_qemu_stl_c, ra, rb, disp16, 0, 0, 1);
         break;
     case 0x2F:
         /* STQ_C */
-        gen_store_mem(ctx, &gen_qemu_stq_c, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &gen_qemu_stq_c, ra, rb, disp16, 0, 0, 1);
         break;
     case 0x30:
         /* BR */

Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Aurelien Jarno]

On Fri, Nov 07, 2008 at 12:34:29PM +0100, Laurent Desnogues wrote:
> Hello,
> 
> this patch is based on Vince Weaver patch for locked loads/stores.
> It was checked against Alpha architecture manual.
> 
> Two fixes were done to Vince's patch:
> 
>    - use a comparison to 1 for lock instead of 0 to be closer to the
>      Alpha spec

I don't agree with this part. The current code use a single variable for 
both address and lock_bit to spare a few tests. Basically it sets
cpu_lock to -1 when not locked and stores the address when locked. Your
patch does not compare the address, so it will break multi-threading.

>    - fix reading of cpu_lock in gen_qemu_stql_c.

I agree with this part, I have applied it.

> On top of Vince's modifications, a new flag was added to
> gen_store_mem to allocate local temps instead of temps;  this flag
> should be set when the tcg_gen_qemu_store callback uses brcond
> before using the temps or else liveness analysis will get rid of the
> temps.
> 
> This also adds lock printing in cpu_dump_state which can help
> debug.
> 
> 
> Laurent
> 
> Signed-off-by: Laurent Desnogues <laurent.desnogues@gmail.com>

> Index: target-alpha/helper.c
> ===================================================================
> --- target-alpha/helper.c	(revision 5643)
> +++ target-alpha/helper.c	(working copy)
> @@ -434,5 +434,6 @@
>          if ((i % 3) == 2)
>              cpu_fprintf(f, "\n");
>      }
> +    cpu_fprintf(f, "\nlock %d\n", env->lock);
>  }
>  
> Index: target-alpha/translate.c
> ===================================================================
> --- target-alpha/translate.c	(revision 5643)
> +++ target-alpha/translate.c	(working copy)
> @@ -138,13 +138,13 @@
>  
>  static always_inline void gen_qemu_ldl_l (TCGv t0, TCGv t1, int flags)
>  {
> -    tcg_gen_mov_i64(cpu_lock, t1);
> +    tcg_gen_movi_i64(cpu_lock, 1);
>      tcg_gen_qemu_ld32s(t0, t1, flags);
>  }
>  
>  static always_inline void gen_qemu_ldq_l (TCGv t0, TCGv t1, int flags)
>  {
> -    tcg_gen_mov_i64(cpu_lock, t1);
> +    tcg_gen_movi_i64(cpu_lock, 1);
>      tcg_gen_qemu_ld64(t0, t1, flags);
>  }
>  
> @@ -201,42 +201,38 @@
>  
>  static always_inline void gen_qemu_stl_c (TCGv t0, TCGv t1, int flags)
>  {
> -    int l1, l2;
> +    int l1;
>  
>      l1 = gen_new_label();
> -    l2 = gen_new_label();
> -    tcg_gen_brcond_i64(TCG_COND_NE, cpu_lock, t1, l1);
> +    tcg_gen_brcondi_i64(TCG_COND_NE, cpu_lock, 1, l1);
>      tcg_gen_qemu_st32(t0, t1, flags);
> -    tcg_gen_movi_i64(t0, 0);
> -    tcg_gen_br(l2);
>      gen_set_label(l1);
> -    tcg_gen_movi_i64(t0, 1);
> -    gen_set_label(l2);
> -    tcg_gen_movi_i64(cpu_lock, -1);
> +    tcg_gen_mov_i64(t0, cpu_lock);
> +    tcg_gen_movi_i64(cpu_lock, 0);
>  }
>  
>  static always_inline void gen_qemu_stq_c (TCGv t0, TCGv t1, int flags)
>  {
> -    int l1, l2;
> +    int l1;
>  
>      l1 = gen_new_label();
> -    l2 = gen_new_label();
> -    tcg_gen_brcond_i64(TCG_COND_NE, cpu_lock, t1, l1);
> +    tcg_gen_brcondi_i64(TCG_COND_NE, cpu_lock, 1, l1);
>      tcg_gen_qemu_st64(t0, t1, flags);
> -    tcg_gen_movi_i64(t0, 0);
> -    tcg_gen_br(l2);
>      gen_set_label(l1);
> -    tcg_gen_movi_i64(t0, 1);
> -    gen_set_label(l2);
> -    tcg_gen_movi_i64(cpu_lock, -1);
> +    tcg_gen_mov_i64(t0, cpu_lock);
> +    tcg_gen_movi_i64(cpu_lock, 0);
>  }
>  
>  static always_inline void gen_store_mem (DisasContext *ctx,
>                                           void (*tcg_gen_qemu_store)(TCGv t0, TCGv t1, int flags),
>                                           int ra, int rb, int32_t disp16,
> -                                         int fp, int clear)
> +                                         int fp, int clear, int local)
>  {
>      TCGv addr = tcg_temp_new(TCG_TYPE_I64);
> +    if (local)
> +        addr = tcg_temp_local_new(TCG_TYPE_I64);
> +    else
> +        addr = tcg_temp_new(TCG_TYPE_I64);
>      if (rb != 31) {
>          tcg_gen_addi_i64(addr, cpu_ir[rb], disp16);
>          if (clear)
> @@ -252,7 +248,11 @@
>          else
>              tcg_gen_qemu_store(cpu_ir[ra], addr, ctx->mem_idx);
>      } else {
> -        TCGv zero = tcg_const_i64(0);
> +        TCGv zero;
> +        if (local)
> +            zero = tcg_const_local_i64(0);
> +        else
> +            zero = tcg_const_i64(0);
>          tcg_gen_qemu_store(zero, addr, ctx->mem_idx);
>          tcg_temp_free(zero);
>      }
> @@ -636,15 +636,15 @@
>          break;
>      case 0x0D:
>          /* STW */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st16, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st16, ra, rb, disp16, 0, 0, 0);
>          break;
>      case 0x0E:
>          /* STB */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st8, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st8, ra, rb, disp16, 0, 0, 0);
>          break;
>      case 0x0F:
>          /* STQ_U */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 1);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 1, 0);
>          break;
>      case 0x10:
>          switch (fn7) {
> @@ -2090,19 +2090,19 @@
>          break;
>      case 0x24:
>          /* STF */
> -        gen_store_mem(ctx, &gen_qemu_stf, ra, rb, disp16, 1, 0);
> +        gen_store_mem(ctx, &gen_qemu_stf, ra, rb, disp16, 1, 0, 0);
>          break;
>      case 0x25:
>          /* STG */
> -        gen_store_mem(ctx, &gen_qemu_stg, ra, rb, disp16, 1, 0);
> +        gen_store_mem(ctx, &gen_qemu_stg, ra, rb, disp16, 1, 0, 0);
>          break;
>      case 0x26:
>          /* STS */
> -        gen_store_mem(ctx, &gen_qemu_sts, ra, rb, disp16, 1, 0);
> +        gen_store_mem(ctx, &gen_qemu_sts, ra, rb, disp16, 1, 0, 0);
>          break;
>      case 0x27:
>          /* STT */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 1, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 1, 0, 0);
>          break;
>      case 0x28:
>          /* LDL */
> @@ -2122,19 +2122,19 @@
>          break;
>      case 0x2C:
>          /* STL */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st32, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st32, ra, rb, disp16, 0, 0, 0);
>          break;
>      case 0x2D:
>          /* STQ */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 0, 0);
>          break;
>      case 0x2E:
>          /* STL_C */
> -        gen_store_mem(ctx, &gen_qemu_stl_c, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &gen_qemu_stl_c, ra, rb, disp16, 0, 0, 1);
>          break;
>      case 0x2F:
>          /* STQ_C */
> -        gen_store_mem(ctx, &gen_qemu_stq_c, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &gen_qemu_stq_c, ra, rb, disp16, 0, 0, 1);
>          break;
>      case 0x30:
>          /* BR */


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Laurent Desnogues]

On Fri, Nov 7, 2008 at 3:00 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Fri, Nov 07, 2008 at 12:34:29PM +0100, Laurent Desnogues wrote:
>> Hello,
>>
>> this patch is based on Vince Weaver patch for locked loads/stores.
>> It was checked against Alpha architecture manual.
>>
>> Two fixes were done to Vince's patch:
>>
>>    - use a comparison to 1 for lock instead of 0 to be closer to the
>>      Alpha spec
>
> I don't agree with this part. The current code use a single variable for
> both address and lock_bit to spare a few tests. Basically it sets
> cpu_lock to -1 when not locked and stores the address when locked. Your
> patch does not compare the address, so it will break multi-threading.

My understanding of the Alpha architecture manual is that
if the addresses don't meet certain criteria (which you
simplify to addresses comparison) then failure or success
of st_c is UNPREDICTABLE (I am not shouting, it's the way
they write it :-) unless some lock clearing occurred (cf
section 4.2.5).

I am not arguing, I just wonder...


Laurent

Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Aurelien Jarno]

On Fri, Nov 07, 2008 at 03:19:08PM +0100, Laurent Desnogues wrote:
> On Fri, Nov 7, 2008 at 3:00 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On Fri, Nov 07, 2008 at 12:34:29PM +0100, Laurent Desnogues wrote:
> >> Hello,
> >>
> >> this patch is based on Vince Weaver patch for locked loads/stores.
> >> It was checked against Alpha architecture manual.
> >>
> >> Two fixes were done to Vince's patch:
> >>
> >>    - use a comparison to 1 for lock instead of 0 to be closer to the
> >>      Alpha spec
> >
> > I don't agree with this part. The current code use a single variable for
> > both address and lock_bit to spare a few tests. Basically it sets
> > cpu_lock to -1 when not locked and stores the address when locked. Your
> > patch does not compare the address, so it will break multi-threading.
> 
> My understanding of the Alpha architecture manual is that
> if the addresses don't meet certain criteria (which you
> simplify to addresses comparison) then failure or success
> of st_c is UNPREDICTABLE (I am not shouting, it's the way
> they write it :-) unless some lock clearing occurred (cf
> section 4.2.5).

The manual is actually not really clear. Section 4.2.5 does not really
speak about storing the locked address, while Section 3.1.4 explicitly
mentions a "locked_physical_address register".

The current implementation, now that it is fixed (can someone confirms
that the problem is actually fixed?), matches the pre-TCG
implementation. I am not sure it is the correct one, but if it works for
now and until someone comes with more arguments, I think we should let
the code as now.

Aurelien 

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Laurent Desnogues]

On Fri, Nov 7, 2008 at 4:18 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> The manual is actually not really clear. Section 4.2.5 does not really
> speak about storing the locked address, while Section 3.1.4 explicitly
> mentions a "locked_physical_address register".

Yes, it's really unclear.

> The current implementation, now that it is fixed (can someone confirms
> that the problem is actually fixed?), matches the pre-TCG
> implementation. I am not sure it is the correct one, but if it works for
> now and until someone comes with more arguments, I think we should let
> the code as now.

I agree.  It's probably better than simply ignoring the address.

We need a way to test the behavior on real machines with some
multi-threaded testcase.


Laurent

Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Vince Weaver]

Hello

So this is a follow-up on the problem.

The patch in current SVN (5646) *does not* make my hello world executable 
work.  It works with the patch I had sent to the list.

The binary I am using can be downloaded here:
     http://www.csl.cornell.edu/~vince/misc/hello.alpha.gz

It was generated with gcc version 4.2.4 (Debian 4.2.4-3) on a
debian unstable Alpha machine.

I'm glad to see there's a lot of people interested in this, espcially 
people with a more in-depth understanding of Qemu than I have.

I might take a look at it again, last night I only had sketchy 
documentation available but here at work I have a copy of the Alpha 
Architecture Reference Manual.

Thanks

Vince

Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Aurelien Jarno]

On Fri, Nov 07, 2008 at 12:44:59PM -0500, Vince Weaver wrote:
> Hello
>
> So this is a follow-up on the problem.
>
> The patch in current SVN (5646) *does not* make my hello world executable 
> work.  It works with the patch I had sent to the list.
>
> The binary I am using can be downloaded here:
>     http://www.csl.cornell.edu/~vince/misc/hello.alpha.gz
>
> It was generated with gcc version 4.2.4 (Debian 4.2.4-3) on a
> debian unstable Alpha machine.
>
> I'm glad to see there's a lot of people interested in this, espcially  
> people with a more in-depth understanding of Qemu than I have.
>
> I might take a look at it again, last night I only had sketchy  
> documentation available but here at work I have a copy of the Alpha  
> Architecture Reference Manual.
>

I have checked a new patch that fixes the problem on my side. Could you
confirm it also fixes the problem on your side?

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Laurent Desnogues]

On Sat, Nov 8, 2008 at 10:12 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> I have checked a new patch that fixes the problem on my side. Could you
> confirm it also fixes the problem on your side?

That fixes it for me. Thanks.


Laurent

Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores

[Paul Brook]

> > > I don't agree with this part. The current code use a single variable
> > > for both address and lock_bit to spare a few tests. Basically it sets
> > > cpu_lock to -1 when not locked and stores the address when locked. Your
> > > patch does not compare the address, so it will break multi-threading.
> >
> > My understanding of the Alpha architecture manual is that
> > if the addresses don't meet certain criteria (which you
> > simplify to addresses comparison) then failure or success
> > of st_c is UNPREDICTABLE (I am not shouting, it's the way
> > they write it :-) unless some lock clearing occurred (cf
> > section 4.2.5).
>
> The manual is actually not really clear. Section 4.2.5 does not really
> speak about storing the locked address, while Section 3.1.4 explicitly
> mentions a "locked_physical_address register".

IIUC The whole point of these instructions is to implement syncronisation 
primitives. Typically this is a read-modify-write sequence and you want the 
write to fail if annother sequence happend in between the two operations.

The current code will probably work for UP guests because the OS will clear 
the exclusive lock on a context switch and in practice these instruction 
always occur in matched pairs.  

SMP guests are where things start to get interesting.  You need to ensure that 
multiple CPUs never have the same address locked. In theory this can be done 
with a single global lock token (that can only be held by one CPU at a time). 
In practice you tend to have finer grained address based locking so that you 
don't get contention between independent locks.

On ARM the lock is also broken if *any* instruction writes to the locked 
address. I don't know if this also applies to Alpha, or whether they 
restricted it to just st_c. My guess would be the latter.

Paul