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

[Paul Brook <paul@codesourcery.com>]

> > > 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