From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36760) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkSmI-0004DH-GM for qemu-devel@nongnu.org; Tue, 21 Apr 2015 03:39:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YkSmE-0002IK-PF for qemu-devel@nongnu.org; Tue, 21 Apr 2015 03:39:30 -0400 Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:45432 helo=socrates.bennee.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkSmE-0002IA-HU for qemu-devel@nongnu.org; Tue, 21 Apr 2015 03:39:26 -0400 References: <1428931324-4973-1-git-send-email-peter.maydell@linaro.org> <1428931324-4973-7-git-send-email-peter.maydell@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1428931324-4973-7-git-send-email-peter.maydell@linaro.org> Date: Tue, 21 Apr 2015 08:39:47 +0100 Message-ID: <87d22yvsjg.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 06/14] exec.c: Make address_space_rw take transaction attributes 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: > Make address_space_rw take transaction attributes, rather > than always using the 'unspecified' attributes. > > Signed-off-by: Peter Maydell > Reviewed-by: Paolo Bonzini > Reviewed-by: Edgar E. Iglesias > --- > dma-helpers.c | 3 ++- > exec.c | 51 ++++++++++++++++++++++++++---------------------- > hw/mips/mips_jazz.c | 6 ++++-- > hw/pci-host/prep.c | 6 ++++-- > include/exec/memory.h | 31 ++++++++++++++++++----------- > include/sysemu/dma.h | 3 ++- > ioport.c | 16 +++++++++------ > kvm-all.c | 3 ++- > scripts/coverity-model.c | 8 +++++--- > 9 files changed, 77 insertions(+), 50 deletions(-) > > diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h > index 3f2f4c8..efa8b99 100644 > --- a/include/sysemu/dma.h > +++ b/include/sysemu/dma.h > @@ -88,7 +88,8 @@ static inline int dma_memory_rw_relaxed(AddressSpace *as, dma_addr_t addr, > void *buf, dma_addr_t len, > DMADirection dir) > { > - return address_space_rw(as, addr, buf, len, dir == DMA_DIRECTION_FROM_DEVICE); > + return (bool)address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, > + buf, len, dir == DMA_DIRECTION_FROM_DEVICE); > } The return does the right thing but I was wondering if it should be more explicit: return MEMTX_OK==address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED,... ? > > static inline int dma_memory_read_relaxed(AddressSpace *as, dma_addr_t addr, > diff --git a/ioport.c b/ioport.c > index 783a3ae..b345bd9 100644 > --- a/ioport.c > +++ b/ioport.c > @@ -64,7 +64,8 @@ void cpu_outb(pio_addr_t addr, uint8_t val) > { > LOG_IOPORT("outb: %04"FMT_pioaddr" %02"PRIx8"\n", addr, val); > trace_cpu_out(addr, val); > - address_space_write(&address_space_io, addr, &val, 1); > + address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, > + &val, 1); > } > > void cpu_outw(pio_addr_t addr, uint16_t val) > @@ -74,7 +75,8 @@ void cpu_outw(pio_addr_t addr, uint16_t val) > LOG_IOPORT("outw: %04"FMT_pioaddr" %04"PRIx16"\n", addr, val); > trace_cpu_out(addr, val); > stw_p(buf, val); > - address_space_write(&address_space_io, addr, buf, 2); > + address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, > + buf, 2); > } > > void cpu_outl(pio_addr_t addr, uint32_t val) > @@ -84,14 +86,16 @@ void cpu_outl(pio_addr_t addr, uint32_t val) > LOG_IOPORT("outl: %04"FMT_pioaddr" %08"PRIx32"\n", addr, val); > trace_cpu_out(addr, val); > stl_p(buf, val); > - address_space_write(&address_space_io, addr, buf, 4); > + address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, > + buf, 4); > } > > uint8_t cpu_inb(pio_addr_t addr) > { > uint8_t val; > > - address_space_read(&address_space_io, addr, &val, 1); > + address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, > + &val, 1); > trace_cpu_in(addr, val); > LOG_IOPORT("inb : %04"FMT_pioaddr" %02"PRIx8"\n", addr, val); > return val; > @@ -102,7 +106,7 @@ uint16_t cpu_inw(pio_addr_t addr) > uint8_t buf[2]; > uint16_t val; > > - address_space_read(&address_space_io, addr, buf, 2); > + address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 2); > val = lduw_p(buf); > trace_cpu_in(addr, val); > LOG_IOPORT("inw : %04"FMT_pioaddr" %04"PRIx16"\n", addr, val); > @@ -114,7 +118,7 @@ uint32_t cpu_inl(pio_addr_t addr) > uint8_t buf[4]; > uint32_t val; > > - address_space_read(&address_space_io, addr, buf, 4); > + address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 4); > val = ldl_p(buf); > trace_cpu_in(addr, val); > LOG_IOPORT("inl : %04"FMT_pioaddr" %08"PRIx32"\n", addr, val); > diff --git a/kvm-all.c b/kvm-all.c > index dd44f8c..4ec153d 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1667,7 +1667,8 @@ static void kvm_handle_io(uint16_t port, void *data, int direction, int size, > uint8_t *ptr = data; > > for (i = 0; i < count; i++) { > - address_space_rw(&address_space_io, port, ptr, size, > + address_space_rw(&address_space_io, port, MEMTXATTRS_UNSPECIFIED, > + ptr, size, > direction == KVM_EXIT_IO_OUT); > ptr += size; > } > diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c > index cdda259..224d2d1 100644 > --- a/scripts/coverity-model.c > +++ b/scripts/coverity-model.c > @@ -46,6 +46,8 @@ typedef struct va_list_str *va_list; > > typedef struct AddressSpace AddressSpace; > typedef uint64_t hwaddr; > +typedef uint32_t MemTxResult; > +typedef uint64_t MemTxAttrs; > > static void __write(uint8_t *buf, ssize_t len) > { > @@ -65,10 +67,10 @@ static void __read(uint8_t *buf, ssize_t len) > int last = buf[len-1]; > } > > -bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > - int len, bool is_write) > +MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > + uint8_t *buf, int len, bool is_write) > { > - bool result; > + MemTxResult result; > > // TODO: investigate impact of treating reads as producing > // tainted data, with __coverity_tainted_data_argument__(buf). Reviewed-by: Alex Bennée -- Alex Bennée