[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores |
Date: |
Fri, 7 Nov 2008 16:18:28 +0100 |
User-agent: |
Mutt/1.5.13 (2006-08-11) |
On Fri, Nov 07, 2008 at 03:19:08PM +0100, Laurent Desnogues wrote:
> On Fri, Nov 7, 2008 at 3:00 PM, Aurelien Jarno <address@hidden> 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
`. `' address@hidden | address@hidden
`- people.debian.org/~aurel32 | www.aurel32.net