From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj8vF-0000LM-2t for qemu-devel@nongnu.org; Fri, 17 Apr 2015 12:15:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yj8vB-0005Cs-Qn for qemu-devel@nongnu.org; Fri, 17 Apr 2015 12:15:17 -0400 Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:60455 helo=socrates.bennee.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj8vB-0005Cl-KO for qemu-devel@nongnu.org; Fri, 17 Apr 2015 12:15:13 -0400 References: <1428931324-4973-1-git-send-email-peter.maydell@linaro.org> <1428931324-4973-6-git-send-email-peter.maydell@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1428931324-4973-6-git-send-email-peter.maydell@linaro.org> Date: Fri, 17 Apr 2015 17:15:32 +0100 Message-ID: <87iocug28r.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 05/14] exec.c: Convert subpage memory ops to _with_attrs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Peter Crosthwaite , patches@linaro.org, "Edgar E. Iglesias" , qemu-devel@nongnu.org, Greg Bellows , Paolo Bonzini , Richard Henderson Peter Maydell writes: > Convert the subpage memory ops to _with_attrs; this will allow > us to pass the attributes through to the underlying access > functions. (Nothing uses the attributes yet.) > > Signed-off-by: Peter Maydell > Reviewed-by: Paolo Bonzini > Reviewed-by: Edgar E. Iglesias > --- > exec.c | 33 +++++++++++++++++++++------------ > 1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/exec.c b/exec.c > index 34dafd2..bba6f26 100644 > --- a/exec.c > +++ b/exec.c > @@ -1941,8 +1941,8 @@ static const MemoryRegionOps watch_mem_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > > -static uint64_t subpage_read(void *opaque, hwaddr addr, > - unsigned len) > +static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data, > + unsigned len, MemTxAttrs attrs) > { > subpage_t *subpage = opaque; > uint8_t buf[8]; > @@ -1951,23 +1951,29 @@ static uint64_t subpage_read(void *opaque, hwaddr addr, > printf("%s: subpage %p len %u addr " TARGET_FMT_plx "\n", __func__, > subpage, len, addr); > #endif > - address_space_read(subpage->as, addr + subpage->base, buf, len); > + if (address_space_read(subpage->as, addr + subpage->base, buf, len)) { > + return MEMTX_DECODE_ERROR; > + } Doesn't this mean we loose potential other error bits that are returned by address_space_read()? Would MEMTX_ERROR ever potentially be returned? > switch (len) { > case 1: > - return ldub_p(buf); > + *data = ldub_p(buf); > + return MEMTX_OK; > case 2: > - return lduw_p(buf); > + *data = lduw_p(buf); > + return MEMTX_OK; > case 4: > - return ldl_p(buf); > + *data = ldl_p(buf); > + return MEMTX_OK; > case 8: > - return ldq_p(buf); > + *data = ldq_p(buf); > + return MEMTX_OK; > default: > abort(); > } > } > > -static void subpage_write(void *opaque, hwaddr addr, > - uint64_t value, unsigned len) > +static MemTxResult subpage_write(void *opaque, hwaddr addr, > + uint64_t value, unsigned len, MemTxAttrs attrs) > { > subpage_t *subpage = opaque; > uint8_t buf[8]; > @@ -1993,7 +1999,10 @@ static void subpage_write(void *opaque, hwaddr addr, > default: > abort(); > } > - address_space_write(subpage->as, addr + subpage->base, buf, len); > + if (address_space_write(subpage->as, addr + subpage->base, buf, len)) { > + return MEMTX_DECODE_ERROR; > + } > + return MEMTX_OK; Same comment as above. > } > > static bool subpage_accepts(void *opaque, hwaddr addr, > @@ -2010,8 +2019,8 @@ static bool subpage_accepts(void *opaque, hwaddr addr, > } > > static const MemoryRegionOps subpage_ops = { > - .read = subpage_read, > - .write = subpage_write, > + .read_with_attrs = subpage_read, > + .write_with_attrs = subpage_write, > .impl.min_access_size = 1, > .impl.max_access_size = 8, > .valid.min_access_size = 1, -- Alex Bennée