From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KyUUG-0000qj-L1 for qemu-devel@nongnu.org; Fri, 07 Nov 2008 11:47:08 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KyUUF-0000oQ-4a for qemu-devel@nongnu.org; Fri, 07 Nov 2008 11:47:08 -0500 Received: from [199.232.76.173] (port=51723 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KyUUD-0000oD-Sb for qemu-devel@nongnu.org; Fri, 07 Nov 2008 11:47:06 -0500 Received: from mail.codesourcery.com ([65.74.133.4]:37282) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KyUUD-00036G-CW for qemu-devel@nongnu.org; Fri, 07 Nov 2008 11:47:05 -0500 From: Paul Brook Subject: Re: [Qemu-devel] [PATCH] Alpha: fix locked loads/stores Date: Fri, 7 Nov 2008 17:47:01 +0100 References: <761ea48b0811070334lb817c00x38624ca7bb41bb57@mail.gmail.com> <761ea48b0811070619y3a6447e9g7b07dee73703ef6c@mail.gmail.com> <20081107151828.GA353@hall.aurel32.net> In-Reply-To: <20081107151828.GA353@hall.aurel32.net> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200811071647.01965.paul@codesourcery.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Aurelien Jarno > > > 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