From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1ItMJ6-0007yc-1i for qemu-devel@nongnu.org; Sat, 17 Nov 2007 06:57:52 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1ItMJ1-0007y0-BM for qemu-devel@nongnu.org; Sat, 17 Nov 2007 06:57:51 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ItMJ1-0007xo-5n for qemu-devel@nongnu.org; Sat, 17 Nov 2007 06:57:47 -0500 Received: from ug-out-1314.google.com ([66.249.92.175]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1ItMJ0-0007X0-UP for qemu-devel@nongnu.org; Sat, 17 Nov 2007 06:57:47 -0500 Received: by ug-out-1314.google.com with SMTP id m2so712274uge for ; Sat, 17 Nov 2007 03:57:45 -0800 (PST) Message-ID: Date: Sat, 17 Nov 2007 12:57:45 +0100 From: "andrzej zaborowski" Subject: Re: [Qemu-devel] qemu softmmu_template.h In-Reply-To: <1195297336.5335.57.camel@rapid> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <1195293653.5335.27.camel@rapid> <1195295212.5335.36.camel@rapid> <1195297336.5335.57.camel@rapid> 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 On 17/11/2007, J. Mayer wrote: > > On Sat, 2007-11-17 at 11:44 +0100, andrzej zaborowski wrote: > > On 17/11/2007, J. Mayer wrote: > > > > > > On Sat, 2007-11-17 at 11:14 +0100, andrzej zaborowski wrote: > > > > On 17/11/2007, J. Mayer wrote: > > > > > > > > > > On Sat, 2007-11-17 at 09:53 +0000, Andrzej Zaborowski wrote: > > > > > > CVSROOT: /sources/qemu > > > > > > Module name: qemu > > > > > > Changes by: Andrzej Zaborowski 07/11/17 09:53:42 > > > > > > > > > > > > Modified files: > > > > > > . : softmmu_template.h > > > > > > > > > > > > Log message: > > > > > > Check permissions for the last byte first in unaligned slow_st accesses (patch from TeLeMan). > > > > > > > > > > > > CVSWeb URLs: > > > > > > http://cvs.savannah.gnu.org/viewcvs/qemu/softmmu_template.h?cvsroot=qemu&r1=1.19&r2=1.20 > > > > > > > > > > > > > > > > Has it been checked that it's legal for all architectures and cannot > > > > > have any nasty side effect to do accesses in the reverse order ? Real > > > > > hardware do not ever seem to do this... > > > > > > > > For real hardware the store is a single operation. > > > > > > For PowerPC, at least, only aligned stores are defined as atomic. It's > > > absolutely legal for an implementation to split all non-atomic accesses > > > into smaller aligned accesses. And I guess it is the same for all > > > architecture that can do unaligned accesses. > > > > > > > Logically it shouldn't have any side effects, but if it does then it > > > > would rather mean that other code for that architecture is (also) > > > > broken, I believe. > > > > > > > > I've only tested ARM, mips, x86 and x86_64 before committing, so > > > > please test. I figured that the patch won't get any comments on the > > > > mailing list if it isn't merged. > > > > > > I don't think it's so easy to test because it may be very hard to > > > trigger the cases that would have side effects, which are target > > > dependent. I then am very curious to know how you did check that there > > > is no problem with this patch.... > > > > Well, for ARM, x86 and x86_64 I only checked that unaligned accesses > > still work, i.e. that I haven't made an obvious typo. I haven't tested > > cross-page accesses with the access to the second page being invalid, > > I also don't know how the specifications for other architectures > > define the effect of such accesses, so maybe I shouldn't have > > committed this, but I assumed a common sense in the design of cpu > > archs, meaning that in the example given by TeLeMan the addition is > > not performed two times on some bytes. > > One case that obviously can have nasty side effects is if doing > unaligned IO accesses. Doing accesses from first byte to the last is > very different than doing the access from the last to the first. Hmm, right, I had not thought about IO accesses. I will watch for reports of any breakage that may have any connection with this and revert if there's any such report. > What also can be very different is what is to happen when the > instruction is to be restarted because of a page fault. > I checked the PowerPC specification, and it appears that it allows > splitted memory accesses to be done in any order. It also specifies that > load and stores are restartable even if they have been partially > executed (ie some registers or memory locations have already been > changed), then this patch is likely not to break this target (but I did > not check all specific implementations to see if some have specific > requirements). > This is to be checked for all other targets before such a patch can be > applied, imho. Yes, although in practice that means the workaround (not a proper bugfix) would never be in qemu CVS and would be maintained in other trees endlessly.