From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60500) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YgBjk-0003Tb-03 for qemu-devel@nongnu.org; Thu, 09 Apr 2015 08:39:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YgBjd-0002X0-AL for qemu-devel@nongnu.org; Thu, 09 Apr 2015 08:39:11 -0400 Received: from mail-ig0-f177.google.com ([209.85.213.177]:36707) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YgBjd-0002Vs-4d for qemu-devel@nongnu.org; Thu, 09 Apr 2015 08:39:05 -0400 Received: by igblo3 with SMTP id lo3so65045137igb.1 for ; Thu, 09 Apr 2015 05:39:04 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <552669DF.40705@redhat.com> References: <1428437400-8474-1-git-send-email-peter.maydell@linaro.org> <1428437400-8474-8-git-send-email-peter.maydell@linaro.org> <55250AFD.3060203@redhat.com> <552669DF.40705@redhat.com> From: Peter Maydell Date: Thu, 9 Apr 2015 13:38:43 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 07/14] exec.c: Add new address_space_ld*/st* functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Peter Crosthwaite , Patch Tracking , QEMU Developers , Greg Bellows , "Edgar E. Iglesias" , =?UTF-8?B?QWxleCBCZW5uw6ll?= , Richard Henderson On 9 April 2015 at 13:00, Paolo Bonzini wrote: > > > On 09/04/2015 13:49, Peter Maydell wrote: >>> > I think we do not want to expose these at all (or at least, all users >>> > should really be CPUs and hence use *_phys functions). >>> > >>> > S390 is always big-endian, and watch_mem_read/write can use the same >>> > buffer trick as subpages (and in fact should probably use memattrs as well). >>> > >>> > So, please at least add a comment that these functions are deprecated, >>> > and check if watch_mem_read/write should be handled like subpages. >> I looked at the subpages code, and it seems to me that it's the >> other way around -- the subpages code should use these new functions. >> At the moment the subpage handlers use address_space_read/write >> to pull the data into a buffer, and then use the ldl_p/stl_p functions >> to do "read data from target-CPU order buffer into host variable". >> It would be better for them to just directly be able to say "do >> a ld/st in target-CPU order into this host variable", which is >> the purpose of these new functions. Indirecting via a buffer seems >> like an ugly workaround for not having the direct operation. > > Using them in subpage code is fine, but then the subpage code is in > exec.c and can use the _internal version directly (and pass > DEVICE_NATIVE_ENDIAN). Still, usage of these outside exec.c is probably > suspicious. I use them (later in the series) in target-arm/ as well, when I want a CPU-target-endian access which lets me specify memory attributes. > It's at least worth pulling these in cpu-all.h; the whole > contents of cpu-common.h look like a sundry of functions that either are > deprecated or should be declared elsewhere. They need to be in memory.h because cpu-all.h doesn't have all the typedefs. Also, memory.h seems to me clearly the best place, since it's where we declare the MemoryRegion and AddressSpace related functions. It's certainly true that using target-cpu-endian functions in a device is somewhat suspicious, but that's generally true of all those functions, not just these new address_space_* ones. So I think I'm happy to add a comment that the functions are deprecated for use outside target-*/, but I think they are still necessary. -- PMM