[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transa
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes |
Date: |
Wed, 08 Apr 2015 14:55:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 |
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 <address@hidden>
> ---
> 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 <address@hidden>
- [Qemu-devel] [PATCH 11/14] target-arm: Use correct memory attributes for page table walks, (continued)
- [Qemu-devel] [PATCH 04/14] Add MemTxAttrs to the IOTLB, Peter Maydell, 2015/04/07
- [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes, Peter Maydell, 2015/04/07
- Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes, Edgar E. Iglesias, 2015/04/09
- Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes, Peter Maydell, 2015/04/09
- Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes, Paolo Bonzini, 2015/04/09
- Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes, Peter Maydell, 2015/04/09
- Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes, Paolo Bonzini, 2015/04/09
- Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes, Peter Maydell, 2015/04/09
[Qemu-devel] [PATCH 02/14] memory: Add MemTxAttrs, MemTxResult to io_mem_read and io_mem_write, Peter Maydell, 2015/04/07