Message ID | 1428437400-8474-7-git-send-email-peter.maydell@linaro.org |
---|---|
State | New |
Headers | show |
On 07/04/2015 22:09, Peter Maydell wrote: > Make address_space_rw take transaction attributes, rather > than always using the 'unspecified' attributes. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > 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/dma-helpers.c b/dma-helpers.c > index 6918572..33b1983 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -28,7 +28,8 @@ int dma_memory_set(AddressSpace *as, dma_addr_t addr, uint8_t c, dma_addr_t len) > memset(fillbuf, c, FILLBUF_SIZE); > while (len > 0) { > l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; > - error |= address_space_rw(as, addr, fillbuf, l, true); > + error |= address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, > + fillbuf, l, true); > len -= l; > addr += l; > } > diff --git a/exec.c b/exec.c > index 7e53f52..29a12fa 100644 > --- a/exec.c > +++ b/exec.c > @@ -1946,13 +1946,16 @@ static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data, > { > subpage_t *subpage = opaque; > uint8_t buf[8]; > + MemTxResult res; > > #if defined(DEBUG_SUBPAGE) > printf("%s: subpage %p len %u addr " TARGET_FMT_plx "\n", __func__, > subpage, len, addr); > #endif > - if (address_space_read(subpage->as, addr + subpage->base, buf, len)) { > - return MEMTX_DECODE_ERROR; > + res = address_space_read(subpage->as, addr + subpage->base, > + attrs, buf, len); > + if (res) { > + return res; > } > switch (len) { > case 1: > @@ -1999,10 +2002,8 @@ static MemTxResult subpage_write(void *opaque, hwaddr addr, > default: > abort(); > } > - if (address_space_write(subpage->as, addr + subpage->base, buf, len)) { > - return MEMTX_DECODE_ERROR; > - } > - return MEMTX_OK; > + return address_space_write(subpage->as, addr + subpage->base, > + attrs, buf, len); > } > > static bool subpage_accepts(void *opaque, hwaddr addr, > @@ -2313,8 +2314,8 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) > return l; > } > > -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) > { > hwaddr l; > uint8_t *ptr; > @@ -2322,7 +2323,6 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > hwaddr addr1; > MemoryRegion *mr; > MemTxResult result = MEMTX_OK; > - MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > > while (len > 0) { > l = len; > @@ -2406,22 +2406,24 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > return result; > } > > -bool address_space_write(AddressSpace *as, hwaddr addr, > - const uint8_t *buf, int len) > +MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > + const uint8_t *buf, int len) > { > - return address_space_rw(as, addr, (uint8_t *)buf, len, true); > + return address_space_rw(as, addr, attrs, (uint8_t *)buf, len, true); > } > > -bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len) > +MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > + uint8_t *buf, int len) > { > - return address_space_rw(as, addr, buf, len, false); > + return address_space_rw(as, addr, attrs, buf, len, false); > } > > > void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf, > int len, int is_write) > { > - address_space_rw(&address_space_memory, addr, buf, len, is_write); > + address_space_rw(&address_space_memory, addr, MEMTXATTRS_UNSPECIFIED, > + buf, len, is_write); > } > > enum write_rom_type { > @@ -2592,7 +2594,8 @@ void *address_space_map(AddressSpace *as, > memory_region_ref(mr); > bounce.mr = mr; > if (!is_write) { > - address_space_read(as, addr, bounce.buffer, l); > + address_space_read(as, addr, MEMTXATTRS_UNSPECIFIED, > + bounce.buffer, l); > } > > *plen = l; > @@ -2645,7 +2648,8 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len, > return; > } > if (is_write) { > - address_space_write(as, bounce.addr, bounce.buffer, access_len); > + address_space_write(as, bounce.addr, MEMTXATTRS_UNSPECIFIED, > + bounce.buffer, access_len); > } > qemu_vfree(bounce.buffer); > bounce.buffer = NULL; > @@ -2787,7 +2791,7 @@ uint64_t ldq_be_phys(AddressSpace *as, hwaddr addr) > uint32_t ldub_phys(AddressSpace *as, hwaddr addr) > { > uint8_t val; > - address_space_rw(as, addr, &val, 1, 0); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, &val, 1, 0); > return val; > } > > @@ -2941,7 +2945,7 @@ void stl_be_phys(AddressSpace *as, hwaddr addr, uint32_t val) > void stb_phys(AddressSpace *as, hwaddr addr, uint32_t val) > { > uint8_t v = val; > - address_space_rw(as, addr, &v, 1, 1); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, &v, 1, 1); > } > > /* warning: addr must be aligned */ > @@ -3004,19 +3008,19 @@ void stw_be_phys(AddressSpace *as, hwaddr addr, uint32_t val) > void stq_phys(AddressSpace *as, hwaddr addr, uint64_t val) > { > val = tswap64(val); > - address_space_rw(as, addr, (void *) &val, 8, 1); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); > } > > void stq_le_phys(AddressSpace *as, hwaddr addr, uint64_t val) > { > val = cpu_to_le64(val); > - address_space_rw(as, addr, (void *) &val, 8, 1); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); > } > > void stq_be_phys(AddressSpace *as, hwaddr addr, uint64_t val) > { > val = cpu_to_be64(val); > - address_space_rw(as, addr, (void *) &val, 8, 1); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); > } > > /* virtual memory access for debug (includes writing to ROM) */ > @@ -3040,7 +3044,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > if (is_write) { > cpu_physical_memory_write_rom(cpu->as, phys_addr, buf, l); > } else { > - address_space_rw(cpu->as, phys_addr, buf, l, 0); > + address_space_rw(cpu->as, phys_addr, MEMTXATTRS_UNSPECIFIED, > + buf, l, 0); > } > len -= l; > buf += l; > diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c > index 07f3c27..2c153e0 100644 > --- a/hw/mips/mips_jazz.c > +++ b/hw/mips/mips_jazz.c > @@ -61,7 +61,8 @@ static void main_cpu_reset(void *opaque) > static uint64_t rtc_read(void *opaque, hwaddr addr, unsigned size) > { > uint8_t val; > - address_space_read(&address_space_memory, 0x90000071, &val, 1); > + address_space_read(&address_space_memory, 0x90000071, > + MEMTXATTRS_UNSPECIFIED, &val, 1); > return val; > } > > @@ -69,7 +70,8 @@ static void rtc_write(void *opaque, hwaddr addr, > uint64_t val, unsigned size) > { > uint8_t buf = val & 0xff; > - address_space_write(&address_space_memory, 0x90000071, &buf, 1); > + address_space_write(&address_space_memory, 0x90000071, > + MEMTXATTRS_UNSPECIFIED, &buf, 1); > } > > static const MemoryRegionOps rtc_ops = { > diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c > index 6cea6ff..c63f45d 100644 > --- a/hw/pci-host/prep.c > +++ b/hw/pci-host/prep.c > @@ -140,7 +140,8 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr, > uint8_t buf[4]; > > addr = raven_io_address(s, addr); > - address_space_read(&s->pci_io_as, addr + 0x80000000, buf, size); > + address_space_read(&s->pci_io_as, addr + 0x80000000, > + MEMTXATTRS_UNSPECIFIED, buf, size); > > if (size == 1) { > return buf[0]; > @@ -171,7 +172,8 @@ static void raven_io_write(void *opaque, hwaddr addr, > g_assert_not_reached(); > } > > - address_space_write(&s->pci_io_as, addr + 0x80000000, buf, size); > + address_space_write(&s->pci_io_as, addr + 0x80000000, > + MEMTXATTRS_UNSPECIFIED, buf, size); > } > > static const MemoryRegionOps raven_io_ops = { > diff --git a/include/exec/memory.h b/include/exec/memory.h > index de2849d..4d6afc8 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -1089,41 +1089,50 @@ void address_space_destroy(AddressSpace *as); > /** > * address_space_rw: read from or write to an address space. > * > - * Return true if the operation hit any unassigned memory or encountered an > - * IOMMU fault. > + * Return a MemTxResult indicating whether the operation succeeded > + * or failed (eg unassigned memory, device rejected the transaction, > + * IOMMU fault). > * > * @as: #AddressSpace to be accessed > * @addr: address within that address space > + * @attrs: memory transaction attributes > * @buf: buffer with the data transferred > * @is_write: indicates the transfer direction > */ > -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); > > /** > * address_space_write: write to address space. > * > - * Return true if the operation hit any unassigned memory or encountered an > - * IOMMU fault. > + * Return a MemTxResult indicating whether the operation succeeded > + * or failed (eg unassigned memory, device rejected the transaction, > + * IOMMU fault). > * > * @as: #AddressSpace to be accessed > * @addr: address within that address space > + * @attrs: memory transaction attributes > * @buf: buffer with the data transferred > */ > -bool address_space_write(AddressSpace *as, hwaddr addr, > - const uint8_t *buf, int len); > +MemTxResult address_space_write(AddressSpace *as, hwaddr addr, > + MemTxAttrs attrs, > + const uint8_t *buf, int len); > > /** > * address_space_read: read from an address space. > * > - * Return true if the operation hit any unassigned memory or encountered an > - * IOMMU fault. > + * Return a MemTxResult indicating whether the operation succeeded > + * or failed (eg unassigned memory, device rejected the transaction, > + * IOMMU fault). > * > * @as: #AddressSpace to be accessed > * @addr: address within that address space > + * @attrs: memory transaction attributes > * @buf: buffer with the data transferred > */ > -bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len); > +MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > + uint8_t *buf, int len); > > /* address_space_translate: translate an address range into an address space > * into a MemoryRegion and an address range into that section > 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); > } > > 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: Paolo Bonzini <pbonzini@redhat.com>
On Tue, Apr 07, 2015 at 09:09:52PM +0100, Peter Maydell wrote: > Make address_space_rw take transaction attributes, rather > than always using the 'unspecified' attributes. Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> I guess that we eventually will need to convert the dma_ functions? > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > 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/dma-helpers.c b/dma-helpers.c > index 6918572..33b1983 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -28,7 +28,8 @@ int dma_memory_set(AddressSpace *as, dma_addr_t addr, uint8_t c, dma_addr_t len) > memset(fillbuf, c, FILLBUF_SIZE); > while (len > 0) { > l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; > - error |= address_space_rw(as, addr, fillbuf, l, true); > + error |= address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, > + fillbuf, l, true); > len -= l; > addr += l; > } > diff --git a/exec.c b/exec.c > index 7e53f52..29a12fa 100644 > --- a/exec.c > +++ b/exec.c > @@ -1946,13 +1946,16 @@ static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data, > { > subpage_t *subpage = opaque; > uint8_t buf[8]; > + MemTxResult res; > > #if defined(DEBUG_SUBPAGE) > printf("%s: subpage %p len %u addr " TARGET_FMT_plx "\n", __func__, > subpage, len, addr); > #endif > - if (address_space_read(subpage->as, addr + subpage->base, buf, len)) { > - return MEMTX_DECODE_ERROR; > + res = address_space_read(subpage->as, addr + subpage->base, > + attrs, buf, len); > + if (res) { > + return res; > } > switch (len) { > case 1: > @@ -1999,10 +2002,8 @@ static MemTxResult subpage_write(void *opaque, hwaddr addr, > default: > abort(); > } > - if (address_space_write(subpage->as, addr + subpage->base, buf, len)) { > - return MEMTX_DECODE_ERROR; > - } > - return MEMTX_OK; > + return address_space_write(subpage->as, addr + subpage->base, > + attrs, buf, len); > } > > static bool subpage_accepts(void *opaque, hwaddr addr, > @@ -2313,8 +2314,8 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) > return l; > } > > -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) > { > hwaddr l; > uint8_t *ptr; > @@ -2322,7 +2323,6 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > hwaddr addr1; > MemoryRegion *mr; > MemTxResult result = MEMTX_OK; > - MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > > while (len > 0) { > l = len; > @@ -2406,22 +2406,24 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > return result; > } > > -bool address_space_write(AddressSpace *as, hwaddr addr, > - const uint8_t *buf, int len) > +MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > + const uint8_t *buf, int len) > { > - return address_space_rw(as, addr, (uint8_t *)buf, len, true); > + return address_space_rw(as, addr, attrs, (uint8_t *)buf, len, true); > } > > -bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len) > +MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > + uint8_t *buf, int len) > { > - return address_space_rw(as, addr, buf, len, false); > + return address_space_rw(as, addr, attrs, buf, len, false); > } > > > void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf, > int len, int is_write) > { > - address_space_rw(&address_space_memory, addr, buf, len, is_write); > + address_space_rw(&address_space_memory, addr, MEMTXATTRS_UNSPECIFIED, > + buf, len, is_write); > } > > enum write_rom_type { > @@ -2592,7 +2594,8 @@ void *address_space_map(AddressSpace *as, > memory_region_ref(mr); > bounce.mr = mr; > if (!is_write) { > - address_space_read(as, addr, bounce.buffer, l); > + address_space_read(as, addr, MEMTXATTRS_UNSPECIFIED, > + bounce.buffer, l); > } > > *plen = l; > @@ -2645,7 +2648,8 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len, > return; > } > if (is_write) { > - address_space_write(as, bounce.addr, bounce.buffer, access_len); > + address_space_write(as, bounce.addr, MEMTXATTRS_UNSPECIFIED, > + bounce.buffer, access_len); > } > qemu_vfree(bounce.buffer); > bounce.buffer = NULL; > @@ -2787,7 +2791,7 @@ uint64_t ldq_be_phys(AddressSpace *as, hwaddr addr) > uint32_t ldub_phys(AddressSpace *as, hwaddr addr) > { > uint8_t val; > - address_space_rw(as, addr, &val, 1, 0); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, &val, 1, 0); > return val; > } > > @@ -2941,7 +2945,7 @@ void stl_be_phys(AddressSpace *as, hwaddr addr, uint32_t val) > void stb_phys(AddressSpace *as, hwaddr addr, uint32_t val) > { > uint8_t v = val; > - address_space_rw(as, addr, &v, 1, 1); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, &v, 1, 1); > } > > /* warning: addr must be aligned */ > @@ -3004,19 +3008,19 @@ void stw_be_phys(AddressSpace *as, hwaddr addr, uint32_t val) > void stq_phys(AddressSpace *as, hwaddr addr, uint64_t val) > { > val = tswap64(val); > - address_space_rw(as, addr, (void *) &val, 8, 1); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); > } > > void stq_le_phys(AddressSpace *as, hwaddr addr, uint64_t val) > { > val = cpu_to_le64(val); > - address_space_rw(as, addr, (void *) &val, 8, 1); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); > } > > void stq_be_phys(AddressSpace *as, hwaddr addr, uint64_t val) > { > val = cpu_to_be64(val); > - address_space_rw(as, addr, (void *) &val, 8, 1); > + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); > } > > /* virtual memory access for debug (includes writing to ROM) */ > @@ -3040,7 +3044,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > if (is_write) { > cpu_physical_memory_write_rom(cpu->as, phys_addr, buf, l); > } else { > - address_space_rw(cpu->as, phys_addr, buf, l, 0); > + address_space_rw(cpu->as, phys_addr, MEMTXATTRS_UNSPECIFIED, > + buf, l, 0); > } > len -= l; > buf += l; > diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c > index 07f3c27..2c153e0 100644 > --- a/hw/mips/mips_jazz.c > +++ b/hw/mips/mips_jazz.c > @@ -61,7 +61,8 @@ static void main_cpu_reset(void *opaque) > static uint64_t rtc_read(void *opaque, hwaddr addr, unsigned size) > { > uint8_t val; > - address_space_read(&address_space_memory, 0x90000071, &val, 1); > + address_space_read(&address_space_memory, 0x90000071, > + MEMTXATTRS_UNSPECIFIED, &val, 1); > return val; > } > > @@ -69,7 +70,8 @@ static void rtc_write(void *opaque, hwaddr addr, > uint64_t val, unsigned size) > { > uint8_t buf = val & 0xff; > - address_space_write(&address_space_memory, 0x90000071, &buf, 1); > + address_space_write(&address_space_memory, 0x90000071, > + MEMTXATTRS_UNSPECIFIED, &buf, 1); > } > > static const MemoryRegionOps rtc_ops = { > diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c > index 6cea6ff..c63f45d 100644 > --- a/hw/pci-host/prep.c > +++ b/hw/pci-host/prep.c > @@ -140,7 +140,8 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr, > uint8_t buf[4]; > > addr = raven_io_address(s, addr); > - address_space_read(&s->pci_io_as, addr + 0x80000000, buf, size); > + address_space_read(&s->pci_io_as, addr + 0x80000000, > + MEMTXATTRS_UNSPECIFIED, buf, size); > > if (size == 1) { > return buf[0]; > @@ -171,7 +172,8 @@ static void raven_io_write(void *opaque, hwaddr addr, > g_assert_not_reached(); > } > > - address_space_write(&s->pci_io_as, addr + 0x80000000, buf, size); > + address_space_write(&s->pci_io_as, addr + 0x80000000, > + MEMTXATTRS_UNSPECIFIED, buf, size); > } > > static const MemoryRegionOps raven_io_ops = { > diff --git a/include/exec/memory.h b/include/exec/memory.h > index de2849d..4d6afc8 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -1089,41 +1089,50 @@ void address_space_destroy(AddressSpace *as); > /** > * address_space_rw: read from or write to an address space. > * > - * Return true if the operation hit any unassigned memory or encountered an > - * IOMMU fault. > + * Return a MemTxResult indicating whether the operation succeeded > + * or failed (eg unassigned memory, device rejected the transaction, > + * IOMMU fault). > * > * @as: #AddressSpace to be accessed > * @addr: address within that address space > + * @attrs: memory transaction attributes > * @buf: buffer with the data transferred > * @is_write: indicates the transfer direction > */ > -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); > > /** > * address_space_write: write to address space. > * > - * Return true if the operation hit any unassigned memory or encountered an > - * IOMMU fault. > + * Return a MemTxResult indicating whether the operation succeeded > + * or failed (eg unassigned memory, device rejected the transaction, > + * IOMMU fault). > * > * @as: #AddressSpace to be accessed > * @addr: address within that address space > + * @attrs: memory transaction attributes > * @buf: buffer with the data transferred > */ > -bool address_space_write(AddressSpace *as, hwaddr addr, > - const uint8_t *buf, int len); > +MemTxResult address_space_write(AddressSpace *as, hwaddr addr, > + MemTxAttrs attrs, > + const uint8_t *buf, int len); > > /** > * address_space_read: read from an address space. > * > - * Return true if the operation hit any unassigned memory or encountered an > - * IOMMU fault. > + * Return a MemTxResult indicating whether the operation succeeded > + * or failed (eg unassigned memory, device rejected the transaction, > + * IOMMU fault). > * > * @as: #AddressSpace to be accessed > * @addr: address within that address space > + * @attrs: memory transaction attributes > * @buf: buffer with the data transferred > */ > -bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len); > +MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > + uint8_t *buf, int len); > > /* address_space_translate: translate an address range into an address space > * into a MemoryRegion and an address range into that section > 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); > } > > 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). > -- > 1.9.1 >
On 9 April 2015 at 10:59, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > On Tue, Apr 07, 2015 at 09:09:52PM +0100, Peter Maydell wrote: >> Make address_space_rw take transaction attributes, rather >> than always using the 'unspecified' attributes. > > Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > I guess that we eventually will need to convert the dma_ > functions? Probably, though I'm not clear what they bring to the party that the basic address_space_* functions don't (part of why I left them alone). -- PMM
On 09/04/2015 12:14, Peter Maydell wrote: > On 9 April 2015 at 10:59, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: >> > On Tue, Apr 07, 2015 at 09:09:52PM +0100, Peter Maydell wrote: >>> >> Make address_space_rw take transaction attributes, rather >>> >> than always using the 'unspecified' attributes. >> > >> > Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> >> > >> > I guess that we eventually will need to convert the dma_ >> > functions? > Probably, though I'm not clear what they bring to the party > that the basic address_space_* functions don't (part of why > I left them alone). At this point, some memory barriers, basically. Initially the IOMMU implementation was done in the dma_* functions (using something called IIRC a DMAContext), but now they just take an AddressSpace. Paolo
On 9 April 2015 at 11:21, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 09/04/2015 12:14, Peter Maydell wrote: >> On 9 April 2015 at 10:59, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: >>> > On Tue, Apr 07, 2015 at 09:09:52PM +0100, Peter Maydell wrote: >>>> >> Make address_space_rw take transaction attributes, rather >>>> >> than always using the 'unspecified' attributes. >>> > >>> > Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> >>> > >>> > I guess that we eventually will need to convert the dma_ >>> > functions? >> Probably, though I'm not clear what they bring to the party >> that the basic address_space_* functions don't (part of why >> I left them alone). > > At this point, some memory barriers, basically. So what distinguishes a device that needs the memory barriers and does its accesses via dma_* from a device that doesn't and uses address_space_* or ld/st*_phys ? (Or for that matter a non-device that does memory accesses...) thanks -- PMM
On 09/04/2015 12:43, Peter Maydell wrote: > > At this point, some memory barriers, basically. > > So what distinguishes a device that needs the memory barriers > and does its accesses via dma_* from a device that doesn't and > uses address_space_* or ld/st*_phys ? (Or for that matter a > non-device that does memory accesses...) I don't know exactly, I didn't follow the discussion very much back then. The memory barriers were fixing PPC bugs; PCI devices definitely need them. Paolo
On 9 April 2015 at 12:40, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 09/04/2015 12:43, Peter Maydell wrote: >> > At this point, some memory barriers, basically. >> >> So what distinguishes a device that needs the memory barriers >> and does its accesses via dma_* from a device that doesn't and >> uses address_space_* or ld/st*_phys ? (Or for that matter a >> non-device that does memory accesses...) > > I don't know exactly, I didn't follow the discussion very much back > then. The memory barriers were fixing PPC bugs; PCI devices definitely > need them. I suspect that actually we need barriers in a lot more places and there shouldn't really be a separate set of APIs here; it's just that nobody's tried using the wider set of devices in PPC KVM systems. -- PMM
diff --git a/dma-helpers.c b/dma-helpers.c index 6918572..33b1983 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -28,7 +28,8 @@ int dma_memory_set(AddressSpace *as, dma_addr_t addr, uint8_t c, dma_addr_t len) memset(fillbuf, c, FILLBUF_SIZE); while (len > 0) { l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; - error |= address_space_rw(as, addr, fillbuf, l, true); + error |= address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, + fillbuf, l, true); len -= l; addr += l; } diff --git a/exec.c b/exec.c index 7e53f52..29a12fa 100644 --- a/exec.c +++ b/exec.c @@ -1946,13 +1946,16 @@ static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data, { subpage_t *subpage = opaque; uint8_t buf[8]; + MemTxResult res; #if defined(DEBUG_SUBPAGE) printf("%s: subpage %p len %u addr " TARGET_FMT_plx "\n", __func__, subpage, len, addr); #endif - if (address_space_read(subpage->as, addr + subpage->base, buf, len)) { - return MEMTX_DECODE_ERROR; + res = address_space_read(subpage->as, addr + subpage->base, + attrs, buf, len); + if (res) { + return res; } switch (len) { case 1: @@ -1999,10 +2002,8 @@ static MemTxResult subpage_write(void *opaque, hwaddr addr, default: abort(); } - if (address_space_write(subpage->as, addr + subpage->base, buf, len)) { - return MEMTX_DECODE_ERROR; - } - return MEMTX_OK; + return address_space_write(subpage->as, addr + subpage->base, + attrs, buf, len); } static bool subpage_accepts(void *opaque, hwaddr addr, @@ -2313,8 +2314,8 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) return l; } -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) { hwaddr l; uint8_t *ptr; @@ -2322,7 +2323,6 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, hwaddr addr1; MemoryRegion *mr; MemTxResult result = MEMTX_OK; - MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; while (len > 0) { l = len; @@ -2406,22 +2406,24 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, return result; } -bool address_space_write(AddressSpace *as, hwaddr addr, - const uint8_t *buf, int len) +MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, + const uint8_t *buf, int len) { - return address_space_rw(as, addr, (uint8_t *)buf, len, true); + return address_space_rw(as, addr, attrs, (uint8_t *)buf, len, true); } -bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len) +MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, + uint8_t *buf, int len) { - return address_space_rw(as, addr, buf, len, false); + return address_space_rw(as, addr, attrs, buf, len, false); } void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf, int len, int is_write) { - address_space_rw(&address_space_memory, addr, buf, len, is_write); + address_space_rw(&address_space_memory, addr, MEMTXATTRS_UNSPECIFIED, + buf, len, is_write); } enum write_rom_type { @@ -2592,7 +2594,8 @@ void *address_space_map(AddressSpace *as, memory_region_ref(mr); bounce.mr = mr; if (!is_write) { - address_space_read(as, addr, bounce.buffer, l); + address_space_read(as, addr, MEMTXATTRS_UNSPECIFIED, + bounce.buffer, l); } *plen = l; @@ -2645,7 +2648,8 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len, return; } if (is_write) { - address_space_write(as, bounce.addr, bounce.buffer, access_len); + address_space_write(as, bounce.addr, MEMTXATTRS_UNSPECIFIED, + bounce.buffer, access_len); } qemu_vfree(bounce.buffer); bounce.buffer = NULL; @@ -2787,7 +2791,7 @@ uint64_t ldq_be_phys(AddressSpace *as, hwaddr addr) uint32_t ldub_phys(AddressSpace *as, hwaddr addr) { uint8_t val; - address_space_rw(as, addr, &val, 1, 0); + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, &val, 1, 0); return val; } @@ -2941,7 +2945,7 @@ void stl_be_phys(AddressSpace *as, hwaddr addr, uint32_t val) void stb_phys(AddressSpace *as, hwaddr addr, uint32_t val) { uint8_t v = val; - address_space_rw(as, addr, &v, 1, 1); + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, &v, 1, 1); } /* warning: addr must be aligned */ @@ -3004,19 +3008,19 @@ void stw_be_phys(AddressSpace *as, hwaddr addr, uint32_t val) void stq_phys(AddressSpace *as, hwaddr addr, uint64_t val) { val = tswap64(val); - address_space_rw(as, addr, (void *) &val, 8, 1); + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); } void stq_le_phys(AddressSpace *as, hwaddr addr, uint64_t val) { val = cpu_to_le64(val); - address_space_rw(as, addr, (void *) &val, 8, 1); + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); } void stq_be_phys(AddressSpace *as, hwaddr addr, uint64_t val) { val = cpu_to_be64(val); - address_space_rw(as, addr, (void *) &val, 8, 1); + address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, (void *) &val, 8, 1); } /* virtual memory access for debug (includes writing to ROM) */ @@ -3040,7 +3044,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, if (is_write) { cpu_physical_memory_write_rom(cpu->as, phys_addr, buf, l); } else { - address_space_rw(cpu->as, phys_addr, buf, l, 0); + address_space_rw(cpu->as, phys_addr, MEMTXATTRS_UNSPECIFIED, + buf, l, 0); } len -= l; buf += l; diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c index 07f3c27..2c153e0 100644 --- a/hw/mips/mips_jazz.c +++ b/hw/mips/mips_jazz.c @@ -61,7 +61,8 @@ static void main_cpu_reset(void *opaque) static uint64_t rtc_read(void *opaque, hwaddr addr, unsigned size) { uint8_t val; - address_space_read(&address_space_memory, 0x90000071, &val, 1); + address_space_read(&address_space_memory, 0x90000071, + MEMTXATTRS_UNSPECIFIED, &val, 1); return val; } @@ -69,7 +70,8 @@ static void rtc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { uint8_t buf = val & 0xff; - address_space_write(&address_space_memory, 0x90000071, &buf, 1); + address_space_write(&address_space_memory, 0x90000071, + MEMTXATTRS_UNSPECIFIED, &buf, 1); } static const MemoryRegionOps rtc_ops = { diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c index 6cea6ff..c63f45d 100644 --- a/hw/pci-host/prep.c +++ b/hw/pci-host/prep.c @@ -140,7 +140,8 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr, uint8_t buf[4]; addr = raven_io_address(s, addr); - address_space_read(&s->pci_io_as, addr + 0x80000000, buf, size); + address_space_read(&s->pci_io_as, addr + 0x80000000, + MEMTXATTRS_UNSPECIFIED, buf, size); if (size == 1) { return buf[0]; @@ -171,7 +172,8 @@ static void raven_io_write(void *opaque, hwaddr addr, g_assert_not_reached(); } - address_space_write(&s->pci_io_as, addr + 0x80000000, buf, size); + address_space_write(&s->pci_io_as, addr + 0x80000000, + MEMTXATTRS_UNSPECIFIED, buf, size); } static const MemoryRegionOps raven_io_ops = { diff --git a/include/exec/memory.h b/include/exec/memory.h index de2849d..4d6afc8 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1089,41 +1089,50 @@ void address_space_destroy(AddressSpace *as); /** * address_space_rw: read from or write to an address space. * - * Return true if the operation hit any unassigned memory or encountered an - * IOMMU fault. + * Return a MemTxResult indicating whether the operation succeeded + * or failed (eg unassigned memory, device rejected the transaction, + * IOMMU fault). * * @as: #AddressSpace to be accessed * @addr: address within that address space + * @attrs: memory transaction attributes * @buf: buffer with the data transferred * @is_write: indicates the transfer direction */ -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); /** * address_space_write: write to address space. * - * Return true if the operation hit any unassigned memory or encountered an - * IOMMU fault. + * Return a MemTxResult indicating whether the operation succeeded + * or failed (eg unassigned memory, device rejected the transaction, + * IOMMU fault). * * @as: #AddressSpace to be accessed * @addr: address within that address space + * @attrs: memory transaction attributes * @buf: buffer with the data transferred */ -bool address_space_write(AddressSpace *as, hwaddr addr, - const uint8_t *buf, int len); +MemTxResult address_space_write(AddressSpace *as, hwaddr addr, + MemTxAttrs attrs, + const uint8_t *buf, int len); /** * address_space_read: read from an address space. * - * Return true if the operation hit any unassigned memory or encountered an - * IOMMU fault. + * Return a MemTxResult indicating whether the operation succeeded + * or failed (eg unassigned memory, device rejected the transaction, + * IOMMU fault). * * @as: #AddressSpace to be accessed * @addr: address within that address space + * @attrs: memory transaction attributes * @buf: buffer with the data transferred */ -bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len); +MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, + uint8_t *buf, int len); /* address_space_translate: translate an address range into an address space * into a MemoryRegion and an address range into that section 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); } 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).
Make address_space_rw take transaction attributes, rather than always using the 'unspecified' attributes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- 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(-)