| Message ID | 1428437400-8474-3-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 07/04/2015 22:09, Peter Maydell wrote: > Add a MemTxAttrs argument to the io_mem_read() and io_mem_write() > functions, and make them return MemTxResult rather than bool, > thus pushing these new arguments out one level of the callstack > (all the callers callers currently pass MEMTXATTRS_UNSPECIFIED > and convert the return value back to bool or ignore it). > > Note that this involves moving the io_mem_read and io_mem_write > prototypes from exec-all.h to memory.h. This allows them to > see the MemTxResult type, and is a better location anyway, > since they are implemented in memory.c and are operations on > MemoryRegions. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > exec.c | 33 +++++++++++++++++---------------- > hw/s390x/s390-pci-inst.c | 7 ++++--- > hw/vfio/pci.c | 4 ++-- > include/exec/exec-all.h | 4 ---- > include/exec/memory.h | 12 ++++++++++++ > memory.c | 13 ++++++------- > softmmu_template.h | 4 ++-- > 7 files changed, 43 insertions(+), 34 deletions(-) > > diff --git a/exec.c b/exec.c > index 874ecfc..52e7a2a 100644 > --- a/exec.c > +++ b/exec.c > @@ -2312,7 +2312,8 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > uint64_t val; > hwaddr addr1; > MemoryRegion *mr; > - bool error = false; > + MemTxResult result = MEMTX_OK; > + MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > > while (len > 0) { > l = len; > @@ -2327,22 +2328,22 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > case 8: > /* 64 bit write access */ > val = ldq_p(buf); > - error |= io_mem_write(mr, addr1, val, 8); > + result |= io_mem_write(mr, addr1, val, 8, attrs); > break; > case 4: > /* 32 bit write access */ > val = ldl_p(buf); > - error |= io_mem_write(mr, addr1, val, 4); > + result |= io_mem_write(mr, addr1, val, 4, attrs); > break; > case 2: > /* 16 bit write access */ > val = lduw_p(buf); > - error |= io_mem_write(mr, addr1, val, 2); > + result |= io_mem_write(mr, addr1, val, 2, attrs); > break; > case 1: > /* 8 bit write access */ > val = ldub_p(buf); > - error |= io_mem_write(mr, addr1, val, 1); > + result |= io_mem_write(mr, addr1, val, 1, attrs); > break; > default: > abort(); > @@ -2361,22 +2362,22 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > switch (l) { > case 8: > /* 64 bit read access */ > - error |= io_mem_read(mr, addr1, &val, 8); > + result |= io_mem_read(mr, addr1, &val, 8, attrs); > stq_p(buf, val); > break; > case 4: > /* 32 bit read access */ > - error |= io_mem_read(mr, addr1, &val, 4); > + result |= io_mem_read(mr, addr1, &val, 4, attrs); > stl_p(buf, val); > break; > case 2: > /* 16 bit read access */ > - error |= io_mem_read(mr, addr1, &val, 2); > + result |= io_mem_read(mr, addr1, &val, 2, attrs); > stw_p(buf, val); > break; > case 1: > /* 8 bit read access */ > - error |= io_mem_read(mr, addr1, &val, 1); > + result |= io_mem_read(mr, addr1, &val, 1, attrs); > stb_p(buf, val); > break; > default: > @@ -2393,7 +2394,7 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > addr += l; > } > > - return error; > + return result; > } > > bool address_space_write(AddressSpace *as, hwaddr addr, > @@ -2669,7 +2670,7 @@ static inline uint32_t ldl_phys_internal(AddressSpace *as, hwaddr addr, > mr = address_space_translate(as, addr, &addr1, &l, false); > if (l < 4 || !memory_access_is_direct(mr, false)) { > /* I/O case */ > - io_mem_read(mr, addr1, &val, 4); > + io_mem_read(mr, addr1, &val, 4, MEMTXATTRS_UNSPECIFIED); > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > val = bswap32(val); > @@ -2728,7 +2729,7 @@ static inline uint64_t ldq_phys_internal(AddressSpace *as, hwaddr addr, > false); > if (l < 8 || !memory_access_is_direct(mr, false)) { > /* I/O case */ > - io_mem_read(mr, addr1, &val, 8); > + io_mem_read(mr, addr1, &val, 8, MEMTXATTRS_UNSPECIFIED); > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > val = bswap64(val); > @@ -2795,7 +2796,7 @@ static inline uint32_t lduw_phys_internal(AddressSpace *as, hwaddr addr, > false); > if (l < 2 || !memory_access_is_direct(mr, false)) { > /* I/O case */ > - io_mem_read(mr, addr1, &val, 2); > + io_mem_read(mr, addr1, &val, 2, MEMTXATTRS_UNSPECIFIED); > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > val = bswap16(val); > @@ -2853,7 +2854,7 @@ void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val) > mr = address_space_translate(as, addr, &addr1, &l, > true); > if (l < 4 || !memory_access_is_direct(mr, true)) { > - io_mem_write(mr, addr1, val, 4); > + io_mem_write(mr, addr1, val, 4, MEMTXATTRS_UNSPECIFIED); > } else { > addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; > ptr = qemu_get_ram_ptr(addr1); > @@ -2892,7 +2893,7 @@ static inline void stl_phys_internal(AddressSpace *as, > val = bswap32(val); > } > #endif > - io_mem_write(mr, addr1, val, 4); > + io_mem_write(mr, addr1, val, 4, MEMTXATTRS_UNSPECIFIED); > } else { > /* RAM case */ > addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; > @@ -2955,7 +2956,7 @@ static inline void stw_phys_internal(AddressSpace *as, > val = bswap16(val); > } > #endif > - io_mem_write(mr, addr1, val, 2); > + io_mem_write(mr, addr1, val, 2, MEMTXATTRS_UNSPECIFIED); > } else { > /* RAM case */ > addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; > diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c > index 08d8aa6..78ff9c0 100644 > --- a/hw/s390x/s390-pci-inst.c > +++ b/hw/s390x/s390-pci-inst.c > @@ -331,7 +331,7 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) > return 0; > } > MemoryRegion *mr = pbdev->pdev->io_regions[pcias].memory; > - io_mem_read(mr, offset, &data, len); > + io_mem_read(mr, offset, &data, len, MEMTXATTRS_UNSPECIFIED); > } else if (pcias == 15) { > if ((4 - (offset & 0x3)) < len) { > program_interrupt(env, PGM_OPERAND, 4); > @@ -456,7 +456,7 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) > mr = pbdev->pdev->io_regions[pcias].memory; > } > > - io_mem_write(mr, offset, data, len); > + io_mem_write(mr, offset, data, len, MEMTXATTRS_UNSPECIFIED); > } else if (pcias == 15) { > if ((4 - (offset & 0x3)) < len) { > program_interrupt(env, PGM_OPERAND, 4); > @@ -606,7 +606,8 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr) > } > > for (i = 0; i < len / 8; i++) { > - io_mem_write(mr, env->regs[r3] + i * 8, ldq_p(buffer + i * 8), 8); > + io_mem_write(mr, env->regs[r3] + i * 8, ldq_p(buffer + i * 8), 8, > + MEMTXATTRS_UNSPECIFIED); > } > > setcc(cpu, ZPCI_PCI_LS_OK); > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 6b80539..3e1df0c 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -1533,7 +1533,7 @@ static uint64_t vfio_rtl8168_window_quirk_read(void *opaque, > > io_mem_read(&vdev->pdev.msix_table_mmio, > (hwaddr)(quirk->data.address_match & 0xfff), > - &val, size); > + &val, size, MEMTXATTRS_UNSPECIFIED); > return val; > } > } > @@ -1563,7 +1563,7 @@ static void vfio_rtl8168_window_quirk_write(void *opaque, hwaddr addr, > > io_mem_write(&vdev->pdev.msix_table_mmio, > (hwaddr)(quirk->data.address_match & 0xfff), > - data, size); > + data, size, MEMTXATTRS_UNSPECIFIED); > } > > quirk->data.flags = 1; > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 8eb0db3..ff1bc3e 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -341,10 +341,6 @@ void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align)); > > struct MemoryRegion *iotlb_to_region(CPUState *cpu, > hwaddr index); > -bool io_mem_read(struct MemoryRegion *mr, hwaddr addr, > - uint64_t *pvalue, unsigned size); > -bool io_mem_write(struct MemoryRegion *mr, hwaddr addr, > - uint64_t value, unsigned size); > > void tlb_fill(CPUState *cpu, target_ulong addr, int is_write, int mmu_idx, > uintptr_t retaddr); > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 703d9e5..de2849d 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -1053,6 +1053,18 @@ void memory_global_dirty_log_stop(void); > void mtree_info(fprintf_function mon_printf, void *f); > > /** > + * io_mem_read: perform an IO read directly to the specified MemoryRegion > + */ > +MemTxResult io_mem_read(struct MemoryRegion *mr, hwaddr addr, > + uint64_t *pvalue, unsigned size, MemTxAttrs attrs); > + > +/** > + * io_mem_write: perform an IO write directly to the specified MemoryRegion > + */ > +MemTxResult io_mem_write(struct MemoryRegion *mr, hwaddr addr, > + uint64_t value, unsigned size, MemTxAttrs attrs); > + > +/** > * address_space_init: initializes an address space > * > * @as: an uninitialized #AddressSpace > diff --git a/memory.c b/memory.c > index 9bb5674..14cda7f 100644 > --- a/memory.c > +++ b/memory.c > @@ -2063,17 +2063,16 @@ void address_space_destroy(AddressSpace *as) > call_rcu(as, do_address_space_destroy, rcu); > } > > -bool io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, unsigned size) > +MemTxResult io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, > + unsigned size, MemTxAttrs attrs) > { > - return memory_region_dispatch_read(mr, addr, pval, size, > - MEMTXATTRS_UNSPECIFIED); > + return memory_region_dispatch_read(mr, addr, pval, size, attrs); > } > > -bool io_mem_write(MemoryRegion *mr, hwaddr addr, > - uint64_t val, unsigned size) > +MemTxResult io_mem_write(MemoryRegion *mr, hwaddr addr, > + uint64_t val, unsigned size, MemTxAttrs attrs) > { > - return memory_region_dispatch_write(mr, addr, val, size, > - MEMTXATTRS_UNSPECIFIED); > + return memory_region_dispatch_write(mr, addr, val, size, attrs); > } > > typedef struct MemoryRegionList MemoryRegionList; > diff --git a/softmmu_template.h b/softmmu_template.h > index 0e3dd35..4b9bae7 100644 > --- a/softmmu_template.h > +++ b/softmmu_template.h > @@ -158,7 +158,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env, > } > > cpu->mem_io_vaddr = addr; > - io_mem_read(mr, physaddr, &val, 1 << SHIFT); > + io_mem_read(mr, physaddr, &val, 1 << SHIFT, MEMTXATTRS_UNSPECIFIED); > return val; > } > #endif > @@ -378,7 +378,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env, > > cpu->mem_io_vaddr = addr; > cpu->mem_io_pc = retaddr; > - io_mem_write(mr, physaddr, val, 1 << SHIFT); > + io_mem_write(mr, physaddr, val, 1 << SHIFT, MEMTXATTRS_UNSPECIFIED); > } > > void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, > This could be the occasion to drop io_mem_read/io_mem_write and use memory_region_dispatch_read/memory_region_dispatch_write altogether. As you prefer. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
On 8 April 2015 at 11:51, Paolo Bonzini <pbonzini@redhat.com> wrote: > This could be the occasion to drop io_mem_read/io_mem_write and use > memory_region_dispatch_read/memory_region_dispatch_write altogether. As > you prefer. Makes sense, I guess; memory_region_* are better named for what they do, so promoting those to non-static and dropping the wrappers seems a good move. Incidentally in the course of this patch I noticed that we have exactly two places outside the memory system that use these functions: hw/s390x/s390-pci-inst.c and hw/vfio/pci.c. Is this a reasonable thing, or should they in an ideal world have created an AddressSpace to access things through? (I have no opinion except that functions used in only one or two places always look a little suspicious to me :-)) -- PMM
On 08/04/2015 12:59, Peter Maydell wrote: > Incidentally in the course of this patch I noticed that we have > exactly two places outside the memory system that use these > functions: hw/s390x/s390-pci-inst.c and hw/vfio/pci.c. Is > this a reasonable thing, or should they in an ideal world > have created an AddressSpace to access things through? In the case of VFIO, I would just export pci_read/write_msix_table functions from the PCI core. In the case of s390 they would have to create an AddressSpace per BAR per device: the current code doesn't work for BARs backed by a RAM region, or with a complicated layout (aliases and the like). This reflects how s390 PCI is basically only used for VFIO, so it's passable: having a separate AddressSpace per BAR per device probably wouldn't scale too well. Paolo > (I have no opinion except that functions used in only one > or two places always look a little suspicious to me :-))
On Tue, Apr 07, 2015 at 09:09:48PM +0100, Peter Maydell wrote: > Add a MemTxAttrs argument to the io_mem_read() and io_mem_write() > functions, and make them return MemTxResult rather than bool, > thus pushing these new arguments out one level of the callstack > (all the callers callers currently pass MEMTXATTRS_UNSPECIFIED > and convert the return value back to bool or ignore it). > > Note that this involves moving the io_mem_read and io_mem_write > prototypes from exec-all.h to memory.h. This allows them to > see the MemTxResult type, and is a better location anyway, > since they are implemented in memory.c and are operations on > MemoryRegions. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > exec.c | 33 +++++++++++++++++---------------- > hw/s390x/s390-pci-inst.c | 7 ++++--- > hw/vfio/pci.c | 4 ++-- > include/exec/exec-all.h | 4 ---- > include/exec/memory.h | 12 ++++++++++++ > memory.c | 13 ++++++------- > softmmu_template.h | 4 ++-- > 7 files changed, 43 insertions(+), 34 deletions(-) > > diff --git a/exec.c b/exec.c > index 874ecfc..52e7a2a 100644 > --- a/exec.c > +++ b/exec.c > @@ -2312,7 +2312,8 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > uint64_t val; > hwaddr addr1; > MemoryRegion *mr; > - bool error = false; > + MemTxResult result = MEMTX_OK; > + MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > > while (len > 0) { > l = len; > @@ -2327,22 +2328,22 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > case 8: > /* 64 bit write access */ > val = ldq_p(buf); > - error |= io_mem_write(mr, addr1, val, 8); > + result |= io_mem_write(mr, addr1, val, 8, attrs); > break; > case 4: > /* 32 bit write access */ > val = ldl_p(buf); > - error |= io_mem_write(mr, addr1, val, 4); > + result |= io_mem_write(mr, addr1, val, 4, attrs); > break; > case 2: > /* 16 bit write access */ > val = lduw_p(buf); > - error |= io_mem_write(mr, addr1, val, 2); > + result |= io_mem_write(mr, addr1, val, 2, attrs); > break; > case 1: > /* 8 bit write access */ > val = ldub_p(buf); > - error |= io_mem_write(mr, addr1, val, 1); > + result |= io_mem_write(mr, addr1, val, 1, attrs); > break; > default: > abort(); > @@ -2361,22 +2362,22 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > switch (l) { > case 8: > /* 64 bit read access */ > - error |= io_mem_read(mr, addr1, &val, 8); > + result |= io_mem_read(mr, addr1, &val, 8, attrs); > stq_p(buf, val); > break; > case 4: > /* 32 bit read access */ > - error |= io_mem_read(mr, addr1, &val, 4); > + result |= io_mem_read(mr, addr1, &val, 4, attrs); > stl_p(buf, val); > break; > case 2: > /* 16 bit read access */ > - error |= io_mem_read(mr, addr1, &val, 2); > + result |= io_mem_read(mr, addr1, &val, 2, attrs); > stw_p(buf, val); > break; > case 1: > /* 8 bit read access */ > - error |= io_mem_read(mr, addr1, &val, 1); > + result |= io_mem_read(mr, addr1, &val, 1, attrs); > stb_p(buf, val); > break; > default: > @@ -2393,7 +2394,7 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > addr += l; > } > > - return error; > + return result; > } > > bool address_space_write(AddressSpace *as, hwaddr addr, > @@ -2669,7 +2670,7 @@ static inline uint32_t ldl_phys_internal(AddressSpace *as, hwaddr addr, > mr = address_space_translate(as, addr, &addr1, &l, false); > if (l < 4 || !memory_access_is_direct(mr, false)) { > /* I/O case */ > - io_mem_read(mr, addr1, &val, 4); > + io_mem_read(mr, addr1, &val, 4, MEMTXATTRS_UNSPECIFIED); > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > val = bswap32(val); > @@ -2728,7 +2729,7 @@ static inline uint64_t ldq_phys_internal(AddressSpace *as, hwaddr addr, > false); > if (l < 8 || !memory_access_is_direct(mr, false)) { > /* I/O case */ > - io_mem_read(mr, addr1, &val, 8); > + io_mem_read(mr, addr1, &val, 8, MEMTXATTRS_UNSPECIFIED); > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > val = bswap64(val); > @@ -2795,7 +2796,7 @@ static inline uint32_t lduw_phys_internal(AddressSpace *as, hwaddr addr, > false); > if (l < 2 || !memory_access_is_direct(mr, false)) { > /* I/O case */ > - io_mem_read(mr, addr1, &val, 2); > + io_mem_read(mr, addr1, &val, 2, MEMTXATTRS_UNSPECIFIED); > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > val = bswap16(val); > @@ -2853,7 +2854,7 @@ void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val) > mr = address_space_translate(as, addr, &addr1, &l, > true); > if (l < 4 || !memory_access_is_direct(mr, true)) { > - io_mem_write(mr, addr1, val, 4); > + io_mem_write(mr, addr1, val, 4, MEMTXATTRS_UNSPECIFIED); > } else { > addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; > ptr = qemu_get_ram_ptr(addr1); > @@ -2892,7 +2893,7 @@ static inline void stl_phys_internal(AddressSpace *as, > val = bswap32(val); > } > #endif > - io_mem_write(mr, addr1, val, 4); > + io_mem_write(mr, addr1, val, 4, MEMTXATTRS_UNSPECIFIED); > } else { > /* RAM case */ > addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; > @@ -2955,7 +2956,7 @@ static inline void stw_phys_internal(AddressSpace *as, > val = bswap16(val); > } > #endif > - io_mem_write(mr, addr1, val, 2); > + io_mem_write(mr, addr1, val, 2, MEMTXATTRS_UNSPECIFIED); > } else { > /* RAM case */ > addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; > diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c > index 08d8aa6..78ff9c0 100644 > --- a/hw/s390x/s390-pci-inst.c > +++ b/hw/s390x/s390-pci-inst.c > @@ -331,7 +331,7 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) > return 0; > } > MemoryRegion *mr = pbdev->pdev->io_regions[pcias].memory; > - io_mem_read(mr, offset, &data, len); > + io_mem_read(mr, offset, &data, len, MEMTXATTRS_UNSPECIFIED); > } else if (pcias == 15) { > if ((4 - (offset & 0x3)) < len) { > program_interrupt(env, PGM_OPERAND, 4); > @@ -456,7 +456,7 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) > mr = pbdev->pdev->io_regions[pcias].memory; > } > > - io_mem_write(mr, offset, data, len); > + io_mem_write(mr, offset, data, len, MEMTXATTRS_UNSPECIFIED); > } else if (pcias == 15) { > if ((4 - (offset & 0x3)) < len) { > program_interrupt(env, PGM_OPERAND, 4); > @@ -606,7 +606,8 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr) > } > > for (i = 0; i < len / 8; i++) { > - io_mem_write(mr, env->regs[r3] + i * 8, ldq_p(buffer + i * 8), 8); > + io_mem_write(mr, env->regs[r3] + i * 8, ldq_p(buffer + i * 8), 8, > + MEMTXATTRS_UNSPECIFIED); > } > > setcc(cpu, ZPCI_PCI_LS_OK); > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 6b80539..3e1df0c 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -1533,7 +1533,7 @@ static uint64_t vfio_rtl8168_window_quirk_read(void *opaque, > > io_mem_read(&vdev->pdev.msix_table_mmio, > (hwaddr)(quirk->data.address_match & 0xfff), > - &val, size); > + &val, size, MEMTXATTRS_UNSPECIFIED); > return val; > } > } > @@ -1563,7 +1563,7 @@ static void vfio_rtl8168_window_quirk_write(void *opaque, hwaddr addr, > > io_mem_write(&vdev->pdev.msix_table_mmio, > (hwaddr)(quirk->data.address_match & 0xfff), > - data, size); > + data, size, MEMTXATTRS_UNSPECIFIED); > } > > quirk->data.flags = 1; > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 8eb0db3..ff1bc3e 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -341,10 +341,6 @@ void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align)); > > struct MemoryRegion *iotlb_to_region(CPUState *cpu, > hwaddr index); > -bool io_mem_read(struct MemoryRegion *mr, hwaddr addr, > - uint64_t *pvalue, unsigned size); > -bool io_mem_write(struct MemoryRegion *mr, hwaddr addr, > - uint64_t value, unsigned size); > > void tlb_fill(CPUState *cpu, target_ulong addr, int is_write, int mmu_idx, > uintptr_t retaddr); > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 703d9e5..de2849d 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -1053,6 +1053,18 @@ void memory_global_dirty_log_stop(void); > void mtree_info(fprintf_function mon_printf, void *f); > > /** > + * io_mem_read: perform an IO read directly to the specified MemoryRegion > + */ > +MemTxResult io_mem_read(struct MemoryRegion *mr, hwaddr addr, > + uint64_t *pvalue, unsigned size, MemTxAttrs attrs); > + > +/** > + * io_mem_write: perform an IO write directly to the specified MemoryRegion > + */ > +MemTxResult io_mem_write(struct MemoryRegion *mr, hwaddr addr, > + uint64_t value, unsigned size, MemTxAttrs attrs); > + > +/** > * address_space_init: initializes an address space > * > * @as: an uninitialized #AddressSpace > diff --git a/memory.c b/memory.c > index 9bb5674..14cda7f 100644 > --- a/memory.c > +++ b/memory.c > @@ -2063,17 +2063,16 @@ void address_space_destroy(AddressSpace *as) > call_rcu(as, do_address_space_destroy, rcu); > } > > -bool io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, unsigned size) > +MemTxResult io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, > + unsigned size, MemTxAttrs attrs) > { > - return memory_region_dispatch_read(mr, addr, pval, size, > - MEMTXATTRS_UNSPECIFIED); > + return memory_region_dispatch_read(mr, addr, pval, size, attrs); > } > > -bool io_mem_write(MemoryRegion *mr, hwaddr addr, > - uint64_t val, unsigned size) > +MemTxResult io_mem_write(MemoryRegion *mr, hwaddr addr, > + uint64_t val, unsigned size, MemTxAttrs attrs) > { > - return memory_region_dispatch_write(mr, addr, val, size, > - MEMTXATTRS_UNSPECIFIED); > + return memory_region_dispatch_write(mr, addr, val, size, attrs); > } > > typedef struct MemoryRegionList MemoryRegionList; > diff --git a/softmmu_template.h b/softmmu_template.h > index 0e3dd35..4b9bae7 100644 > --- a/softmmu_template.h > +++ b/softmmu_template.h > @@ -158,7 +158,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env, > } > > cpu->mem_io_vaddr = addr; > - io_mem_read(mr, physaddr, &val, 1 << SHIFT); > + io_mem_read(mr, physaddr, &val, 1 << SHIFT, MEMTXATTRS_UNSPECIFIED); > return val; > } > #endif > @@ -378,7 +378,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env, > > cpu->mem_io_vaddr = addr; > cpu->mem_io_pc = retaddr; > - io_mem_write(mr, physaddr, val, 1 << SHIFT); > + io_mem_write(mr, physaddr, val, 1 << SHIFT, MEMTXATTRS_UNSPECIFIED); > } > > void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, > -- > 1.9.1 >
diff --git a/exec.c b/exec.c index 874ecfc..52e7a2a 100644 --- a/exec.c +++ b/exec.c @@ -2312,7 +2312,8 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, uint64_t val; hwaddr addr1; MemoryRegion *mr; - bool error = false; + MemTxResult result = MEMTX_OK; + MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; while (len > 0) { l = len; @@ -2327,22 +2328,22 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, case 8: /* 64 bit write access */ val = ldq_p(buf); - error |= io_mem_write(mr, addr1, val, 8); + result |= io_mem_write(mr, addr1, val, 8, attrs); break; case 4: /* 32 bit write access */ val = ldl_p(buf); - error |= io_mem_write(mr, addr1, val, 4); + result |= io_mem_write(mr, addr1, val, 4, attrs); break; case 2: /* 16 bit write access */ val = lduw_p(buf); - error |= io_mem_write(mr, addr1, val, 2); + result |= io_mem_write(mr, addr1, val, 2, attrs); break; case 1: /* 8 bit write access */ val = ldub_p(buf); - error |= io_mem_write(mr, addr1, val, 1); + result |= io_mem_write(mr, addr1, val, 1, attrs); break; default: abort(); @@ -2361,22 +2362,22 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, switch (l) { case 8: /* 64 bit read access */ - error |= io_mem_read(mr, addr1, &val, 8); + result |= io_mem_read(mr, addr1, &val, 8, attrs); stq_p(buf, val); break; case 4: /* 32 bit read access */ - error |= io_mem_read(mr, addr1, &val, 4); + result |= io_mem_read(mr, addr1, &val, 4, attrs); stl_p(buf, val); break; case 2: /* 16 bit read access */ - error |= io_mem_read(mr, addr1, &val, 2); + result |= io_mem_read(mr, addr1, &val, 2, attrs); stw_p(buf, val); break; case 1: /* 8 bit read access */ - error |= io_mem_read(mr, addr1, &val, 1); + result |= io_mem_read(mr, addr1, &val, 1, attrs); stb_p(buf, val); break; default: @@ -2393,7 +2394,7 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, addr += l; } - return error; + return result; } bool address_space_write(AddressSpace *as, hwaddr addr, @@ -2669,7 +2670,7 @@ static inline uint32_t ldl_phys_internal(AddressSpace *as, hwaddr addr, mr = address_space_translate(as, addr, &addr1, &l, false); if (l < 4 || !memory_access_is_direct(mr, false)) { /* I/O case */ - io_mem_read(mr, addr1, &val, 4); + io_mem_read(mr, addr1, &val, 4, MEMTXATTRS_UNSPECIFIED); #if defined(TARGET_WORDS_BIGENDIAN) if (endian == DEVICE_LITTLE_ENDIAN) { val = bswap32(val); @@ -2728,7 +2729,7 @@ static inline uint64_t ldq_phys_internal(AddressSpace *as, hwaddr addr, false); if (l < 8 || !memory_access_is_direct(mr, false)) { /* I/O case */ - io_mem_read(mr, addr1, &val, 8); + io_mem_read(mr, addr1, &val, 8, MEMTXATTRS_UNSPECIFIED); #if defined(TARGET_WORDS_BIGENDIAN) if (endian == DEVICE_LITTLE_ENDIAN) { val = bswap64(val); @@ -2795,7 +2796,7 @@ static inline uint32_t lduw_phys_internal(AddressSpace *as, hwaddr addr, false); if (l < 2 || !memory_access_is_direct(mr, false)) { /* I/O case */ - io_mem_read(mr, addr1, &val, 2); + io_mem_read(mr, addr1, &val, 2, MEMTXATTRS_UNSPECIFIED); #if defined(TARGET_WORDS_BIGENDIAN) if (endian == DEVICE_LITTLE_ENDIAN) { val = bswap16(val); @@ -2853,7 +2854,7 @@ void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val) mr = address_space_translate(as, addr, &addr1, &l, true); if (l < 4 || !memory_access_is_direct(mr, true)) { - io_mem_write(mr, addr1, val, 4); + io_mem_write(mr, addr1, val, 4, MEMTXATTRS_UNSPECIFIED); } else { addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; ptr = qemu_get_ram_ptr(addr1); @@ -2892,7 +2893,7 @@ static inline void stl_phys_internal(AddressSpace *as, val = bswap32(val); } #endif - io_mem_write(mr, addr1, val, 4); + io_mem_write(mr, addr1, val, 4, MEMTXATTRS_UNSPECIFIED); } else { /* RAM case */ addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; @@ -2955,7 +2956,7 @@ static inline void stw_phys_internal(AddressSpace *as, val = bswap16(val); } #endif - io_mem_write(mr, addr1, val, 2); + io_mem_write(mr, addr1, val, 2, MEMTXATTRS_UNSPECIFIED); } else { /* RAM case */ addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK; diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index 08d8aa6..78ff9c0 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -331,7 +331,7 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) return 0; } MemoryRegion *mr = pbdev->pdev->io_regions[pcias].memory; - io_mem_read(mr, offset, &data, len); + io_mem_read(mr, offset, &data, len, MEMTXATTRS_UNSPECIFIED); } else if (pcias == 15) { if ((4 - (offset & 0x3)) < len) { program_interrupt(env, PGM_OPERAND, 4); @@ -456,7 +456,7 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) mr = pbdev->pdev->io_regions[pcias].memory; } - io_mem_write(mr, offset, data, len); + io_mem_write(mr, offset, data, len, MEMTXATTRS_UNSPECIFIED); } else if (pcias == 15) { if ((4 - (offset & 0x3)) < len) { program_interrupt(env, PGM_OPERAND, 4); @@ -606,7 +606,8 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr) } for (i = 0; i < len / 8; i++) { - io_mem_write(mr, env->regs[r3] + i * 8, ldq_p(buffer + i * 8), 8); + io_mem_write(mr, env->regs[r3] + i * 8, ldq_p(buffer + i * 8), 8, + MEMTXATTRS_UNSPECIFIED); } setcc(cpu, ZPCI_PCI_LS_OK); diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 6b80539..3e1df0c 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -1533,7 +1533,7 @@ static uint64_t vfio_rtl8168_window_quirk_read(void *opaque, io_mem_read(&vdev->pdev.msix_table_mmio, (hwaddr)(quirk->data.address_match & 0xfff), - &val, size); + &val, size, MEMTXATTRS_UNSPECIFIED); return val; } } @@ -1563,7 +1563,7 @@ static void vfio_rtl8168_window_quirk_write(void *opaque, hwaddr addr, io_mem_write(&vdev->pdev.msix_table_mmio, (hwaddr)(quirk->data.address_match & 0xfff), - data, size); + data, size, MEMTXATTRS_UNSPECIFIED); } quirk->data.flags = 1; diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 8eb0db3..ff1bc3e 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -341,10 +341,6 @@ void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align)); struct MemoryRegion *iotlb_to_region(CPUState *cpu, hwaddr index); -bool io_mem_read(struct MemoryRegion *mr, hwaddr addr, - uint64_t *pvalue, unsigned size); -bool io_mem_write(struct MemoryRegion *mr, hwaddr addr, - uint64_t value, unsigned size); void tlb_fill(CPUState *cpu, target_ulong addr, int is_write, int mmu_idx, uintptr_t retaddr); diff --git a/include/exec/memory.h b/include/exec/memory.h index 703d9e5..de2849d 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1053,6 +1053,18 @@ void memory_global_dirty_log_stop(void); void mtree_info(fprintf_function mon_printf, void *f); /** + * io_mem_read: perform an IO read directly to the specified MemoryRegion + */ +MemTxResult io_mem_read(struct MemoryRegion *mr, hwaddr addr, + uint64_t *pvalue, unsigned size, MemTxAttrs attrs); + +/** + * io_mem_write: perform an IO write directly to the specified MemoryRegion + */ +MemTxResult io_mem_write(struct MemoryRegion *mr, hwaddr addr, + uint64_t value, unsigned size, MemTxAttrs attrs); + +/** * address_space_init: initializes an address space * * @as: an uninitialized #AddressSpace diff --git a/memory.c b/memory.c index 9bb5674..14cda7f 100644 --- a/memory.c +++ b/memory.c @@ -2063,17 +2063,16 @@ void address_space_destroy(AddressSpace *as) call_rcu(as, do_address_space_destroy, rcu); } -bool io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, unsigned size) +MemTxResult io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, + unsigned size, MemTxAttrs attrs) { - return memory_region_dispatch_read(mr, addr, pval, size, - MEMTXATTRS_UNSPECIFIED); + return memory_region_dispatch_read(mr, addr, pval, size, attrs); } -bool io_mem_write(MemoryRegion *mr, hwaddr addr, - uint64_t val, unsigned size) +MemTxResult io_mem_write(MemoryRegion *mr, hwaddr addr, + uint64_t val, unsigned size, MemTxAttrs attrs) { - return memory_region_dispatch_write(mr, addr, val, size, - MEMTXATTRS_UNSPECIFIED); + return memory_region_dispatch_write(mr, addr, val, size, attrs); } typedef struct MemoryRegionList MemoryRegionList; diff --git a/softmmu_template.h b/softmmu_template.h index 0e3dd35..4b9bae7 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -158,7 +158,7 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env, } cpu->mem_io_vaddr = addr; - io_mem_read(mr, physaddr, &val, 1 << SHIFT); + io_mem_read(mr, physaddr, &val, 1 << SHIFT, MEMTXATTRS_UNSPECIFIED); return val; } #endif @@ -378,7 +378,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env, cpu->mem_io_vaddr = addr; cpu->mem_io_pc = retaddr; - io_mem_write(mr, physaddr, val, 1 << SHIFT); + io_mem_write(mr, physaddr, val, 1 << SHIFT, MEMTXATTRS_UNSPECIFIED); } void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
Add a MemTxAttrs argument to the io_mem_read() and io_mem_write() functions, and make them return MemTxResult rather than bool, thus pushing these new arguments out one level of the callstack (all the callers callers currently pass MEMTXATTRS_UNSPECIFIED and convert the return value back to bool or ignore it). Note that this involves moving the io_mem_read and io_mem_write prototypes from exec-all.h to memory.h. This allows them to see the MemTxResult type, and is a better location anyway, since they are implemented in memory.c and are operations on MemoryRegions. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- exec.c | 33 +++++++++++++++++---------------- hw/s390x/s390-pci-inst.c | 7 ++++--- hw/vfio/pci.c | 4 ++-- include/exec/exec-all.h | 4 ---- include/exec/memory.h | 12 ++++++++++++ memory.c | 13 ++++++------- softmmu_template.h | 4 ++-- 7 files changed, 43 insertions(+), 34 deletions(-)