From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46656) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YgAyN-0006im-GN for qemu-devel@nongnu.org; Thu, 09 Apr 2015 07:50:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YgAyK-00006F-8l for qemu-devel@nongnu.org; Thu, 09 Apr 2015 07:50:15 -0400 Received: from mail-ig0-f180.google.com ([209.85.213.180]:33271) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YgAyK-00006B-48 for qemu-devel@nongnu.org; Thu, 09 Apr 2015 07:50:12 -0400 Received: by ignm3 with SMTP id m3so47323024ign.0 for ; Thu, 09 Apr 2015 04:50:11 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <55250AFD.3060203@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> From: Peter Maydell Date: Thu, 9 Apr 2015 12:49:51 +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 8 April 2015 at 12:03, Paolo Bonzini wrote: > > > On 07/04/2015 22:09, Peter Maydell wrote: >> +#ifdef NEED_CPU_H >> +uint32_t address_space_lduw(AddressSpace *as, hwaddr addr, >> + MemTxAttrs attrs, MemTxResult *result); >> +uint32_t address_space_ldl(AddressSpace *as, hwaddr addr, >> + MemTxAttrs attrs, MemTxResult *result); >> +uint64_t address_space_ldq(AddressSpace *as, hwaddr addr, >> + MemTxAttrs attrs, MemTxResult *result); >> +void address_space_stl_notdirty(AddressSpace *as, hwaddr addr, uint32_t val, >> + MemTxAttrs attrs, MemTxResult *result); >> +void address_space_stw(AddressSpace *as, hwaddr addr, uint32_t val, >> + MemTxAttrs attrs, MemTxResult *result); >> +void address_space_stl(AddressSpace *as, hwaddr addr, uint32_t val, >> + MemTxAttrs attrs, MemTxResult *result); >> +void address_space_stq(AddressSpace *as, hwaddr addr, uint64_t val, >> + MemTxAttrs attrs, MemTxResult *result); >> +#endif > > 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. -- PMM