| Message ID | 20200218112457.22712-1-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Avoid address_space_rw() with a constant is_write argument | expand |
On Tue, Feb 18, 2020 at 11:24:57AM +0000, Peter Maydell wrote: > The address_space_rw() function allows either reads or writes > depending on the is_write argument passed to it; this is useful > when the direction of the access is determined programmatically > (as for instance when handling the KVM_EXIT_MMIO exit reason). > Under the hood it just calls either address_space_write() or > address_space_read_full(). > > We also use it a lot with a constant is_write argument, though, > which has two issues: > * when reading "address_space_rw(..., 1)" this is less > immediately clear to the reader as being a write than > "address_space_write(...)" > * calling address_space_rw() bypasses the optimization > in address_space_read() that fast-paths reads of a > fixed length > > This commit was produced with the included Coccinelle script > scripts/coccinelle/as-rw-const.patch. > > Two lines in hw/net/dp8393x.c that Coccinelle produced that > were over 80 characters were re-wrapped by hand. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I could break this down into separate patches by submaintainer, > but the patch is not that large and I would argue that it's > better for the project if we can try to avoid introducing too > much friction into the process of doing 'safe' tree-wide > minor refactorings. For xlnx-zdma: Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > v1->v2: put the coccinelle script in scripts/coccinelle rather > than just in the commit message. > --- > accel/kvm/kvm-all.c | 6 +-- > dma-helpers.c | 4 +- > exec.c | 4 +- > hw/dma/xlnx-zdma.c | 11 ++--- > hw/net/dp8393x.c | 68 ++++++++++++++-------------- > hw/net/i82596.c | 25 +++++----- > hw/net/lasi_i82596.c | 5 +- > hw/ppc/pnv_lpc.c | 8 ++-- > hw/s390x/css.c | 12 ++--- > qtest.c | 52 ++++++++++----------- > target/i386/hvf/x86_mmu.c | 12 ++--- > scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ > 12 files changed, 133 insertions(+), 104 deletions(-) > create mode 100644 scripts/coccinelle/as_rw_const.cocci > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index c111312dfdd..0cfe6fd8ded 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2178,9 +2178,9 @@ void kvm_flush_coalesced_mmio_buffer(void) > ent = &ring->coalesced_mmio[ring->first]; > > if (ent->pio == 1) { > - address_space_rw(&address_space_io, ent->phys_addr, > - MEMTXATTRS_UNSPECIFIED, ent->data, > - ent->len, true); > + address_space_write(&address_space_io, ent->phys_addr, > + MEMTXATTRS_UNSPECIFIED, ent->data, > + ent->len); > } else { > cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len); > } > diff --git a/dma-helpers.c b/dma-helpers.c > index d3871dc61ea..e8a26e81e16 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -28,8 +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, MEMTXATTRS_UNSPECIFIED, > - fillbuf, l, true); > + error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, > + fillbuf, l); > len -= l; > addr += l; > } > diff --git a/exec.c b/exec.c > index 8e9cc3b47cf..baefe582393 100644 > --- a/exec.c > +++ b/exec.c > @@ -3810,8 +3810,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, > attrs, buf, l); > } else { > - address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, > - attrs, buf, l, 0); > + address_space_read(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, > + l); > } > len -= l; > buf += l; > diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c > index 8fb83f5b078..31936061e21 100644 > --- a/hw/dma/xlnx-zdma.c > +++ b/hw/dma/xlnx-zdma.c > @@ -311,8 +311,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) > return false; > } > > - address_space_rw(s->dma_as, addr, s->attr, > - buf, sizeof(XlnxZDMADescr), false); > + address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr)); > return true; > } > > @@ -364,7 +363,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA *s, bool type, > } else { > addr = zdma_get_regaddr64(s, basereg); > addr += sizeof(s->dsc_dst); > - address_space_rw(s->dma_as, addr, s->attr, (void *) &next, 8, false); > + address_space_read(s->dma_as, addr, s->attr, (void *)&next, 8); > zdma_put_regaddr64(s, basereg, next); > } > return next; > @@ -416,8 +415,7 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) > } > } > > - address_space_rw(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen, > - true); > + address_space_write(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen); > if (burst_type == AXI_BURST_INCR) { > s->dsc_dst.addr += dlen; > } > @@ -493,8 +491,7 @@ static void zdma_process_descr(XlnxZDMA *s) > len = s->cfg.bus_width / 8; > } > } else { > - address_space_rw(s->dma_as, src_addr, s->attr, s->buf, len, > - false); > + address_space_read(s->dma_as, src_addr, s->attr, s->buf, len); > if (burst_type == AXI_BURST_INCR) { > src_addr += len; > } > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index a134d431ae3..f5f1c669e80 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -275,8 +275,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > /* Fill current entry */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff; > s->cam[index][1] = dp8393x_get(s, width, 1) >> 8; > s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff; > @@ -293,8 +293,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > } > > /* Read CAM enable */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); > > @@ -311,8 +311,8 @@ static void dp8393x_do_read_rra(dp8393xState *s) > /* Read memory */ > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > size = sizeof(uint16_t) * 4 * width; > - address_space_rw(&s->as, dp8393x_rrp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Update SONIC registers */ > s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > @@ -426,8 +426,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > size = sizeof(uint16_t) * 6 * width; > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); > - address_space_rw(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > tx_len = 0; > > /* Update registers */ > @@ -451,17 +451,19 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > if (tx_len + len > sizeof(s->tx_buffer)) { > len = sizeof(s->tx_buffer) - tx_len; > } > - address_space_rw(&s->as, dp8393x_tsa(s), > - MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); > + address_space_read(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, > + &s->tx_buffer[tx_len], len); > tx_len += len; > > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > size = sizeof(uint16_t) * 3 * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * i) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > @@ -494,18 +496,18 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > dp8393x_put(s, width, 0, > s->regs[SONIC_TCR] & 0x0fff); /* status */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > /* Read footer of packet */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + > - sizeof(uint16_t) * > - (4 + 3 * s->regs[SONIC_TFC]) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) > + * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1; > if (dp8393x_get(s, width, 0) & 0x1) { > /* EOL detected */ > @@ -767,8 +769,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Are we still in resource exhaustion? */ > size = sizeof(uint16_t) * 1 * width; > address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > if (dp8393x_get(s, width, 0) & 0x1) { > /* Still EOL ; stop reception */ > return -1; > @@ -787,11 +789,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Put packet into RBA */ > DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); > address = dp8393x_crba(s); > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)buf, rx_len); > address += rx_len; > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&checksum, 4); > rx_len += 4; > s->regs[SONIC_CRBA1] = address >> 16; > s->regs[SONIC_CRBA0] = address & 0xffff; > @@ -819,13 +821,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > size = sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, dp8393x_crda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Move to next descriptor */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > if (s->regs[SONIC_LLFA] & 0x1) { > /* EOL detected */ > @@ -838,8 +840,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > offset += sizeof(uint16_t); > } > s->data[0] = 0; > - address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, sizeof(uint16_t), 1); > + address_space_write(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, sizeof(uint16_t)); > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; > s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); > diff --git a/hw/net/i82596.c b/hw/net/i82596.c > index 3a0e1ec4c05..6a80c24af23 100644 > --- a/hw/net/i82596.c > +++ b/hw/net/i82596.c > @@ -148,8 +148,8 @@ static void i82596_transmit(I82596State *s, uint32_t addr) > > if (s->nic && len) { > assert(len <= sizeof(s->tx_buffer)); > - address_space_rw(&address_space_memory, tba, > - MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len, 0); > + address_space_read(&address_space_memory, tba, > + MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len); > DBG(PRINT_PKTHDR("Send", &s->tx_buffer)); > DBG(printf("Sending %d bytes\n", len)); > qemu_send_packet(qemu_get_queue(s->nic), s->tx_buffer, len); > @@ -172,8 +172,8 @@ static void set_individual_address(I82596State *s, uint32_t addr) > > nc = qemu_get_queue(s->nic); > m = s->conf.macaddr.a; > - address_space_rw(&address_space_memory, addr + 8, > - MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + 8, > + MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN); > qemu_format_nic_info_str(nc, m); > trace_i82596_new_mac(nc->info_str); > } > @@ -190,9 +190,8 @@ static void set_multicast_list(I82596State *s, uint32_t addr) > } > for (i = 0; i < mc_count; i++) { > uint8_t multicast_addr[ETH_ALEN]; > - address_space_rw(&address_space_memory, > - addr + i * ETH_ALEN, MEMTXATTRS_UNSPECIFIED, > - multicast_addr, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + i * ETH_ALEN, > + MEMTXATTRS_UNSPECIFIED, multicast_addr, ETH_ALEN); > DBG(printf("Add multicast entry " MAC_FMT "\n", > MAC_ARG(multicast_addr))); > unsigned mcast_idx = (net_crc32(multicast_addr, ETH_ALEN) & > @@ -260,8 +259,8 @@ static void command_loop(I82596State *s) > byte_cnt = MAX(byte_cnt, 4); > byte_cnt = MIN(byte_cnt, sizeof(s->config)); > /* copy byte_cnt max. */ > - address_space_rw(&address_space_memory, s->cmd_p + 8, > - MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt, 0); > + address_space_read(&address_space_memory, s->cmd_p + 8, > + MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt); > /* config byte according to page 35ff */ > s->config[2] &= 0x82; /* mask valid bits */ > s->config[2] |= 0x40; > @@ -640,14 +639,14 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) > } > rba = get_uint32(rbd + 8); > /* printf("rba is 0x%x\n", rba); */ > - address_space_rw(&address_space_memory, rba, > - MEMTXATTRS_UNSPECIFIED, (void *)buf, num, 1); > + address_space_write(&address_space_memory, rba, > + MEMTXATTRS_UNSPECIFIED, (void *)buf, num); > rba += num; > buf += num; > len -= num; > if (len == 0) { /* copy crc */ > - address_space_rw(&address_space_memory, rba - 4, > - MEMTXATTRS_UNSPECIFIED, crc_ptr, 4, 1); > + address_space_write(&address_space_memory, rba - 4, > + MEMTXATTRS_UNSPECIFIED, crc_ptr, 4); > } > > num |= 0x4000; /* set F BIT */ > diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c > index 427b3fbf701..52637a562d8 100644 > --- a/hw/net/lasi_i82596.c > +++ b/hw/net/lasi_i82596.c > @@ -55,8 +55,9 @@ static void lasi_82596_mem_write(void *opaque, hwaddr addr, > * Provided for SeaBIOS only. Write MAC of Network card to addr @val. > * Needed for the PDC_LAN_STATION_ID_READ PDC call. > */ > - address_space_rw(&address_space_memory, val, > - MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, ETH_ALEN, 1); > + address_space_write(&address_space_memory, val, > + MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, > + ETH_ALEN); > break; > } > } > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c > index 5989d723c50..f150deca340 100644 > --- a/hw/ppc/pnv_lpc.c > +++ b/hw/ppc/pnv_lpc.c > @@ -238,16 +238,16 @@ static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits and FW read caching here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, false); > + return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, true); > + return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > #define ECCB_CTL_READ PPC_BIT(15) > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 844caab4082..0e0fccd050e 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -874,18 +874,18 @@ static inline int ida_read_next_idaw(CcwDataStream *cds) > if (idaw_addr & 0x07 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt2, > - sizeof(idaw.fmt2), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt2, > + sizeof(idaw.fmt2)); > cds->cda = be64_to_cpu(idaw.fmt2); > } else { > idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw; > if (idaw_addr & 0x03 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt1, > - sizeof(idaw.fmt1), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt1, > + sizeof(idaw.fmt1)); > cds->cda = be64_to_cpu(idaw.fmt1); > if (cds->cda & 0x80000000) { > return -EINVAL; /* channel program check */ > diff --git a/qtest.c b/qtest.c > index 12432f99cf4..328d674bcc8 100644 > --- a/qtest.c > +++ b/qtest.c > @@ -429,23 +429,23 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][5] == 'b') { > uint8_t data = value; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > } else if (words[0][5] == 'w') { > uint16_t data = value; > tswap16s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > } else if (words[0][5] == 'l') { > uint32_t data = value; > tswap32s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > } else if (words[0][5] == 'q') { > uint64_t data = value; > tswap64s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 8, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 8); > } > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > @@ -463,22 +463,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][4] == 'b') { > uint8_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > value = data; > } else if (words[0][4] == 'w') { > uint16_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > value = tswap16(data); > } else if (words[0][4] == 'l') { > uint32_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > value = tswap32(data); > } else if (words[0][4] == 'q') { > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &value, 8, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&value, 8); > tswap64s(&value); > } > qtest_send_prefix(chr); > @@ -498,8 +498,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(len); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > enc = g_malloc(2 * len + 1); > for (i = 0; i < len; i++) { > @@ -524,8 +524,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(ret == 0); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > b64_data = g_base64_encode(data, len); > qtest_send_prefix(chr); > qtest_sendf(chr, "OK %s\n", b64_data); > @@ -559,8 +559,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > data[i] = 0; > } > } > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > g_free(data); > > qtest_send_prefix(chr); > @@ -582,8 +582,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > if (len) { > data = g_malloc(len); > memset(data, pattern, len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + data, len); > g_free(data); > } > > @@ -616,8 +616,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > out_len = MIN(out_len, len); > } > > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > diff --git a/target/i386/hvf/x86_mmu.c b/target/i386/hvf/x86_mmu.c > index d5a0efe7188..ff016fc0145 100644 > --- a/target/i386/hvf/x86_mmu.c > +++ b/target/i386/hvf/x86_mmu.c > @@ -88,8 +88,8 @@ static bool get_pt_entry(struct CPUState *cpu, struct gpt_translation *pt, > } > > index = gpt_entry(pt->gva, level, pae); > - address_space_rw(&address_space_memory, gpa + index * pte_size(pae), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae), 0); > + address_space_read(&address_space_memory, gpa + index * pte_size(pae), > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae)); > > pt->pte[level - 1] = pte; > > @@ -238,8 +238,8 @@ void vmx_write_mem(struct CPUState *cpu, target_ulong gva, void *data, int bytes > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } else { > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 1); > + address_space_write(&address_space_memory, gpa, > + MEMTXATTRS_UNSPECIFIED, data, copy); > } > > bytes -= copy; > @@ -259,8 +259,8 @@ void vmx_read_mem(struct CPUState *cpu, void *data, target_ulong gva, int bytes) > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 0); > + address_space_read(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > + data, copy); > > bytes -= copy; > gva += copy; > diff --git a/scripts/coccinelle/as_rw_const.cocci b/scripts/coccinelle/as_rw_const.cocci > new file mode 100644 > index 00000000000..30da707701b > --- /dev/null > +++ b/scripts/coccinelle/as_rw_const.cocci > @@ -0,0 +1,30 @@ > +// Avoid uses of address_space_rw() with a constant is_write argument. > +// Usage: > +// spatch --sp-file as-rw-const.spatch --dir . --in-place > + > +@@ > +expression E1, E2, E3, E4, E5; > +symbol false; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, false) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 0) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +symbol true; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, true) > ++ address_space_write(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 1) > ++ address_space_write(E1, E2, E3, E4, E5) > -- > 2.20.1 >
On 2/18/20 12:24 PM, Peter Maydell wrote: > The address_space_rw() function allows either reads or writes > depending on the is_write argument passed to it; this is useful > when the direction of the access is determined programmatically > (as for instance when handling the KVM_EXIT_MMIO exit reason). > Under the hood it just calls either address_space_write() or > address_space_read_full(). > > We also use it a lot with a constant is_write argument, though, > which has two issues: > * when reading "address_space_rw(..., 1)" this is less > immediately clear to the reader as being a write than > "address_space_write(...)" > * calling address_space_rw() bypasses the optimization > in address_space_read() that fast-paths reads of a > fixed length > > This commit was produced with the included Coccinelle script > scripts/coccinelle/as-rw-const.patch. > > Two lines in hw/net/dp8393x.c that Coccinelle produced that > were over 80 characters were re-wrapped by hand. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I could break this down into separate patches by submaintainer, > but the patch is not that large and I would argue that it's > better for the project if we can try to avoid introducing too > much friction into the process of doing 'safe' tree-wide > minor refactorings. > > v1->v2: put the coccinelle script in scripts/coccinelle rather > than just in the commit message. > --- > accel/kvm/kvm-all.c | 6 +-- > dma-helpers.c | 4 +- > exec.c | 4 +- > hw/dma/xlnx-zdma.c | 11 ++--- > hw/net/dp8393x.c | 68 ++++++++++++++-------------- > hw/net/i82596.c | 25 +++++----- > hw/net/lasi_i82596.c | 5 +- > hw/ppc/pnv_lpc.c | 8 ++-- > hw/s390x/css.c | 12 ++--- > qtest.c | 52 ++++++++++----------- > target/i386/hvf/x86_mmu.c | 12 ++--- > scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ > 12 files changed, 133 insertions(+), 104 deletions(-) > create mode 100644 scripts/coccinelle/as_rw_const.cocci > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index c111312dfdd..0cfe6fd8ded 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2178,9 +2178,9 @@ void kvm_flush_coalesced_mmio_buffer(void) > ent = &ring->coalesced_mmio[ring->first]; > > if (ent->pio == 1) { > - address_space_rw(&address_space_io, ent->phys_addr, > - MEMTXATTRS_UNSPECIFIED, ent->data, > - ent->len, true); > + address_space_write(&address_space_io, ent->phys_addr, > + MEMTXATTRS_UNSPECIFIED, ent->data, > + ent->len); > } else { > cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len); > } > diff --git a/dma-helpers.c b/dma-helpers.c > index d3871dc61ea..e8a26e81e16 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -28,8 +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, MEMTXATTRS_UNSPECIFIED, > - fillbuf, l, true); > + error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, > + fillbuf, l); > len -= l; > addr += l; > } > diff --git a/exec.c b/exec.c > index 8e9cc3b47cf..baefe582393 100644 > --- a/exec.c > +++ b/exec.c > @@ -3810,8 +3810,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, > attrs, buf, l); > } else { > - address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, > - attrs, buf, l, 0); > + address_space_read(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, > + l); > } > len -= l; > buf += l; > diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c > index 8fb83f5b078..31936061e21 100644 > --- a/hw/dma/xlnx-zdma.c > +++ b/hw/dma/xlnx-zdma.c > @@ -311,8 +311,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) > return false; > } > > - address_space_rw(s->dma_as, addr, s->attr, > - buf, sizeof(XlnxZDMADescr), false); > + address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr)); > return true; > } > > @@ -364,7 +363,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA *s, bool type, > } else { > addr = zdma_get_regaddr64(s, basereg); > addr += sizeof(s->dsc_dst); > - address_space_rw(s->dma_as, addr, s->attr, (void *) &next, 8, false); > + address_space_read(s->dma_as, addr, s->attr, (void *)&next, 8); > zdma_put_regaddr64(s, basereg, next); > } > return next; > @@ -416,8 +415,7 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) > } > } > > - address_space_rw(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen, > - true); > + address_space_write(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen); > if (burst_type == AXI_BURST_INCR) { > s->dsc_dst.addr += dlen; > } > @@ -493,8 +491,7 @@ static void zdma_process_descr(XlnxZDMA *s) > len = s->cfg.bus_width / 8; > } > } else { > - address_space_rw(s->dma_as, src_addr, s->attr, s->buf, len, > - false); > + address_space_read(s->dma_as, src_addr, s->attr, s->buf, len); > if (burst_type == AXI_BURST_INCR) { > src_addr += len; > } > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index a134d431ae3..f5f1c669e80 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -275,8 +275,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > /* Fill current entry */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff; > s->cam[index][1] = dp8393x_get(s, width, 1) >> 8; > s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff; > @@ -293,8 +293,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > } > > /* Read CAM enable */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); > > @@ -311,8 +311,8 @@ static void dp8393x_do_read_rra(dp8393xState *s) > /* Read memory */ > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > size = sizeof(uint16_t) * 4 * width; > - address_space_rw(&s->as, dp8393x_rrp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Update SONIC registers */ > s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > @@ -426,8 +426,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > size = sizeof(uint16_t) * 6 * width; > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); > - address_space_rw(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > tx_len = 0; > > /* Update registers */ > @@ -451,17 +451,19 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > if (tx_len + len > sizeof(s->tx_buffer)) { > len = sizeof(s->tx_buffer) - tx_len; > } > - address_space_rw(&s->as, dp8393x_tsa(s), > - MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); > + address_space_read(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, > + &s->tx_buffer[tx_len], len); > tx_len += len; > > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > size = sizeof(uint16_t) * 3 * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * i) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > @@ -494,18 +496,18 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > dp8393x_put(s, width, 0, > s->regs[SONIC_TCR] & 0x0fff); /* status */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > /* Read footer of packet */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + > - sizeof(uint16_t) * > - (4 + 3 * s->regs[SONIC_TFC]) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) > + * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1; > if (dp8393x_get(s, width, 0) & 0x1) { > /* EOL detected */ > @@ -767,8 +769,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Are we still in resource exhaustion? */ > size = sizeof(uint16_t) * 1 * width; > address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > if (dp8393x_get(s, width, 0) & 0x1) { > /* Still EOL ; stop reception */ > return -1; > @@ -787,11 +789,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Put packet into RBA */ > DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); > address = dp8393x_crba(s); > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)buf, rx_len); > address += rx_len; > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&checksum, 4); > rx_len += 4; > s->regs[SONIC_CRBA1] = address >> 16; > s->regs[SONIC_CRBA0] = address & 0xffff; > @@ -819,13 +821,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > size = sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, dp8393x_crda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Move to next descriptor */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > if (s->regs[SONIC_LLFA] & 0x1) { > /* EOL detected */ > @@ -838,8 +840,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > offset += sizeof(uint16_t); > } > s->data[0] = 0; > - address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, sizeof(uint16_t), 1); > + address_space_write(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, sizeof(uint16_t)); > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; > s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); > diff --git a/hw/net/i82596.c b/hw/net/i82596.c > index 3a0e1ec4c05..6a80c24af23 100644 > --- a/hw/net/i82596.c > +++ b/hw/net/i82596.c > @@ -148,8 +148,8 @@ static void i82596_transmit(I82596State *s, uint32_t addr) > > if (s->nic && len) { > assert(len <= sizeof(s->tx_buffer)); > - address_space_rw(&address_space_memory, tba, > - MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len, 0); > + address_space_read(&address_space_memory, tba, > + MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len); > DBG(PRINT_PKTHDR("Send", &s->tx_buffer)); > DBG(printf("Sending %d bytes\n", len)); > qemu_send_packet(qemu_get_queue(s->nic), s->tx_buffer, len); > @@ -172,8 +172,8 @@ static void set_individual_address(I82596State *s, uint32_t addr) > > nc = qemu_get_queue(s->nic); > m = s->conf.macaddr.a; > - address_space_rw(&address_space_memory, addr + 8, > - MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + 8, > + MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN); > qemu_format_nic_info_str(nc, m); > trace_i82596_new_mac(nc->info_str); > } > @@ -190,9 +190,8 @@ static void set_multicast_list(I82596State *s, uint32_t addr) > } > for (i = 0; i < mc_count; i++) { > uint8_t multicast_addr[ETH_ALEN]; > - address_space_rw(&address_space_memory, > - addr + i * ETH_ALEN, MEMTXATTRS_UNSPECIFIED, > - multicast_addr, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + i * ETH_ALEN, > + MEMTXATTRS_UNSPECIFIED, multicast_addr, ETH_ALEN); > DBG(printf("Add multicast entry " MAC_FMT "\n", > MAC_ARG(multicast_addr))); > unsigned mcast_idx = (net_crc32(multicast_addr, ETH_ALEN) & > @@ -260,8 +259,8 @@ static void command_loop(I82596State *s) > byte_cnt = MAX(byte_cnt, 4); > byte_cnt = MIN(byte_cnt, sizeof(s->config)); > /* copy byte_cnt max. */ > - address_space_rw(&address_space_memory, s->cmd_p + 8, > - MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt, 0); > + address_space_read(&address_space_memory, s->cmd_p + 8, > + MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt); > /* config byte according to page 35ff */ > s->config[2] &= 0x82; /* mask valid bits */ > s->config[2] |= 0x40; > @@ -640,14 +639,14 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) > } > rba = get_uint32(rbd + 8); > /* printf("rba is 0x%x\n", rba); */ > - address_space_rw(&address_space_memory, rba, > - MEMTXATTRS_UNSPECIFIED, (void *)buf, num, 1); > + address_space_write(&address_space_memory, rba, > + MEMTXATTRS_UNSPECIFIED, (void *)buf, num); > rba += num; > buf += num; > len -= num; > if (len == 0) { /* copy crc */ > - address_space_rw(&address_space_memory, rba - 4, > - MEMTXATTRS_UNSPECIFIED, crc_ptr, 4, 1); > + address_space_write(&address_space_memory, rba - 4, > + MEMTXATTRS_UNSPECIFIED, crc_ptr, 4); > } > > num |= 0x4000; /* set F BIT */ > diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c > index 427b3fbf701..52637a562d8 100644 > --- a/hw/net/lasi_i82596.c > +++ b/hw/net/lasi_i82596.c > @@ -55,8 +55,9 @@ static void lasi_82596_mem_write(void *opaque, hwaddr addr, > * Provided for SeaBIOS only. Write MAC of Network card to addr @val. > * Needed for the PDC_LAN_STATION_ID_READ PDC call. > */ > - address_space_rw(&address_space_memory, val, > - MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, ETH_ALEN, 1); > + address_space_write(&address_space_memory, val, > + MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, > + ETH_ALEN); > break; > } > } > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c > index 5989d723c50..f150deca340 100644 > --- a/hw/ppc/pnv_lpc.c > +++ b/hw/ppc/pnv_lpc.c > @@ -238,16 +238,16 @@ static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits and FW read caching here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, false); > + return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, true); > + return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > #define ECCB_CTL_READ PPC_BIT(15) > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 844caab4082..0e0fccd050e 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -874,18 +874,18 @@ static inline int ida_read_next_idaw(CcwDataStream *cds) > if (idaw_addr & 0x07 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt2, > - sizeof(idaw.fmt2), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt2, > + sizeof(idaw.fmt2)); > cds->cda = be64_to_cpu(idaw.fmt2); > } else { > idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw; > if (idaw_addr & 0x03 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt1, > - sizeof(idaw.fmt1), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt1, > + sizeof(idaw.fmt1)); > cds->cda = be64_to_cpu(idaw.fmt1); > if (cds->cda & 0x80000000) { > return -EINVAL; /* channel program check */ > diff --git a/qtest.c b/qtest.c > index 12432f99cf4..328d674bcc8 100644 > --- a/qtest.c > +++ b/qtest.c > @@ -429,23 +429,23 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][5] == 'b') { > uint8_t data = value; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > } else if (words[0][5] == 'w') { > uint16_t data = value; > tswap16s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > } else if (words[0][5] == 'l') { > uint32_t data = value; > tswap32s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > } else if (words[0][5] == 'q') { > uint64_t data = value; > tswap64s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 8, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 8); > } > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > @@ -463,22 +463,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][4] == 'b') { > uint8_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > value = data; > } else if (words[0][4] == 'w') { > uint16_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > value = tswap16(data); > } else if (words[0][4] == 'l') { > uint32_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > value = tswap32(data); > } else if (words[0][4] == 'q') { > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &value, 8, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&value, 8); > tswap64s(&value); > } > qtest_send_prefix(chr); > @@ -498,8 +498,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(len); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > enc = g_malloc(2 * len + 1); > for (i = 0; i < len; i++) { > @@ -524,8 +524,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(ret == 0); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > b64_data = g_base64_encode(data, len); > qtest_send_prefix(chr); > qtest_sendf(chr, "OK %s\n", b64_data); > @@ -559,8 +559,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > data[i] = 0; > } > } > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > g_free(data); > > qtest_send_prefix(chr); > @@ -582,8 +582,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > if (len) { > data = g_malloc(len); > memset(data, pattern, len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + data, len); > g_free(data); > } > > @@ -616,8 +616,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > out_len = MIN(out_len, len); > } > > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > diff --git a/target/i386/hvf/x86_mmu.c b/target/i386/hvf/x86_mmu.c > index d5a0efe7188..ff016fc0145 100644 > --- a/target/i386/hvf/x86_mmu.c > +++ b/target/i386/hvf/x86_mmu.c > @@ -88,8 +88,8 @@ static bool get_pt_entry(struct CPUState *cpu, struct gpt_translation *pt, > } > > index = gpt_entry(pt->gva, level, pae); > - address_space_rw(&address_space_memory, gpa + index * pte_size(pae), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae), 0); > + address_space_read(&address_space_memory, gpa + index * pte_size(pae), > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae)); > > pt->pte[level - 1] = pte; > > @@ -238,8 +238,8 @@ void vmx_write_mem(struct CPUState *cpu, target_ulong gva, void *data, int bytes > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } else { > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 1); > + address_space_write(&address_space_memory, gpa, > + MEMTXATTRS_UNSPECIFIED, data, copy); > } > > bytes -= copy; > @@ -259,8 +259,8 @@ void vmx_read_mem(struct CPUState *cpu, void *data, target_ulong gva, int bytes) > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 0); > + address_space_read(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > + data, copy); > > bytes -= copy; > gva += copy; > diff --git a/scripts/coccinelle/as_rw_const.cocci b/scripts/coccinelle/as_rw_const.cocci > new file mode 100644 > index 00000000000..30da707701b > --- /dev/null > +++ b/scripts/coccinelle/as_rw_const.cocci > @@ -0,0 +1,30 @@ > +// Avoid uses of address_space_rw() with a constant is_write argument. > +// Usage: > +// spatch --sp-file as-rw-const.spatch --dir . --in-place Nitpick, script is now scripts/coccinelle/as_rw_const.cocci. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + > +@@ > +expression E1, E2, E3, E4, E5; > +symbol false; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, false) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 0) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +symbol true; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, true) > ++ address_space_write(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 1) > ++ address_space_write(E1, E2, E3, E4, E5) >
On 18/02/2020 12:24, Peter Maydell wrote: > The address_space_rw() function allows either reads or writes > depending on the is_write argument passed to it; this is useful > when the direction of the access is determined programmatically > (as for instance when handling the KVM_EXIT_MMIO exit reason). > Under the hood it just calls either address_space_write() or > address_space_read_full(). > > We also use it a lot with a constant is_write argument, though, > which has two issues: > * when reading "address_space_rw(..., 1)" this is less > immediately clear to the reader as being a write than > "address_space_write(...)" > * calling address_space_rw() bypasses the optimization > in address_space_read() that fast-paths reads of a > fixed length > > This commit was produced with the included Coccinelle script > scripts/coccinelle/as-rw-const.patch. > > Two lines in hw/net/dp8393x.c that Coccinelle produced that > were over 80 characters were re-wrapped by hand. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I could break this down into separate patches by submaintainer, > but the patch is not that large and I would argue that it's > better for the project if we can try to avoid introducing too > much friction into the process of doing 'safe' tree-wide > minor refactorings. > > v1->v2: put the coccinelle script in scripts/coccinelle rather > than just in the commit message. > --- > accel/kvm/kvm-all.c | 6 +-- > dma-helpers.c | 4 +- > exec.c | 4 +- > hw/dma/xlnx-zdma.c | 11 ++--- > hw/net/dp8393x.c | 68 ++++++++++++++-------------- > hw/net/i82596.c | 25 +++++----- > hw/net/lasi_i82596.c | 5 +- > hw/ppc/pnv_lpc.c | 8 ++-- > hw/s390x/css.c | 12 ++--- > qtest.c | 52 ++++++++++----------- > target/i386/hvf/x86_mmu.c | 12 ++--- > scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ > 12 files changed, 133 insertions(+), 104 deletions(-) > create mode 100644 scripts/coccinelle/as_rw_const.cocci There is one in target/i386/hvf/vmx.h: macvm_set_cr0() you didn't change. You must update the script name in the script comment (as suggested by Philippe) and in the commit message. Anyway: Reviewed-by: Laurent Vivier <lvivier@redhat.com> > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index c111312dfdd..0cfe6fd8ded 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2178,9 +2178,9 @@ void kvm_flush_coalesced_mmio_buffer(void) > ent = &ring->coalesced_mmio[ring->first]; > > if (ent->pio == 1) { > - address_space_rw(&address_space_io, ent->phys_addr, > - MEMTXATTRS_UNSPECIFIED, ent->data, > - ent->len, true); > + address_space_write(&address_space_io, ent->phys_addr, > + MEMTXATTRS_UNSPECIFIED, ent->data, > + ent->len); > } else { > cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len); > } > diff --git a/dma-helpers.c b/dma-helpers.c > index d3871dc61ea..e8a26e81e16 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -28,8 +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, MEMTXATTRS_UNSPECIFIED, > - fillbuf, l, true); > + error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, > + fillbuf, l); > len -= l; > addr += l; > } > diff --git a/exec.c b/exec.c > index 8e9cc3b47cf..baefe582393 100644 > --- a/exec.c > +++ b/exec.c > @@ -3810,8 +3810,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, > attrs, buf, l); > } else { > - address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, > - attrs, buf, l, 0); > + address_space_read(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, > + l); > } > len -= l; > buf += l; > diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c > index 8fb83f5b078..31936061e21 100644 > --- a/hw/dma/xlnx-zdma.c > +++ b/hw/dma/xlnx-zdma.c > @@ -311,8 +311,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) > return false; > } > > - address_space_rw(s->dma_as, addr, s->attr, > - buf, sizeof(XlnxZDMADescr), false); > + address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr)); > return true; > } > > @@ -364,7 +363,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA *s, bool type, > } else { > addr = zdma_get_regaddr64(s, basereg); > addr += sizeof(s->dsc_dst); > - address_space_rw(s->dma_as, addr, s->attr, (void *) &next, 8, false); > + address_space_read(s->dma_as, addr, s->attr, (void *)&next, 8); > zdma_put_regaddr64(s, basereg, next); > } > return next; > @@ -416,8 +415,7 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) > } > } > > - address_space_rw(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen, > - true); > + address_space_write(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen); > if (burst_type == AXI_BURST_INCR) { > s->dsc_dst.addr += dlen; > } > @@ -493,8 +491,7 @@ static void zdma_process_descr(XlnxZDMA *s) > len = s->cfg.bus_width / 8; > } > } else { > - address_space_rw(s->dma_as, src_addr, s->attr, s->buf, len, > - false); > + address_space_read(s->dma_as, src_addr, s->attr, s->buf, len); > if (burst_type == AXI_BURST_INCR) { > src_addr += len; > } > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index a134d431ae3..f5f1c669e80 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -275,8 +275,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > /* Fill current entry */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff; > s->cam[index][1] = dp8393x_get(s, width, 1) >> 8; > s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff; > @@ -293,8 +293,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > } > > /* Read CAM enable */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); > > @@ -311,8 +311,8 @@ static void dp8393x_do_read_rra(dp8393xState *s) > /* Read memory */ > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > size = sizeof(uint16_t) * 4 * width; > - address_space_rw(&s->as, dp8393x_rrp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Update SONIC registers */ > s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > @@ -426,8 +426,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > size = sizeof(uint16_t) * 6 * width; > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); > - address_space_rw(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > tx_len = 0; > > /* Update registers */ > @@ -451,17 +451,19 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > if (tx_len + len > sizeof(s->tx_buffer)) { > len = sizeof(s->tx_buffer) - tx_len; > } > - address_space_rw(&s->as, dp8393x_tsa(s), > - MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); > + address_space_read(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, > + &s->tx_buffer[tx_len], len); > tx_len += len; > > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > size = sizeof(uint16_t) * 3 * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * i) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > @@ -494,18 +496,18 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > dp8393x_put(s, width, 0, > s->regs[SONIC_TCR] & 0x0fff); /* status */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > /* Read footer of packet */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + > - sizeof(uint16_t) * > - (4 + 3 * s->regs[SONIC_TFC]) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) > + * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1; > if (dp8393x_get(s, width, 0) & 0x1) { > /* EOL detected */ > @@ -767,8 +769,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Are we still in resource exhaustion? */ > size = sizeof(uint16_t) * 1 * width; > address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > if (dp8393x_get(s, width, 0) & 0x1) { > /* Still EOL ; stop reception */ > return -1; > @@ -787,11 +789,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Put packet into RBA */ > DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); > address = dp8393x_crba(s); > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)buf, rx_len); > address += rx_len; > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&checksum, 4); > rx_len += 4; > s->regs[SONIC_CRBA1] = address >> 16; > s->regs[SONIC_CRBA0] = address & 0xffff; > @@ -819,13 +821,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > size = sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, dp8393x_crda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Move to next descriptor */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > if (s->regs[SONIC_LLFA] & 0x1) { > /* EOL detected */ > @@ -838,8 +840,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > offset += sizeof(uint16_t); > } > s->data[0] = 0; > - address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, sizeof(uint16_t), 1); > + address_space_write(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, sizeof(uint16_t)); > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; > s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); > diff --git a/hw/net/i82596.c b/hw/net/i82596.c > index 3a0e1ec4c05..6a80c24af23 100644 > --- a/hw/net/i82596.c > +++ b/hw/net/i82596.c > @@ -148,8 +148,8 @@ static void i82596_transmit(I82596State *s, uint32_t addr) > > if (s->nic && len) { > assert(len <= sizeof(s->tx_buffer)); > - address_space_rw(&address_space_memory, tba, > - MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len, 0); > + address_space_read(&address_space_memory, tba, > + MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len); > DBG(PRINT_PKTHDR("Send", &s->tx_buffer)); > DBG(printf("Sending %d bytes\n", len)); > qemu_send_packet(qemu_get_queue(s->nic), s->tx_buffer, len); > @@ -172,8 +172,8 @@ static void set_individual_address(I82596State *s, uint32_t addr) > > nc = qemu_get_queue(s->nic); > m = s->conf.macaddr.a; > - address_space_rw(&address_space_memory, addr + 8, > - MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + 8, > + MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN); > qemu_format_nic_info_str(nc, m); > trace_i82596_new_mac(nc->info_str); > } > @@ -190,9 +190,8 @@ static void set_multicast_list(I82596State *s, uint32_t addr) > } > for (i = 0; i < mc_count; i++) { > uint8_t multicast_addr[ETH_ALEN]; > - address_space_rw(&address_space_memory, > - addr + i * ETH_ALEN, MEMTXATTRS_UNSPECIFIED, > - multicast_addr, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + i * ETH_ALEN, > + MEMTXATTRS_UNSPECIFIED, multicast_addr, ETH_ALEN); > DBG(printf("Add multicast entry " MAC_FMT "\n", > MAC_ARG(multicast_addr))); > unsigned mcast_idx = (net_crc32(multicast_addr, ETH_ALEN) & > @@ -260,8 +259,8 @@ static void command_loop(I82596State *s) > byte_cnt = MAX(byte_cnt, 4); > byte_cnt = MIN(byte_cnt, sizeof(s->config)); > /* copy byte_cnt max. */ > - address_space_rw(&address_space_memory, s->cmd_p + 8, > - MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt, 0); > + address_space_read(&address_space_memory, s->cmd_p + 8, > + MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt); > /* config byte according to page 35ff */ > s->config[2] &= 0x82; /* mask valid bits */ > s->config[2] |= 0x40; > @@ -640,14 +639,14 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) > } > rba = get_uint32(rbd + 8); > /* printf("rba is 0x%x\n", rba); */ > - address_space_rw(&address_space_memory, rba, > - MEMTXATTRS_UNSPECIFIED, (void *)buf, num, 1); > + address_space_write(&address_space_memory, rba, > + MEMTXATTRS_UNSPECIFIED, (void *)buf, num); > rba += num; > buf += num; > len -= num; > if (len == 0) { /* copy crc */ > - address_space_rw(&address_space_memory, rba - 4, > - MEMTXATTRS_UNSPECIFIED, crc_ptr, 4, 1); > + address_space_write(&address_space_memory, rba - 4, > + MEMTXATTRS_UNSPECIFIED, crc_ptr, 4); > } > > num |= 0x4000; /* set F BIT */ > diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c > index 427b3fbf701..52637a562d8 100644 > --- a/hw/net/lasi_i82596.c > +++ b/hw/net/lasi_i82596.c > @@ -55,8 +55,9 @@ static void lasi_82596_mem_write(void *opaque, hwaddr addr, > * Provided for SeaBIOS only. Write MAC of Network card to addr @val. > * Needed for the PDC_LAN_STATION_ID_READ PDC call. > */ > - address_space_rw(&address_space_memory, val, > - MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, ETH_ALEN, 1); > + address_space_write(&address_space_memory, val, > + MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, > + ETH_ALEN); > break; > } > } > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c > index 5989d723c50..f150deca340 100644 > --- a/hw/ppc/pnv_lpc.c > +++ b/hw/ppc/pnv_lpc.c > @@ -238,16 +238,16 @@ static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits and FW read caching here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, false); > + return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, true); > + return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > #define ECCB_CTL_READ PPC_BIT(15) > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 844caab4082..0e0fccd050e 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -874,18 +874,18 @@ static inline int ida_read_next_idaw(CcwDataStream *cds) > if (idaw_addr & 0x07 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt2, > - sizeof(idaw.fmt2), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt2, > + sizeof(idaw.fmt2)); > cds->cda = be64_to_cpu(idaw.fmt2); > } else { > idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw; > if (idaw_addr & 0x03 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt1, > - sizeof(idaw.fmt1), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt1, > + sizeof(idaw.fmt1)); > cds->cda = be64_to_cpu(idaw.fmt1); > if (cds->cda & 0x80000000) { > return -EINVAL; /* channel program check */ > diff --git a/qtest.c b/qtest.c > index 12432f99cf4..328d674bcc8 100644 > --- a/qtest.c > +++ b/qtest.c > @@ -429,23 +429,23 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][5] == 'b') { > uint8_t data = value; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > } else if (words[0][5] == 'w') { > uint16_t data = value; > tswap16s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > } else if (words[0][5] == 'l') { > uint32_t data = value; > tswap32s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > } else if (words[0][5] == 'q') { > uint64_t data = value; > tswap64s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 8, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 8); > } > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > @@ -463,22 +463,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][4] == 'b') { > uint8_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > value = data; > } else if (words[0][4] == 'w') { > uint16_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > value = tswap16(data); > } else if (words[0][4] == 'l') { > uint32_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > value = tswap32(data); > } else if (words[0][4] == 'q') { > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &value, 8, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&value, 8); > tswap64s(&value); > } > qtest_send_prefix(chr); > @@ -498,8 +498,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(len); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > enc = g_malloc(2 * len + 1); > for (i = 0; i < len; i++) { > @@ -524,8 +524,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(ret == 0); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > b64_data = g_base64_encode(data, len); > qtest_send_prefix(chr); > qtest_sendf(chr, "OK %s\n", b64_data); > @@ -559,8 +559,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > data[i] = 0; > } > } > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > g_free(data); > > qtest_send_prefix(chr); > @@ -582,8 +582,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > if (len) { > data = g_malloc(len); > memset(data, pattern, len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + data, len); > g_free(data); > } > > @@ -616,8 +616,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > out_len = MIN(out_len, len); > } > > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > diff --git a/target/i386/hvf/x86_mmu.c b/target/i386/hvf/x86_mmu.c > index d5a0efe7188..ff016fc0145 100644 > --- a/target/i386/hvf/x86_mmu.c > +++ b/target/i386/hvf/x86_mmu.c > @@ -88,8 +88,8 @@ static bool get_pt_entry(struct CPUState *cpu, struct gpt_translation *pt, > } > > index = gpt_entry(pt->gva, level, pae); > - address_space_rw(&address_space_memory, gpa + index * pte_size(pae), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae), 0); > + address_space_read(&address_space_memory, gpa + index * pte_size(pae), > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae)); > > pt->pte[level - 1] = pte; > > @@ -238,8 +238,8 @@ void vmx_write_mem(struct CPUState *cpu, target_ulong gva, void *data, int bytes > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } else { > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 1); > + address_space_write(&address_space_memory, gpa, > + MEMTXATTRS_UNSPECIFIED, data, copy); > } > > bytes -= copy; > @@ -259,8 +259,8 @@ void vmx_read_mem(struct CPUState *cpu, void *data, target_ulong gva, int bytes) > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 0); > + address_space_read(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > + data, copy); > > bytes -= copy; > gva += copy; > diff --git a/scripts/coccinelle/as_rw_const.cocci b/scripts/coccinelle/as_rw_const.cocci > new file mode 100644 > index 00000000000..30da707701b > --- /dev/null > +++ b/scripts/coccinelle/as_rw_const.cocci > @@ -0,0 +1,30 @@ > +// Avoid uses of address_space_rw() with a constant is_write argument. > +// Usage: > +// spatch --sp-file as-rw-const.spatch --dir . --in-place > + > +@@ > +expression E1, E2, E3, E4, E5; > +symbol false; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, false) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 0) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +symbol true; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, true) > ++ address_space_write(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 1) > ++ address_space_write(E1, E2, E3, E4, E5) >
On Tue, 18 Feb 2020 at 13:14, Laurent Vivier <lvivier@redhat.com> wrote:
> There is one in target/i386/hvf/vmx.h: macvm_set_cr0() you didn't change.
It turns out that "spatch --dir ." only looks at .c files,
and you need to add --include-headers to get it to look at
.h files too. The only extra change is this one:
diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index eb8894cd589..c245dc6d5bb 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -125,10 +125,9 @@ static inline void macvm_set_cr0(hv_vcpuid_t
vcpu, uint64_t cr0)
if ((cr0 & CR0_PG) && (rvmcs(vcpu, VMCS_GUEST_CR4) & CR4_PAE) &&
!(efer & MSR_EFER_LME)) {
- address_space_rw(&address_space_memory,
- rvmcs(vcpu, VMCS_GUEST_CR3) & ~0x1f,
- MEMTXATTRS_UNSPECIFIED,
- (uint8_t *)pdpte, 32, 0);
+ address_space_read(&address_space_memory,
+ rvmcs(vcpu, VMCS_GUEST_CR3) & ~0x1f,
+ MEMTXATTRS_UNSPECIFIED, (uint8_t *)pdpte, 32);
/* Only set PDPTE when appropriate. */
for (i = 0; i < 4; i++) {
wvmcs(vcpu, VMCS_GUEST_PDPTE0 + i * 2, pdpte[i]);
which I propose to squash in, together with the fixes to use
the right filename in the commit message and comment.
thanks
-- PMM
On 2/18/20 12:24 PM, Peter Maydell wrote: > The address_space_rw() function allows either reads or writes > depending on the is_write argument passed to it; this is useful > when the direction of the access is determined programmatically > (as for instance when handling the KVM_EXIT_MMIO exit reason). > Under the hood it just calls either address_space_write() or > address_space_read_full(). > > We also use it a lot with a constant is_write argument, though, > which has two issues: > * when reading "address_space_rw(..., 1)" this is less > immediately clear to the reader as being a write than > "address_space_write(...)" > * calling address_space_rw() bypasses the optimization > in address_space_read() that fast-paths reads of a > fixed length > > This commit was produced with the included Coccinelle script > scripts/coccinelle/as-rw-const.patch. > > Two lines in hw/net/dp8393x.c that Coccinelle produced that > were over 80 characters were re-wrapped by hand. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> > --- > I could break this down into separate patches by submaintainer, > but the patch is not that large and I would argue that it's > better for the project if we can try to avoid introducing too > much friction into the process of doing 'safe' tree-wide > minor refactorings. > > v1->v2: put the coccinelle script in scripts/coccinelle rather > than just in the commit message. > --- > accel/kvm/kvm-all.c | 6 +-- > dma-helpers.c | 4 +- > exec.c | 4 +- > hw/dma/xlnx-zdma.c | 11 ++--- > hw/net/dp8393x.c | 68 ++++++++++++++-------------- > hw/net/i82596.c | 25 +++++----- > hw/net/lasi_i82596.c | 5 +- > hw/ppc/pnv_lpc.c | 8 ++-- > hw/s390x/css.c | 12 ++--- > qtest.c | 52 ++++++++++----------- > target/i386/hvf/x86_mmu.c | 12 ++--- > scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ > 12 files changed, 133 insertions(+), 104 deletions(-) > create mode 100644 scripts/coccinelle/as_rw_const.cocci > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index c111312dfdd..0cfe6fd8ded 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2178,9 +2178,9 @@ void kvm_flush_coalesced_mmio_buffer(void) > ent = &ring->coalesced_mmio[ring->first]; > > if (ent->pio == 1) { > - address_space_rw(&address_space_io, ent->phys_addr, > - MEMTXATTRS_UNSPECIFIED, ent->data, > - ent->len, true); > + address_space_write(&address_space_io, ent->phys_addr, > + MEMTXATTRS_UNSPECIFIED, ent->data, > + ent->len); > } else { > cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len); > } > diff --git a/dma-helpers.c b/dma-helpers.c > index d3871dc61ea..e8a26e81e16 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -28,8 +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, MEMTXATTRS_UNSPECIFIED, > - fillbuf, l, true); > + error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, > + fillbuf, l); > len -= l; > addr += l; > } > diff --git a/exec.c b/exec.c > index 8e9cc3b47cf..baefe582393 100644 > --- a/exec.c > +++ b/exec.c > @@ -3810,8 +3810,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, > attrs, buf, l); > } else { > - address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, > - attrs, buf, l, 0); > + address_space_read(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, > + l); > } > len -= l; > buf += l; > diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c > index 8fb83f5b078..31936061e21 100644 > --- a/hw/dma/xlnx-zdma.c > +++ b/hw/dma/xlnx-zdma.c > @@ -311,8 +311,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) > return false; > } > > - address_space_rw(s->dma_as, addr, s->attr, > - buf, sizeof(XlnxZDMADescr), false); > + address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr)); > return true; > } > > @@ -364,7 +363,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA *s, bool type, > } else { > addr = zdma_get_regaddr64(s, basereg); > addr += sizeof(s->dsc_dst); > - address_space_rw(s->dma_as, addr, s->attr, (void *) &next, 8, false); > + address_space_read(s->dma_as, addr, s->attr, (void *)&next, 8); > zdma_put_regaddr64(s, basereg, next); > } > return next; > @@ -416,8 +415,7 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) > } > } > > - address_space_rw(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen, > - true); > + address_space_write(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen); > if (burst_type == AXI_BURST_INCR) { > s->dsc_dst.addr += dlen; > } > @@ -493,8 +491,7 @@ static void zdma_process_descr(XlnxZDMA *s) > len = s->cfg.bus_width / 8; > } > } else { > - address_space_rw(s->dma_as, src_addr, s->attr, s->buf, len, > - false); > + address_space_read(s->dma_as, src_addr, s->attr, s->buf, len); > if (burst_type == AXI_BURST_INCR) { > src_addr += len; > } > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index a134d431ae3..f5f1c669e80 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -275,8 +275,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > /* Fill current entry */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff; > s->cam[index][1] = dp8393x_get(s, width, 1) >> 8; > s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff; > @@ -293,8 +293,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > } > > /* Read CAM enable */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); > > @@ -311,8 +311,8 @@ static void dp8393x_do_read_rra(dp8393xState *s) > /* Read memory */ > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > size = sizeof(uint16_t) * 4 * width; > - address_space_rw(&s->as, dp8393x_rrp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Update SONIC registers */ > s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > @@ -426,8 +426,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > size = sizeof(uint16_t) * 6 * width; > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); > - address_space_rw(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > tx_len = 0; > > /* Update registers */ > @@ -451,17 +451,19 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > if (tx_len + len > sizeof(s->tx_buffer)) { > len = sizeof(s->tx_buffer) - tx_len; > } > - address_space_rw(&s->as, dp8393x_tsa(s), > - MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); > + address_space_read(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, > + &s->tx_buffer[tx_len], len); > tx_len += len; > > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > size = sizeof(uint16_t) * 3 * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * i) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > @@ -494,18 +496,18 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > dp8393x_put(s, width, 0, > s->regs[SONIC_TCR] & 0x0fff); /* status */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > /* Read footer of packet */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + > - sizeof(uint16_t) * > - (4 + 3 * s->regs[SONIC_TFC]) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) > + * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1; > if (dp8393x_get(s, width, 0) & 0x1) { > /* EOL detected */ > @@ -767,8 +769,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Are we still in resource exhaustion? */ > size = sizeof(uint16_t) * 1 * width; > address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > if (dp8393x_get(s, width, 0) & 0x1) { > /* Still EOL ; stop reception */ > return -1; > @@ -787,11 +789,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Put packet into RBA */ > DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); > address = dp8393x_crba(s); > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)buf, rx_len); > address += rx_len; > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&checksum, 4); > rx_len += 4; > s->regs[SONIC_CRBA1] = address >> 16; > s->regs[SONIC_CRBA0] = address & 0xffff; > @@ -819,13 +821,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > size = sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, dp8393x_crda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Move to next descriptor */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > if (s->regs[SONIC_LLFA] & 0x1) { > /* EOL detected */ > @@ -838,8 +840,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > offset += sizeof(uint16_t); > } > s->data[0] = 0; > - address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, sizeof(uint16_t), 1); > + address_space_write(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, sizeof(uint16_t)); > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; > s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); > diff --git a/hw/net/i82596.c b/hw/net/i82596.c > index 3a0e1ec4c05..6a80c24af23 100644 > --- a/hw/net/i82596.c > +++ b/hw/net/i82596.c > @@ -148,8 +148,8 @@ static void i82596_transmit(I82596State *s, uint32_t addr) > > if (s->nic && len) { > assert(len <= sizeof(s->tx_buffer)); > - address_space_rw(&address_space_memory, tba, > - MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len, 0); > + address_space_read(&address_space_memory, tba, > + MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len); > DBG(PRINT_PKTHDR("Send", &s->tx_buffer)); > DBG(printf("Sending %d bytes\n", len)); > qemu_send_packet(qemu_get_queue(s->nic), s->tx_buffer, len); > @@ -172,8 +172,8 @@ static void set_individual_address(I82596State *s, uint32_t addr) > > nc = qemu_get_queue(s->nic); > m = s->conf.macaddr.a; > - address_space_rw(&address_space_memory, addr + 8, > - MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + 8, > + MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN); > qemu_format_nic_info_str(nc, m); > trace_i82596_new_mac(nc->info_str); > } > @@ -190,9 +190,8 @@ static void set_multicast_list(I82596State *s, uint32_t addr) > } > for (i = 0; i < mc_count; i++) { > uint8_t multicast_addr[ETH_ALEN]; > - address_space_rw(&address_space_memory, > - addr + i * ETH_ALEN, MEMTXATTRS_UNSPECIFIED, > - multicast_addr, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + i * ETH_ALEN, > + MEMTXATTRS_UNSPECIFIED, multicast_addr, ETH_ALEN); > DBG(printf("Add multicast entry " MAC_FMT "\n", > MAC_ARG(multicast_addr))); > unsigned mcast_idx = (net_crc32(multicast_addr, ETH_ALEN) & > @@ -260,8 +259,8 @@ static void command_loop(I82596State *s) > byte_cnt = MAX(byte_cnt, 4); > byte_cnt = MIN(byte_cnt, sizeof(s->config)); > /* copy byte_cnt max. */ > - address_space_rw(&address_space_memory, s->cmd_p + 8, > - MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt, 0); > + address_space_read(&address_space_memory, s->cmd_p + 8, > + MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt); > /* config byte according to page 35ff */ > s->config[2] &= 0x82; /* mask valid bits */ > s->config[2] |= 0x40; > @@ -640,14 +639,14 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) > } > rba = get_uint32(rbd + 8); > /* printf("rba is 0x%x\n", rba); */ > - address_space_rw(&address_space_memory, rba, > - MEMTXATTRS_UNSPECIFIED, (void *)buf, num, 1); > + address_space_write(&address_space_memory, rba, > + MEMTXATTRS_UNSPECIFIED, (void *)buf, num); > rba += num; > buf += num; > len -= num; > if (len == 0) { /* copy crc */ > - address_space_rw(&address_space_memory, rba - 4, > - MEMTXATTRS_UNSPECIFIED, crc_ptr, 4, 1); > + address_space_write(&address_space_memory, rba - 4, > + MEMTXATTRS_UNSPECIFIED, crc_ptr, 4); > } > > num |= 0x4000; /* set F BIT */ > diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c > index 427b3fbf701..52637a562d8 100644 > --- a/hw/net/lasi_i82596.c > +++ b/hw/net/lasi_i82596.c > @@ -55,8 +55,9 @@ static void lasi_82596_mem_write(void *opaque, hwaddr addr, > * Provided for SeaBIOS only. Write MAC of Network card to addr @val. > * Needed for the PDC_LAN_STATION_ID_READ PDC call. > */ > - address_space_rw(&address_space_memory, val, > - MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, ETH_ALEN, 1); > + address_space_write(&address_space_memory, val, > + MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, > + ETH_ALEN); > break; > } > } > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c > index 5989d723c50..f150deca340 100644 > --- a/hw/ppc/pnv_lpc.c > +++ b/hw/ppc/pnv_lpc.c > @@ -238,16 +238,16 @@ static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits and FW read caching here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, false); > + return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, true); > + return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > #define ECCB_CTL_READ PPC_BIT(15) > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 844caab4082..0e0fccd050e 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -874,18 +874,18 @@ static inline int ida_read_next_idaw(CcwDataStream *cds) > if (idaw_addr & 0x07 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt2, > - sizeof(idaw.fmt2), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt2, > + sizeof(idaw.fmt2)); > cds->cda = be64_to_cpu(idaw.fmt2); > } else { > idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw; > if (idaw_addr & 0x03 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt1, > - sizeof(idaw.fmt1), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt1, > + sizeof(idaw.fmt1)); > cds->cda = be64_to_cpu(idaw.fmt1); > if (cds->cda & 0x80000000) { > return -EINVAL; /* channel program check */ > diff --git a/qtest.c b/qtest.c > index 12432f99cf4..328d674bcc8 100644 > --- a/qtest.c > +++ b/qtest.c > @@ -429,23 +429,23 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][5] == 'b') { > uint8_t data = value; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > } else if (words[0][5] == 'w') { > uint16_t data = value; > tswap16s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > } else if (words[0][5] == 'l') { > uint32_t data = value; > tswap32s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > } else if (words[0][5] == 'q') { > uint64_t data = value; > tswap64s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 8, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 8); > } > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > @@ -463,22 +463,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][4] == 'b') { > uint8_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > value = data; > } else if (words[0][4] == 'w') { > uint16_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > value = tswap16(data); > } else if (words[0][4] == 'l') { > uint32_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > value = tswap32(data); > } else if (words[0][4] == 'q') { > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &value, 8, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&value, 8); > tswap64s(&value); > } > qtest_send_prefix(chr); > @@ -498,8 +498,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(len); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > enc = g_malloc(2 * len + 1); > for (i = 0; i < len; i++) { > @@ -524,8 +524,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(ret == 0); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > b64_data = g_base64_encode(data, len); > qtest_send_prefix(chr); > qtest_sendf(chr, "OK %s\n", b64_data); > @@ -559,8 +559,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > data[i] = 0; > } > } > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > g_free(data); > > qtest_send_prefix(chr); > @@ -582,8 +582,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > if (len) { > data = g_malloc(len); > memset(data, pattern, len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + data, len); > g_free(data); > } > > @@ -616,8 +616,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > out_len = MIN(out_len, len); > } > > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > diff --git a/target/i386/hvf/x86_mmu.c b/target/i386/hvf/x86_mmu.c > index d5a0efe7188..ff016fc0145 100644 > --- a/target/i386/hvf/x86_mmu.c > +++ b/target/i386/hvf/x86_mmu.c > @@ -88,8 +88,8 @@ static bool get_pt_entry(struct CPUState *cpu, struct gpt_translation *pt, > } > > index = gpt_entry(pt->gva, level, pae); > - address_space_rw(&address_space_memory, gpa + index * pte_size(pae), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae), 0); > + address_space_read(&address_space_memory, gpa + index * pte_size(pae), > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae)); > > pt->pte[level - 1] = pte; > > @@ -238,8 +238,8 @@ void vmx_write_mem(struct CPUState *cpu, target_ulong gva, void *data, int bytes > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } else { > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 1); > + address_space_write(&address_space_memory, gpa, > + MEMTXATTRS_UNSPECIFIED, data, copy); > } > > bytes -= copy; > @@ -259,8 +259,8 @@ void vmx_read_mem(struct CPUState *cpu, void *data, target_ulong gva, int bytes) > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 0); > + address_space_read(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > + data, copy); > > bytes -= copy; > gva += copy; > diff --git a/scripts/coccinelle/as_rw_const.cocci b/scripts/coccinelle/as_rw_const.cocci > new file mode 100644 > index 00000000000..30da707701b > --- /dev/null > +++ b/scripts/coccinelle/as_rw_const.cocci > @@ -0,0 +1,30 @@ > +// Avoid uses of address_space_rw() with a constant is_write argument. > +// Usage: > +// spatch --sp-file as-rw-const.spatch --dir . --in-place > + > +@@ > +expression E1, E2, E3, E4, E5; > +symbol false; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, false) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 0) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +symbol true; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, true) > ++ address_space_write(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 1) > ++ address_space_write(E1, E2, E3, E4, E5) >
On 2/18/20 6:56 AM, Philippe Mathieu-Daudé wrote: >> +++ b/scripts/coccinelle/as_rw_const.cocci >> @@ -0,0 +1,30 @@ >> +// Avoid uses of address_space_rw() with a constant is_write argument. >> +// Usage: >> +// spatch --sp-file as-rw-const.spatch --dir . --in-place > > Nitpick, script is now scripts/coccinelle/as_rw_const.cocci. > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> + >> +@@ >> +expression E1, E2, E3, E4, E5; >> +symbol false; >> +@@ >> + >> +- address_space_rw(E1, E2, E3, E4, E5, false) >> ++ address_space_read(E1, E2, E3, E4, E5) >> +@@ >> +expression E1, E2, E3, E4, E5; >> +@@ >> + >> +- address_space_rw(E1, E2, E3, E4, E5, 0) >> ++ address_space_read(E1, E2, E3, E4, E5) This feels a bit redundant. Doesn't coccinelle have enough smarts about isomorphisms (such as 0 == false, 1 == true) that it could get by with one @@ hunk instead of 2, if we come up with the right way to represent any isomorphism to a constant value? But admittedly, I don't know what that representation would actually be, and your verbose patch works even if it is not the most concise possible. So don't let my remarks hold this patch up.
On Tue, 18 Feb 2020 at 13:33, Eric Blake <eblake@redhat.com> wrote: > > On 2/18/20 6:56 AM, Philippe Mathieu-Daudé wrote: > > >> +++ b/scripts/coccinelle/as_rw_const.cocci > >> @@ -0,0 +1,30 @@ > >> +// Avoid uses of address_space_rw() with a constant is_write argument. > >> +// Usage: > >> +// spatch --sp-file as-rw-const.spatch --dir . --in-place > > > > Nitpick, script is now scripts/coccinelle/as_rw_const.cocci. > > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > >> + > >> +@@ > >> +expression E1, E2, E3, E4, E5; > >> +symbol false; > >> +@@ > >> + > >> +- address_space_rw(E1, E2, E3, E4, E5, false) > >> ++ address_space_read(E1, E2, E3, E4, E5) > >> +@@ > >> +expression E1, E2, E3, E4, E5; > >> +@@ > >> + > >> +- address_space_rw(E1, E2, E3, E4, E5, 0) > >> ++ address_space_read(E1, E2, E3, E4, E5) > > This feels a bit redundant. Doesn't coccinelle have enough smarts about > isomorphisms (such as 0 == false, 1 == true) that it could get by with > one @@ hunk instead of 2, if we come up with the right way to represent > any isomorphism to a constant value? But admittedly, I don't know what > that representation would actually be, and your verbose patch works even > if it is not the most concise possible. So don't let my remarks hold > this patch up. My experience with Coccinelle has generally been that trying to make semantic patches smaller and less redundant is futile and a massive timesink. In this case as far as I can tell Coccinelle has no idea at all about the existence of the 'bool' type and that 'true' and 'false' are equivalent to 1 and 0. Thus the 'symbol' declaration, otherwise it thinks that 'false' is a random semantic identifier and doesn't look for a literal match of it. thanks -- PMM
On 18.02.20 12:24, Peter Maydell wrote: > The address_space_rw() function allows either reads or writes > depending on the is_write argument passed to it; this is useful > when the direction of the access is determined programmatically > (as for instance when handling the KVM_EXIT_MMIO exit reason). > Under the hood it just calls either address_space_write() or > address_space_read_full(). > > We also use it a lot with a constant is_write argument, though, > which has two issues: > * when reading "address_space_rw(..., 1)" this is less > immediately clear to the reader as being a write than > "address_space_write(...)" > * calling address_space_rw() bypasses the optimization > in address_space_read() that fast-paths reads of a > fixed length > > This commit was produced with the included Coccinelle script > scripts/coccinelle/as-rw-const.patch. > > Two lines in hw/net/dp8393x.c that Coccinelle produced that > were over 80 characters were re-wrapped by hand. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> s390 part Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > I could break this down into separate patches by submaintainer, > but the patch is not that large and I would argue that it's > better for the project if we can try to avoid introducing too > much friction into the process of doing 'safe' tree-wide > minor refactorings. > > v1->v2: put the coccinelle script in scripts/coccinelle rather > than just in the commit message. [...] > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -874,18 +874,18 @@ static inline int ida_read_next_idaw(CcwDataStream *cds) > if (idaw_addr & 0x07 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt2, > - sizeof(idaw.fmt2), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt2, > + sizeof(idaw.fmt2)); > cds->cda = be64_to_cpu(idaw.fmt2); > } else { > idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw; > if (idaw_addr & 0x03 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt1, > - sizeof(idaw.fmt1), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt1, > + sizeof(idaw.fmt1)); > cds->cda = be64_to_cpu(idaw.fmt1); > if (cds->cda & 0x80000000) { > return -EINVAL; /* channel program check */
On Tue, 18 Feb 2020 11:24:57 +0000 Peter Maydell <peter.maydell@linaro.org> wrote: > The address_space_rw() function allows either reads or writes > depending on the is_write argument passed to it; this is useful > when the direction of the access is determined programmatically > (as for instance when handling the KVM_EXIT_MMIO exit reason). > Under the hood it just calls either address_space_write() or > address_space_read_full(). > > We also use it a lot with a constant is_write argument, though, > which has two issues: > * when reading "address_space_rw(..., 1)" this is less > immediately clear to the reader as being a write than > "address_space_write(...)" > * calling address_space_rw() bypasses the optimization > in address_space_read() that fast-paths reads of a > fixed length > > This commit was produced with the included Coccinelle script > scripts/coccinelle/as-rw-const.patch. > > Two lines in hw/net/dp8393x.c that Coccinelle produced that > were over 80 characters were re-wrapped by hand. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I could break this down into separate patches by submaintainer, > but the patch is not that large and I would argue that it's > better for the project if we can try to avoid introducing too > much friction into the process of doing 'safe' tree-wide > minor refactorings. > > v1->v2: put the coccinelle script in scripts/coccinelle rather > than just in the commit message. > --- > accel/kvm/kvm-all.c | 6 +-- > dma-helpers.c | 4 +- > exec.c | 4 +- > hw/dma/xlnx-zdma.c | 11 ++--- > hw/net/dp8393x.c | 68 ++++++++++++++-------------- > hw/net/i82596.c | 25 +++++----- > hw/net/lasi_i82596.c | 5 +- > hw/ppc/pnv_lpc.c | 8 ++-- > hw/s390x/css.c | 12 ++--- s390 part: Reviewed-by: Cornelia Huck <cohuck@redhat.com> > qtest.c | 52 ++++++++++----------- > target/i386/hvf/x86_mmu.c | 12 ++--- > scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ > 12 files changed, 133 insertions(+), 104 deletions(-) > create mode 100644 scripts/coccinelle/as_rw_const.cocci
On Tue, Feb 18, 2020 at 3:25 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > The address_space_rw() function allows either reads or writes > depending on the is_write argument passed to it; this is useful > when the direction of the access is determined programmatically > (as for instance when handling the KVM_EXIT_MMIO exit reason). > Under the hood it just calls either address_space_write() or > address_space_read_full(). > > We also use it a lot with a constant is_write argument, though, > which has two issues: > * when reading "address_space_rw(..., 1)" this is less > immediately clear to the reader as being a write than > "address_space_write(...)" > * calling address_space_rw() bypasses the optimization > in address_space_read() that fast-paths reads of a > fixed length > > This commit was produced with the included Coccinelle script > scripts/coccinelle/as-rw-const.patch. > > Two lines in hw/net/dp8393x.c that Coccinelle produced that > were over 80 characters were re-wrapped by hand. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > I could break this down into separate patches by submaintainer, > but the patch is not that large and I would argue that it's > better for the project if we can try to avoid introducing too > much friction into the process of doing 'safe' tree-wide > minor refactorings. > > v1->v2: put the coccinelle script in scripts/coccinelle rather > than just in the commit message. > --- > accel/kvm/kvm-all.c | 6 +-- > dma-helpers.c | 4 +- > exec.c | 4 +- > hw/dma/xlnx-zdma.c | 11 ++--- > hw/net/dp8393x.c | 68 ++++++++++++++-------------- > hw/net/i82596.c | 25 +++++----- > hw/net/lasi_i82596.c | 5 +- > hw/ppc/pnv_lpc.c | 8 ++-- > hw/s390x/css.c | 12 ++--- > qtest.c | 52 ++++++++++----------- > target/i386/hvf/x86_mmu.c | 12 ++--- > scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ > 12 files changed, 133 insertions(+), 104 deletions(-) > create mode 100644 scripts/coccinelle/as_rw_const.cocci > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index c111312dfdd..0cfe6fd8ded 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2178,9 +2178,9 @@ void kvm_flush_coalesced_mmio_buffer(void) > ent = &ring->coalesced_mmio[ring->first]; > > if (ent->pio == 1) { > - address_space_rw(&address_space_io, ent->phys_addr, > - MEMTXATTRS_UNSPECIFIED, ent->data, > - ent->len, true); > + address_space_write(&address_space_io, ent->phys_addr, > + MEMTXATTRS_UNSPECIFIED, ent->data, > + ent->len); > } else { > cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len); > } > diff --git a/dma-helpers.c b/dma-helpers.c > index d3871dc61ea..e8a26e81e16 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -28,8 +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, MEMTXATTRS_UNSPECIFIED, > - fillbuf, l, true); > + error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, > + fillbuf, l); > len -= l; > addr += l; > } > diff --git a/exec.c b/exec.c > index 8e9cc3b47cf..baefe582393 100644 > --- a/exec.c > +++ b/exec.c > @@ -3810,8 +3810,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, > attrs, buf, l); > } else { > - address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, > - attrs, buf, l, 0); > + address_space_read(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, > + l); > } > len -= l; > buf += l; > diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c > index 8fb83f5b078..31936061e21 100644 > --- a/hw/dma/xlnx-zdma.c > +++ b/hw/dma/xlnx-zdma.c > @@ -311,8 +311,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) > return false; > } > > - address_space_rw(s->dma_as, addr, s->attr, > - buf, sizeof(XlnxZDMADescr), false); > + address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr)); > return true; > } > > @@ -364,7 +363,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA *s, bool type, > } else { > addr = zdma_get_regaddr64(s, basereg); > addr += sizeof(s->dsc_dst); > - address_space_rw(s->dma_as, addr, s->attr, (void *) &next, 8, false); > + address_space_read(s->dma_as, addr, s->attr, (void *)&next, 8); > zdma_put_regaddr64(s, basereg, next); > } > return next; > @@ -416,8 +415,7 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) > } > } > > - address_space_rw(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen, > - true); > + address_space_write(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen); > if (burst_type == AXI_BURST_INCR) { > s->dsc_dst.addr += dlen; > } > @@ -493,8 +491,7 @@ static void zdma_process_descr(XlnxZDMA *s) > len = s->cfg.bus_width / 8; > } > } else { > - address_space_rw(s->dma_as, src_addr, s->attr, s->buf, len, > - false); > + address_space_read(s->dma_as, src_addr, s->attr, s->buf, len); > if (burst_type == AXI_BURST_INCR) { > src_addr += len; > } > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index a134d431ae3..f5f1c669e80 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -275,8 +275,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > /* Fill current entry */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff; > s->cam[index][1] = dp8393x_get(s, width, 1) >> 8; > s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff; > @@ -293,8 +293,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > } > > /* Read CAM enable */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); > > @@ -311,8 +311,8 @@ static void dp8393x_do_read_rra(dp8393xState *s) > /* Read memory */ > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > size = sizeof(uint16_t) * 4 * width; > - address_space_rw(&s->as, dp8393x_rrp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Update SONIC registers */ > s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > @@ -426,8 +426,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > size = sizeof(uint16_t) * 6 * width; > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); > - address_space_rw(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > tx_len = 0; > > /* Update registers */ > @@ -451,17 +451,19 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > if (tx_len + len > sizeof(s->tx_buffer)) { > len = sizeof(s->tx_buffer) - tx_len; > } > - address_space_rw(&s->as, dp8393x_tsa(s), > - MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); > + address_space_read(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, > + &s->tx_buffer[tx_len], len); > tx_len += len; > > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > size = sizeof(uint16_t) * 3 * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * i) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > @@ -494,18 +496,18 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > dp8393x_put(s, width, 0, > s->regs[SONIC_TCR] & 0x0fff); /* status */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > /* Read footer of packet */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + > - sizeof(uint16_t) * > - (4 + 3 * s->regs[SONIC_TFC]) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) > + * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1; > if (dp8393x_get(s, width, 0) & 0x1) { > /* EOL detected */ > @@ -767,8 +769,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Are we still in resource exhaustion? */ > size = sizeof(uint16_t) * 1 * width; > address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > if (dp8393x_get(s, width, 0) & 0x1) { > /* Still EOL ; stop reception */ > return -1; > @@ -787,11 +789,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Put packet into RBA */ > DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); > address = dp8393x_crba(s); > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)buf, rx_len); > address += rx_len; > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&checksum, 4); > rx_len += 4; > s->regs[SONIC_CRBA1] = address >> 16; > s->regs[SONIC_CRBA0] = address & 0xffff; > @@ -819,13 +821,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > size = sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, dp8393x_crda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Move to next descriptor */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > if (s->regs[SONIC_LLFA] & 0x1) { > /* EOL detected */ > @@ -838,8 +840,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > offset += sizeof(uint16_t); > } > s->data[0] = 0; > - address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, sizeof(uint16_t), 1); > + address_space_write(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, sizeof(uint16_t)); > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; > s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); > diff --git a/hw/net/i82596.c b/hw/net/i82596.c > index 3a0e1ec4c05..6a80c24af23 100644 > --- a/hw/net/i82596.c > +++ b/hw/net/i82596.c > @@ -148,8 +148,8 @@ static void i82596_transmit(I82596State *s, uint32_t addr) > > if (s->nic && len) { > assert(len <= sizeof(s->tx_buffer)); > - address_space_rw(&address_space_memory, tba, > - MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len, 0); > + address_space_read(&address_space_memory, tba, > + MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len); > DBG(PRINT_PKTHDR("Send", &s->tx_buffer)); > DBG(printf("Sending %d bytes\n", len)); > qemu_send_packet(qemu_get_queue(s->nic), s->tx_buffer, len); > @@ -172,8 +172,8 @@ static void set_individual_address(I82596State *s, uint32_t addr) > > nc = qemu_get_queue(s->nic); > m = s->conf.macaddr.a; > - address_space_rw(&address_space_memory, addr + 8, > - MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + 8, > + MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN); > qemu_format_nic_info_str(nc, m); > trace_i82596_new_mac(nc->info_str); > } > @@ -190,9 +190,8 @@ static void set_multicast_list(I82596State *s, uint32_t addr) > } > for (i = 0; i < mc_count; i++) { > uint8_t multicast_addr[ETH_ALEN]; > - address_space_rw(&address_space_memory, > - addr + i * ETH_ALEN, MEMTXATTRS_UNSPECIFIED, > - multicast_addr, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + i * ETH_ALEN, > + MEMTXATTRS_UNSPECIFIED, multicast_addr, ETH_ALEN); > DBG(printf("Add multicast entry " MAC_FMT "\n", > MAC_ARG(multicast_addr))); > unsigned mcast_idx = (net_crc32(multicast_addr, ETH_ALEN) & > @@ -260,8 +259,8 @@ static void command_loop(I82596State *s) > byte_cnt = MAX(byte_cnt, 4); > byte_cnt = MIN(byte_cnt, sizeof(s->config)); > /* copy byte_cnt max. */ > - address_space_rw(&address_space_memory, s->cmd_p + 8, > - MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt, 0); > + address_space_read(&address_space_memory, s->cmd_p + 8, > + MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt); > /* config byte according to page 35ff */ > s->config[2] &= 0x82; /* mask valid bits */ > s->config[2] |= 0x40; > @@ -640,14 +639,14 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) > } > rba = get_uint32(rbd + 8); > /* printf("rba is 0x%x\n", rba); */ > - address_space_rw(&address_space_memory, rba, > - MEMTXATTRS_UNSPECIFIED, (void *)buf, num, 1); > + address_space_write(&address_space_memory, rba, > + MEMTXATTRS_UNSPECIFIED, (void *)buf, num); > rba += num; > buf += num; > len -= num; > if (len == 0) { /* copy crc */ > - address_space_rw(&address_space_memory, rba - 4, > - MEMTXATTRS_UNSPECIFIED, crc_ptr, 4, 1); > + address_space_write(&address_space_memory, rba - 4, > + MEMTXATTRS_UNSPECIFIED, crc_ptr, 4); > } > > num |= 0x4000; /* set F BIT */ > diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c > index 427b3fbf701..52637a562d8 100644 > --- a/hw/net/lasi_i82596.c > +++ b/hw/net/lasi_i82596.c > @@ -55,8 +55,9 @@ static void lasi_82596_mem_write(void *opaque, hwaddr addr, > * Provided for SeaBIOS only. Write MAC of Network card to addr @val. > * Needed for the PDC_LAN_STATION_ID_READ PDC call. > */ > - address_space_rw(&address_space_memory, val, > - MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, ETH_ALEN, 1); > + address_space_write(&address_space_memory, val, > + MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, > + ETH_ALEN); > break; > } > } > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c > index 5989d723c50..f150deca340 100644 > --- a/hw/ppc/pnv_lpc.c > +++ b/hw/ppc/pnv_lpc.c > @@ -238,16 +238,16 @@ static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits and FW read caching here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, false); > + return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, true); > + return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > #define ECCB_CTL_READ PPC_BIT(15) > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 844caab4082..0e0fccd050e 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -874,18 +874,18 @@ static inline int ida_read_next_idaw(CcwDataStream *cds) > if (idaw_addr & 0x07 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt2, > - sizeof(idaw.fmt2), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt2, > + sizeof(idaw.fmt2)); > cds->cda = be64_to_cpu(idaw.fmt2); > } else { > idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw; > if (idaw_addr & 0x03 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt1, > - sizeof(idaw.fmt1), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt1, > + sizeof(idaw.fmt1)); > cds->cda = be64_to_cpu(idaw.fmt1); > if (cds->cda & 0x80000000) { > return -EINVAL; /* channel program check */ > diff --git a/qtest.c b/qtest.c > index 12432f99cf4..328d674bcc8 100644 > --- a/qtest.c > +++ b/qtest.c > @@ -429,23 +429,23 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][5] == 'b') { > uint8_t data = value; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > } else if (words[0][5] == 'w') { > uint16_t data = value; > tswap16s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > } else if (words[0][5] == 'l') { > uint32_t data = value; > tswap32s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > } else if (words[0][5] == 'q') { > uint64_t data = value; > tswap64s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 8, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 8); > } > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > @@ -463,22 +463,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][4] == 'b') { > uint8_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > value = data; > } else if (words[0][4] == 'w') { > uint16_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > value = tswap16(data); > } else if (words[0][4] == 'l') { > uint32_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > value = tswap32(data); > } else if (words[0][4] == 'q') { > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &value, 8, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&value, 8); > tswap64s(&value); > } > qtest_send_prefix(chr); > @@ -498,8 +498,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(len); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > enc = g_malloc(2 * len + 1); > for (i = 0; i < len; i++) { > @@ -524,8 +524,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(ret == 0); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > b64_data = g_base64_encode(data, len); > qtest_send_prefix(chr); > qtest_sendf(chr, "OK %s\n", b64_data); > @@ -559,8 +559,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > data[i] = 0; > } > } > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > g_free(data); > > qtest_send_prefix(chr); > @@ -582,8 +582,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > if (len) { > data = g_malloc(len); > memset(data, pattern, len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + data, len); > g_free(data); > } > > @@ -616,8 +616,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > out_len = MIN(out_len, len); > } > > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > diff --git a/target/i386/hvf/x86_mmu.c b/target/i386/hvf/x86_mmu.c > index d5a0efe7188..ff016fc0145 100644 > --- a/target/i386/hvf/x86_mmu.c > +++ b/target/i386/hvf/x86_mmu.c > @@ -88,8 +88,8 @@ static bool get_pt_entry(struct CPUState *cpu, struct gpt_translation *pt, > } > > index = gpt_entry(pt->gva, level, pae); > - address_space_rw(&address_space_memory, gpa + index * pte_size(pae), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae), 0); > + address_space_read(&address_space_memory, gpa + index * pte_size(pae), > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae)); > > pt->pte[level - 1] = pte; > > @@ -238,8 +238,8 @@ void vmx_write_mem(struct CPUState *cpu, target_ulong gva, void *data, int bytes > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } else { > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 1); > + address_space_write(&address_space_memory, gpa, > + MEMTXATTRS_UNSPECIFIED, data, copy); > } > > bytes -= copy; > @@ -259,8 +259,8 @@ void vmx_read_mem(struct CPUState *cpu, void *data, target_ulong gva, int bytes) > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 0); > + address_space_read(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > + data, copy); > > bytes -= copy; > gva += copy; > diff --git a/scripts/coccinelle/as_rw_const.cocci b/scripts/coccinelle/as_rw_const.cocci > new file mode 100644 > index 00000000000..30da707701b > --- /dev/null > +++ b/scripts/coccinelle/as_rw_const.cocci > @@ -0,0 +1,30 @@ > +// Avoid uses of address_space_rw() with a constant is_write argument. > +// Usage: > +// spatch --sp-file as-rw-const.spatch --dir . --in-place > + > +@@ > +expression E1, E2, E3, E4, E5; > +symbol false; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, false) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 0) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +symbol true; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, true) > ++ address_space_write(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 1) > ++ address_space_write(E1, E2, E3, E4, E5) > -- > 2.20.1 > >
On Tue, Feb 18, 2020 at 11:24:57AM +0000, Peter Maydell wrote: > The address_space_rw() function allows either reads or writes > depending on the is_write argument passed to it; this is useful > when the direction of the access is determined programmatically > (as for instance when handling the KVM_EXIT_MMIO exit reason). > Under the hood it just calls either address_space_write() or > address_space_read_full(). > > We also use it a lot with a constant is_write argument, though, > which has two issues: > * when reading "address_space_rw(..., 1)" this is less > immediately clear to the reader as being a write than > "address_space_write(...)" > * calling address_space_rw() bypasses the optimization > in address_space_read() that fast-paths reads of a > fixed length > > This commit was produced with the included Coccinelle script > scripts/coccinelle/as-rw-const.patch. > > Two lines in hw/net/dp8393x.c that Coccinelle produced that > were over 80 characters were re-wrapped by hand. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> ppc parts Acked-by: David Gibson <david@gibson.dropbear.id.au> > --- > I could break this down into separate patches by submaintainer, > but the patch is not that large and I would argue that it's > better for the project if we can try to avoid introducing too > much friction into the process of doing 'safe' tree-wide > minor refactorings. > > v1->v2: put the coccinelle script in scripts/coccinelle rather > than just in the commit message. > --- > accel/kvm/kvm-all.c | 6 +-- > dma-helpers.c | 4 +- > exec.c | 4 +- > hw/dma/xlnx-zdma.c | 11 ++--- > hw/net/dp8393x.c | 68 ++++++++++++++-------------- > hw/net/i82596.c | 25 +++++----- > hw/net/lasi_i82596.c | 5 +- > hw/ppc/pnv_lpc.c | 8 ++-- > hw/s390x/css.c | 12 ++--- > qtest.c | 52 ++++++++++----------- > target/i386/hvf/x86_mmu.c | 12 ++--- > scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ > 12 files changed, 133 insertions(+), 104 deletions(-) > create mode 100644 scripts/coccinelle/as_rw_const.cocci > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index c111312dfdd..0cfe6fd8ded 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2178,9 +2178,9 @@ void kvm_flush_coalesced_mmio_buffer(void) > ent = &ring->coalesced_mmio[ring->first]; > > if (ent->pio == 1) { > - address_space_rw(&address_space_io, ent->phys_addr, > - MEMTXATTRS_UNSPECIFIED, ent->data, > - ent->len, true); > + address_space_write(&address_space_io, ent->phys_addr, > + MEMTXATTRS_UNSPECIFIED, ent->data, > + ent->len); > } else { > cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len); > } > diff --git a/dma-helpers.c b/dma-helpers.c > index d3871dc61ea..e8a26e81e16 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -28,8 +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, MEMTXATTRS_UNSPECIFIED, > - fillbuf, l, true); > + error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, > + fillbuf, l); > len -= l; > addr += l; > } > diff --git a/exec.c b/exec.c > index 8e9cc3b47cf..baefe582393 100644 > --- a/exec.c > +++ b/exec.c > @@ -3810,8 +3810,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, > attrs, buf, l); > } else { > - address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, > - attrs, buf, l, 0); > + address_space_read(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, > + l); > } > len -= l; > buf += l; > diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c > index 8fb83f5b078..31936061e21 100644 > --- a/hw/dma/xlnx-zdma.c > +++ b/hw/dma/xlnx-zdma.c > @@ -311,8 +311,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) > return false; > } > > - address_space_rw(s->dma_as, addr, s->attr, > - buf, sizeof(XlnxZDMADescr), false); > + address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr)); > return true; > } > > @@ -364,7 +363,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA *s, bool type, > } else { > addr = zdma_get_regaddr64(s, basereg); > addr += sizeof(s->dsc_dst); > - address_space_rw(s->dma_as, addr, s->attr, (void *) &next, 8, false); > + address_space_read(s->dma_as, addr, s->attr, (void *)&next, 8); > zdma_put_regaddr64(s, basereg, next); > } > return next; > @@ -416,8 +415,7 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) > } > } > > - address_space_rw(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen, > - true); > + address_space_write(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen); > if (burst_type == AXI_BURST_INCR) { > s->dsc_dst.addr += dlen; > } > @@ -493,8 +491,7 @@ static void zdma_process_descr(XlnxZDMA *s) > len = s->cfg.bus_width / 8; > } > } else { > - address_space_rw(s->dma_as, src_addr, s->attr, s->buf, len, > - false); > + address_space_read(s->dma_as, src_addr, s->attr, s->buf, len); > if (burst_type == AXI_BURST_INCR) { > src_addr += len; > } > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index a134d431ae3..f5f1c669e80 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -275,8 +275,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > /* Fill current entry */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff; > s->cam[index][1] = dp8393x_get(s, width, 1) >> 8; > s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff; > @@ -293,8 +293,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) > } > > /* Read CAM enable */ > - address_space_rw(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); > > @@ -311,8 +311,8 @@ static void dp8393x_do_read_rra(dp8393xState *s) > /* Read memory */ > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > size = sizeof(uint16_t) * 4 * width; > - address_space_rw(&s->as, dp8393x_rrp(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Update SONIC registers */ > s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > @@ -426,8 +426,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > size = sizeof(uint16_t) * 6 * width; > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); > - address_space_rw(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > tx_len = 0; > > /* Update registers */ > @@ -451,17 +451,19 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > if (tx_len + len > sizeof(s->tx_buffer)) { > len = sizeof(s->tx_buffer) - tx_len; > } > - address_space_rw(&s->as, dp8393x_tsa(s), > - MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); > + address_space_read(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, > + &s->tx_buffer[tx_len], len); > tx_len += len; > > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > size = sizeof(uint16_t) * 3 * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * i) * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > @@ -494,18 +496,18 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > dp8393x_put(s, width, 0, > s->regs[SONIC_TCR] & 0x0fff); /* status */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > /* Read footer of packet */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, > - dp8393x_ttda(s) + > - sizeof(uint16_t) * > - (4 + 3 * s->regs[SONIC_TFC]) * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, > + dp8393x_ttda(s) > + + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) > + * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, > + size); > s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1; > if (dp8393x_get(s, width, 0) & 0x1) { > /* EOL detected */ > @@ -767,8 +769,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Are we still in resource exhaustion? */ > size = sizeof(uint16_t) * 1 * width; > address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > if (dp8393x_get(s, width, 0) & 0x1) { > /* Still EOL ; stop reception */ > return -1; > @@ -787,11 +789,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Put packet into RBA */ > DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); > address = dp8393x_crba(s); > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)buf, rx_len); > address += rx_len; > - address_space_rw(&s->as, address, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1); > + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&checksum, 4); > rx_len += 4; > s->regs[SONIC_CRBA1] = address >> 16; > s->regs[SONIC_CRBA0] = address & 0xffff; > @@ -819,13 +821,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > size = sizeof(uint16_t) * 5 * width; > - address_space_rw(&s->as, dp8393x_crda(s), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); > + address_space_write(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, size); > > /* Move to next descriptor */ > size = sizeof(uint16_t) * width; > - address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); > + address_space_read(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); > s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > if (s->regs[SONIC_LLFA] & 0x1) { > /* EOL detected */ > @@ -838,8 +840,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > offset += sizeof(uint16_t); > } > s->data[0] = 0; > - address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, sizeof(uint16_t), 1); > + address_space_write(&s->as, offset, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)s->data, sizeof(uint16_t)); > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; > s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); > diff --git a/hw/net/i82596.c b/hw/net/i82596.c > index 3a0e1ec4c05..6a80c24af23 100644 > --- a/hw/net/i82596.c > +++ b/hw/net/i82596.c > @@ -148,8 +148,8 @@ static void i82596_transmit(I82596State *s, uint32_t addr) > > if (s->nic && len) { > assert(len <= sizeof(s->tx_buffer)); > - address_space_rw(&address_space_memory, tba, > - MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len, 0); > + address_space_read(&address_space_memory, tba, > + MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len); > DBG(PRINT_PKTHDR("Send", &s->tx_buffer)); > DBG(printf("Sending %d bytes\n", len)); > qemu_send_packet(qemu_get_queue(s->nic), s->tx_buffer, len); > @@ -172,8 +172,8 @@ static void set_individual_address(I82596State *s, uint32_t addr) > > nc = qemu_get_queue(s->nic); > m = s->conf.macaddr.a; > - address_space_rw(&address_space_memory, addr + 8, > - MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + 8, > + MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN); > qemu_format_nic_info_str(nc, m); > trace_i82596_new_mac(nc->info_str); > } > @@ -190,9 +190,8 @@ static void set_multicast_list(I82596State *s, uint32_t addr) > } > for (i = 0; i < mc_count; i++) { > uint8_t multicast_addr[ETH_ALEN]; > - address_space_rw(&address_space_memory, > - addr + i * ETH_ALEN, MEMTXATTRS_UNSPECIFIED, > - multicast_addr, ETH_ALEN, 0); > + address_space_read(&address_space_memory, addr + i * ETH_ALEN, > + MEMTXATTRS_UNSPECIFIED, multicast_addr, ETH_ALEN); > DBG(printf("Add multicast entry " MAC_FMT "\n", > MAC_ARG(multicast_addr))); > unsigned mcast_idx = (net_crc32(multicast_addr, ETH_ALEN) & > @@ -260,8 +259,8 @@ static void command_loop(I82596State *s) > byte_cnt = MAX(byte_cnt, 4); > byte_cnt = MIN(byte_cnt, sizeof(s->config)); > /* copy byte_cnt max. */ > - address_space_rw(&address_space_memory, s->cmd_p + 8, > - MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt, 0); > + address_space_read(&address_space_memory, s->cmd_p + 8, > + MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt); > /* config byte according to page 35ff */ > s->config[2] &= 0x82; /* mask valid bits */ > s->config[2] |= 0x40; > @@ -640,14 +639,14 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) > } > rba = get_uint32(rbd + 8); > /* printf("rba is 0x%x\n", rba); */ > - address_space_rw(&address_space_memory, rba, > - MEMTXATTRS_UNSPECIFIED, (void *)buf, num, 1); > + address_space_write(&address_space_memory, rba, > + MEMTXATTRS_UNSPECIFIED, (void *)buf, num); > rba += num; > buf += num; > len -= num; > if (len == 0) { /* copy crc */ > - address_space_rw(&address_space_memory, rba - 4, > - MEMTXATTRS_UNSPECIFIED, crc_ptr, 4, 1); > + address_space_write(&address_space_memory, rba - 4, > + MEMTXATTRS_UNSPECIFIED, crc_ptr, 4); > } > > num |= 0x4000; /* set F BIT */ > diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c > index 427b3fbf701..52637a562d8 100644 > --- a/hw/net/lasi_i82596.c > +++ b/hw/net/lasi_i82596.c > @@ -55,8 +55,9 @@ static void lasi_82596_mem_write(void *opaque, hwaddr addr, > * Provided for SeaBIOS only. Write MAC of Network card to addr @val. > * Needed for the PDC_LAN_STATION_ID_READ PDC call. > */ > - address_space_rw(&address_space_memory, val, > - MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, ETH_ALEN, 1); > + address_space_write(&address_space_memory, val, > + MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, > + ETH_ALEN); > break; > } > } > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c > index 5989d723c50..f150deca340 100644 > --- a/hw/ppc/pnv_lpc.c > +++ b/hw/ppc/pnv_lpc.c > @@ -238,16 +238,16 @@ static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits and FW read caching here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, false); > + return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data, > int sz) > { > /* XXX Handle access size limits here */ > - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > - data, sz, true); > + return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, > + data, sz); > } > > #define ECCB_CTL_READ PPC_BIT(15) > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 844caab4082..0e0fccd050e 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -874,18 +874,18 @@ static inline int ida_read_next_idaw(CcwDataStream *cds) > if (idaw_addr & 0x07 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt2, > - sizeof(idaw.fmt2), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt2, > + sizeof(idaw.fmt2)); > cds->cda = be64_to_cpu(idaw.fmt2); > } else { > idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw; > if (idaw_addr & 0x03 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { > return -EINVAL; /* channel program check */ > } > - ret = address_space_rw(&address_space_memory, idaw_addr, > - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt1, > - sizeof(idaw.fmt1), false); > + ret = address_space_read(&address_space_memory, idaw_addr, > + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt1, > + sizeof(idaw.fmt1)); > cds->cda = be64_to_cpu(idaw.fmt1); > if (cds->cda & 0x80000000) { > return -EINVAL; /* channel program check */ > diff --git a/qtest.c b/qtest.c > index 12432f99cf4..328d674bcc8 100644 > --- a/qtest.c > +++ b/qtest.c > @@ -429,23 +429,23 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][5] == 'b') { > uint8_t data = value; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > } else if (words[0][5] == 'w') { > uint16_t data = value; > tswap16s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > } else if (words[0][5] == 'l') { > uint32_t data = value; > tswap32s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > } else if (words[0][5] == 'q') { > uint64_t data = value; > tswap64s(&data); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 8, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 8); > } > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > @@ -463,22 +463,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > > if (words[0][4] == 'b') { > uint8_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - &data, 1, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + &data, 1); > value = data; > } else if (words[0][4] == 'w') { > uint16_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 2, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 2); > value = tswap16(data); > } else if (words[0][4] == 'l') { > uint32_t data; > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &data, 4, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&data, 4); > value = tswap32(data); > } else if (words[0][4] == 'q') { > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *) &value, 8, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + (uint8_t *)&value, 8); > tswap64s(&value); > } > qtest_send_prefix(chr); > @@ -498,8 +498,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(len); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > enc = g_malloc(2 * len + 1); > for (i = 0; i < len; i++) { > @@ -524,8 +524,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > g_assert(ret == 0); > > data = g_malloc(len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, false); > + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > b64_data = g_base64_encode(data, len); > qtest_send_prefix(chr); > qtest_sendf(chr, "OK %s\n", b64_data); > @@ -559,8 +559,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > data[i] = 0; > } > } > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > g_free(data); > > qtest_send_prefix(chr); > @@ -582,8 +582,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > if (len) { > data = g_malloc(len); > memset(data, pattern, len); > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > + data, len); > g_free(data); > } > > @@ -616,8 +616,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > out_len = MIN(out_len, len); > } > > - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > - data, len, true); > + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > diff --git a/target/i386/hvf/x86_mmu.c b/target/i386/hvf/x86_mmu.c > index d5a0efe7188..ff016fc0145 100644 > --- a/target/i386/hvf/x86_mmu.c > +++ b/target/i386/hvf/x86_mmu.c > @@ -88,8 +88,8 @@ static bool get_pt_entry(struct CPUState *cpu, struct gpt_translation *pt, > } > > index = gpt_entry(pt->gva, level, pae); > - address_space_rw(&address_space_memory, gpa + index * pte_size(pae), > - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae), 0); > + address_space_read(&address_space_memory, gpa + index * pte_size(pae), > + MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae)); > > pt->pte[level - 1] = pte; > > @@ -238,8 +238,8 @@ void vmx_write_mem(struct CPUState *cpu, target_ulong gva, void *data, int bytes > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } else { > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 1); > + address_space_write(&address_space_memory, gpa, > + MEMTXATTRS_UNSPECIFIED, data, copy); > } > > bytes -= copy; > @@ -259,8 +259,8 @@ void vmx_read_mem(struct CPUState *cpu, void *data, target_ulong gva, int bytes) > if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { > VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); > } > - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > - data, copy, 0); > + address_space_read(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, > + data, copy); > > bytes -= copy; > gva += copy; > diff --git a/scripts/coccinelle/as_rw_const.cocci b/scripts/coccinelle/as_rw_const.cocci > new file mode 100644 > index 00000000000..30da707701b > --- /dev/null > +++ b/scripts/coccinelle/as_rw_const.cocci > @@ -0,0 +1,30 @@ > +// Avoid uses of address_space_rw() with a constant is_write argument. > +// Usage: > +// spatch --sp-file as-rw-const.spatch --dir . --in-place > + > +@@ > +expression E1, E2, E3, E4, E5; > +symbol false; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, false) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 0) > ++ address_space_read(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +symbol true; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, true) > ++ address_space_write(E1, E2, E3, E4, E5) > +@@ > +expression E1, E2, E3, E4, E5; > +@@ > + > +- address_space_rw(E1, E2, E3, E4, E5, 1) > ++ address_space_write(E1, E2, E3, E4, E5)
On 2/18/20 1:56 PM, Philippe Mathieu-Daudé wrote: > On 2/18/20 12:24 PM, Peter Maydell wrote: >> The address_space_rw() function allows either reads or writes >> depending on the is_write argument passed to it; this is useful >> when the direction of the access is determined programmatically >> (as for instance when handling the KVM_EXIT_MMIO exit reason). >> Under the hood it just calls either address_space_write() or >> address_space_read_full(). >> >> We also use it a lot with a constant is_write argument, though, >> which has two issues: >> * when reading "address_space_rw(..., 1)" this is less >> immediately clear to the reader as being a write than >> "address_space_write(...)" >> * calling address_space_rw() bypasses the optimization >> in address_space_read() that fast-paths reads of a >> fixed length >> >> This commit was produced with the included Coccinelle script >> scripts/coccinelle/as-rw-const.patch. Script is "scripts/coccinelle/as_rw_const.cocci". I plan to respin this patch (fixed) in a larger series. >> >> Two lines in hw/net/dp8393x.c that Coccinelle produced that >> were over 80 characters were re-wrapped by hand. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> I could break this down into separate patches by submaintainer, >> but the patch is not that large and I would argue that it's >> better for the project if we can try to avoid introducing too >> much friction into the process of doing 'safe' tree-wide >> minor refactorings. >> >> v1->v2: put the coccinelle script in scripts/coccinelle rather >> than just in the commit message. >> --- >> accel/kvm/kvm-all.c | 6 +-- >> dma-helpers.c | 4 +- >> exec.c | 4 +- >> hw/dma/xlnx-zdma.c | 11 ++--- >> hw/net/dp8393x.c | 68 ++++++++++++++-------------- >> hw/net/i82596.c | 25 +++++----- >> hw/net/lasi_i82596.c | 5 +- >> hw/ppc/pnv_lpc.c | 8 ++-- >> hw/s390x/css.c | 12 ++--- >> qtest.c | 52 ++++++++++----------- >> target/i386/hvf/x86_mmu.c | 12 ++--- >> scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ >> 12 files changed, 133 insertions(+), 104 deletions(-) >> create mode 100644 scripts/coccinelle/as_rw_const.cocci
On 18/02/20 14:13, Laurent Vivier wrote: > There is one in target/i386/hvf/vmx.h: macvm_set_cr0() you didn't change. > > You must update the script name in the script comment (as suggested by > Philippe) and in the commit message. > > Anyway: > > Reviewed-by: Laurent Vivier <lvivier@redhat.com> Thanks, I squashed the change to vmx.h. Paolo
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index c111312dfdd..0cfe6fd8ded 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -2178,9 +2178,9 @@ void kvm_flush_coalesced_mmio_buffer(void) ent = &ring->coalesced_mmio[ring->first]; if (ent->pio == 1) { - address_space_rw(&address_space_io, ent->phys_addr, - MEMTXATTRS_UNSPECIFIED, ent->data, - ent->len, true); + address_space_write(&address_space_io, ent->phys_addr, + MEMTXATTRS_UNSPECIFIED, ent->data, + ent->len); } else { cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len); } diff --git a/dma-helpers.c b/dma-helpers.c index d3871dc61ea..e8a26e81e16 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -28,8 +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, MEMTXATTRS_UNSPECIFIED, - fillbuf, l, true); + error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, + fillbuf, l); len -= l; addr += l; } diff --git a/exec.c b/exec.c index 8e9cc3b47cf..baefe582393 100644 --- a/exec.c +++ b/exec.c @@ -3810,8 +3810,8 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, l); } else { - address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, - attrs, buf, l, 0); + address_space_read(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, + l); } len -= l; buf += l; diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c index 8fb83f5b078..31936061e21 100644 --- a/hw/dma/xlnx-zdma.c +++ b/hw/dma/xlnx-zdma.c @@ -311,8 +311,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) return false; } - address_space_rw(s->dma_as, addr, s->attr, - buf, sizeof(XlnxZDMADescr), false); + address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr)); return true; } @@ -364,7 +363,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA *s, bool type, } else { addr = zdma_get_regaddr64(s, basereg); addr += sizeof(s->dsc_dst); - address_space_rw(s->dma_as, addr, s->attr, (void *) &next, 8, false); + address_space_read(s->dma_as, addr, s->attr, (void *)&next, 8); zdma_put_regaddr64(s, basereg, next); } return next; @@ -416,8 +415,7 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) } } - address_space_rw(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen, - true); + address_space_write(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen); if (burst_type == AXI_BURST_INCR) { s->dsc_dst.addr += dlen; } @@ -493,8 +491,7 @@ static void zdma_process_descr(XlnxZDMA *s) len = s->cfg.bus_width / 8; } } else { - address_space_rw(s->dma_as, src_addr, s->attr, s->buf, len, - false); + address_space_read(s->dma_as, src_addr, s->attr, s->buf, len); if (burst_type == AXI_BURST_INCR) { src_addr += len; } diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index a134d431ae3..f5f1c669e80 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -275,8 +275,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) while (s->regs[SONIC_CDC] & 0x1f) { /* Fill current entry */ - address_space_rw(&s->as, dp8393x_cdp(s), - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, + (uint8_t *)s->data, size); s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff; s->cam[index][1] = dp8393x_get(s, width, 1) >> 8; s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff; @@ -293,8 +293,8 @@ static void dp8393x_do_load_cam(dp8393xState *s) } /* Read CAM enable */ - address_space_rw(&s->as, dp8393x_cdp(s), - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); + address_space_read(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, + (uint8_t *)s->data, size); s->regs[SONIC_CE] = dp8393x_get(s, width, 0); DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); @@ -311,8 +311,8 @@ static void dp8393x_do_read_rra(dp8393xState *s) /* Read memory */ width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; size = sizeof(uint16_t) * 4 * width; - address_space_rw(&s->as, dp8393x_rrp(s), - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); + address_space_read(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, + (uint8_t *)s->data, size); /* Update SONIC registers */ s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); @@ -426,8 +426,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) size = sizeof(uint16_t) * 6 * width; s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); - address_space_rw(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); + address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); tx_len = 0; /* Update registers */ @@ -451,17 +451,19 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) if (tx_len + len > sizeof(s->tx_buffer)) { len = sizeof(s->tx_buffer) - tx_len; } - address_space_rw(&s->as, dp8393x_tsa(s), - MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); + address_space_read(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, + &s->tx_buffer[tx_len], len); tx_len += len; i++; if (i != s->regs[SONIC_TFC]) { /* Read next fragment details */ size = sizeof(uint16_t) * 3 * width; - address_space_rw(&s->as, - dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); + address_space_read(&s->as, + dp8393x_ttda(s) + + sizeof(uint16_t) * (4 + 3 * i) * width, + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, + size); s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); @@ -494,18 +496,18 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) dp8393x_put(s, width, 0, s->regs[SONIC_TCR] & 0x0fff); /* status */ size = sizeof(uint16_t) * width; - address_space_rw(&s->as, - dp8393x_ttda(s), - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); + address_space_write(&s->as, dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, + (uint8_t *)s->data, size); if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { /* Read footer of packet */ size = sizeof(uint16_t) * width; - address_space_rw(&s->as, - dp8393x_ttda(s) + - sizeof(uint16_t) * - (4 + 3 * s->regs[SONIC_TFC]) * width, - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); + address_space_read(&s->as, + dp8393x_ttda(s) + + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) + * width, + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, + size); s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1; if (dp8393x_get(s, width, 0) & 0x1) { /* EOL detected */ @@ -767,8 +769,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* Are we still in resource exhaustion? */ size = sizeof(uint16_t) * 1 * width; address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; - address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, - (uint8_t *)s->data, size, 0); + address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)s->data, size); if (dp8393x_get(s, width, 0) & 0x1) { /* Still EOL ; stop reception */ return -1; @@ -787,11 +789,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* Put packet into RBA */ DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); address = dp8393x_crba(s); - address_space_rw(&s->as, address, - MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)buf, rx_len); address += rx_len; - address_space_rw(&s->as, address, - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1); + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)&checksum, 4); rx_len += 4; s->regs[SONIC_CRBA1] = address >> 16; s->regs[SONIC_CRBA0] = address & 0xffff; @@ -819,13 +821,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ size = sizeof(uint16_t) * 5 * width; - address_space_rw(&s->as, dp8393x_crda(s), - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1); + address_space_write(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, + (uint8_t *)s->data, size); /* Move to next descriptor */ size = sizeof(uint16_t) * width; - address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, - MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0); + address_space_read(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, + MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size); s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); if (s->regs[SONIC_LLFA] & 0x1) { /* EOL detected */ @@ -838,8 +840,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, offset += sizeof(uint16_t); } s->data[0] = 0; - address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED, - (uint8_t *)s->data, sizeof(uint16_t), 1); + address_space_write(&s->as, offset, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)s->data, sizeof(uint16_t)); s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); diff --git a/hw/net/i82596.c b/hw/net/i82596.c index 3a0e1ec4c05..6a80c24af23 100644 --- a/hw/net/i82596.c +++ b/hw/net/i82596.c @@ -148,8 +148,8 @@ static void i82596_transmit(I82596State *s, uint32_t addr) if (s->nic && len) { assert(len <= sizeof(s->tx_buffer)); - address_space_rw(&address_space_memory, tba, - MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len, 0); + address_space_read(&address_space_memory, tba, + MEMTXATTRS_UNSPECIFIED, s->tx_buffer, len); DBG(PRINT_PKTHDR("Send", &s->tx_buffer)); DBG(printf("Sending %d bytes\n", len)); qemu_send_packet(qemu_get_queue(s->nic), s->tx_buffer, len); @@ -172,8 +172,8 @@ static void set_individual_address(I82596State *s, uint32_t addr) nc = qemu_get_queue(s->nic); m = s->conf.macaddr.a; - address_space_rw(&address_space_memory, addr + 8, - MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN, 0); + address_space_read(&address_space_memory, addr + 8, + MEMTXATTRS_UNSPECIFIED, m, ETH_ALEN); qemu_format_nic_info_str(nc, m); trace_i82596_new_mac(nc->info_str); } @@ -190,9 +190,8 @@ static void set_multicast_list(I82596State *s, uint32_t addr) } for (i = 0; i < mc_count; i++) { uint8_t multicast_addr[ETH_ALEN]; - address_space_rw(&address_space_memory, - addr + i * ETH_ALEN, MEMTXATTRS_UNSPECIFIED, - multicast_addr, ETH_ALEN, 0); + address_space_read(&address_space_memory, addr + i * ETH_ALEN, + MEMTXATTRS_UNSPECIFIED, multicast_addr, ETH_ALEN); DBG(printf("Add multicast entry " MAC_FMT "\n", MAC_ARG(multicast_addr))); unsigned mcast_idx = (net_crc32(multicast_addr, ETH_ALEN) & @@ -260,8 +259,8 @@ static void command_loop(I82596State *s) byte_cnt = MAX(byte_cnt, 4); byte_cnt = MIN(byte_cnt, sizeof(s->config)); /* copy byte_cnt max. */ - address_space_rw(&address_space_memory, s->cmd_p + 8, - MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt, 0); + address_space_read(&address_space_memory, s->cmd_p + 8, + MEMTXATTRS_UNSPECIFIED, s->config, byte_cnt); /* config byte according to page 35ff */ s->config[2] &= 0x82; /* mask valid bits */ s->config[2] |= 0x40; @@ -640,14 +639,14 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) } rba = get_uint32(rbd + 8); /* printf("rba is 0x%x\n", rba); */ - address_space_rw(&address_space_memory, rba, - MEMTXATTRS_UNSPECIFIED, (void *)buf, num, 1); + address_space_write(&address_space_memory, rba, + MEMTXATTRS_UNSPECIFIED, (void *)buf, num); rba += num; buf += num; len -= num; if (len == 0) { /* copy crc */ - address_space_rw(&address_space_memory, rba - 4, - MEMTXATTRS_UNSPECIFIED, crc_ptr, 4, 1); + address_space_write(&address_space_memory, rba - 4, + MEMTXATTRS_UNSPECIFIED, crc_ptr, 4); } num |= 0x4000; /* set F BIT */ diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c index 427b3fbf701..52637a562d8 100644 --- a/hw/net/lasi_i82596.c +++ b/hw/net/lasi_i82596.c @@ -55,8 +55,9 @@ static void lasi_82596_mem_write(void *opaque, hwaddr addr, * Provided for SeaBIOS only. Write MAC of Network card to addr @val. * Needed for the PDC_LAN_STATION_ID_READ PDC call. */ - address_space_rw(&address_space_memory, val, - MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, ETH_ALEN, 1); + address_space_write(&address_space_memory, val, + MEMTXATTRS_UNSPECIFIED, d->state.conf.macaddr.a, + ETH_ALEN); break; } } diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c index 5989d723c50..f150deca340 100644 --- a/hw/ppc/pnv_lpc.c +++ b/hw/ppc/pnv_lpc.c @@ -238,16 +238,16 @@ static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data, int sz) { /* XXX Handle access size limits and FW read caching here */ - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, - data, sz, false); + return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, + data, sz); } static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data, int sz) { /* XXX Handle access size limits here */ - return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, - data, sz, true); + return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED, + data, sz); } #define ECCB_CTL_READ PPC_BIT(15) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 844caab4082..0e0fccd050e 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -874,18 +874,18 @@ static inline int ida_read_next_idaw(CcwDataStream *cds) if (idaw_addr & 0x07 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { return -EINVAL; /* channel program check */ } - ret = address_space_rw(&address_space_memory, idaw_addr, - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt2, - sizeof(idaw.fmt2), false); + ret = address_space_read(&address_space_memory, idaw_addr, + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt2, + sizeof(idaw.fmt2)); cds->cda = be64_to_cpu(idaw.fmt2); } else { idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw; if (idaw_addr & 0x03 || !cds_ccw_addrs_ok(idaw_addr, 0, ccw_fmt1)) { return -EINVAL; /* channel program check */ } - ret = address_space_rw(&address_space_memory, idaw_addr, - MEMTXATTRS_UNSPECIFIED, (void *) &idaw.fmt1, - sizeof(idaw.fmt1), false); + ret = address_space_read(&address_space_memory, idaw_addr, + MEMTXATTRS_UNSPECIFIED, (void *)&idaw.fmt1, + sizeof(idaw.fmt1)); cds->cda = be64_to_cpu(idaw.fmt1); if (cds->cda & 0x80000000) { return -EINVAL; /* channel program check */ diff --git a/qtest.c b/qtest.c index 12432f99cf4..328d674bcc8 100644 --- a/qtest.c +++ b/qtest.c @@ -429,23 +429,23 @@ static void qtest_process_command(CharBackend *chr, gchar **words) if (words[0][5] == 'b') { uint8_t data = value; - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - &data, 1, true); + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + &data, 1); } else if (words[0][5] == 'w') { uint16_t data = value; tswap16s(&data); - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - (uint8_t *) &data, 2, true); + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)&data, 2); } else if (words[0][5] == 'l') { uint32_t data = value; tswap32s(&data); - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - (uint8_t *) &data, 4, true); + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)&data, 4); } else if (words[0][5] == 'q') { uint64_t data = value; tswap64s(&data); - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - (uint8_t *) &data, 8, true); + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)&data, 8); } qtest_send_prefix(chr); qtest_send(chr, "OK\n"); @@ -463,22 +463,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words) if (words[0][4] == 'b') { uint8_t data; - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - &data, 1, false); + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + &data, 1); value = data; } else if (words[0][4] == 'w') { uint16_t data; - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - (uint8_t *) &data, 2, false); + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)&data, 2); value = tswap16(data); } else if (words[0][4] == 'l') { uint32_t data; - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - (uint8_t *) &data, 4, false); + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)&data, 4); value = tswap32(data); } else if (words[0][4] == 'q') { - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - (uint8_t *) &value, 8, false); + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)&value, 8); tswap64s(&value); } qtest_send_prefix(chr); @@ -498,8 +498,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) g_assert(len); data = g_malloc(len); - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - data, len, false); + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, + len); enc = g_malloc(2 * len + 1); for (i = 0; i < len; i++) { @@ -524,8 +524,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) g_assert(ret == 0); data = g_malloc(len); - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - data, len, false); + address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, + len); b64_data = g_base64_encode(data, len); qtest_send_prefix(chr); qtest_sendf(chr, "OK %s\n", b64_data); @@ -559,8 +559,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) data[i] = 0; } } - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - data, len, true); + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, + len); g_free(data); qtest_send_prefix(chr); @@ -582,8 +582,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) if (len) { data = g_malloc(len); memset(data, pattern, len); - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - data, len, true); + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, + data, len); g_free(data); } @@ -616,8 +616,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) out_len = MIN(out_len, len); } - address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, - data, len, true); + address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data, + len); qtest_send_prefix(chr); qtest_send(chr, "OK\n"); diff --git a/target/i386/hvf/x86_mmu.c b/target/i386/hvf/x86_mmu.c index d5a0efe7188..ff016fc0145 100644 --- a/target/i386/hvf/x86_mmu.c +++ b/target/i386/hvf/x86_mmu.c @@ -88,8 +88,8 @@ static bool get_pt_entry(struct CPUState *cpu, struct gpt_translation *pt, } index = gpt_entry(pt->gva, level, pae); - address_space_rw(&address_space_memory, gpa + index * pte_size(pae), - MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae), 0); + address_space_read(&address_space_memory, gpa + index * pte_size(pae), + MEMTXATTRS_UNSPECIFIED, (uint8_t *)&pte, pte_size(pae)); pt->pte[level - 1] = pte; @@ -238,8 +238,8 @@ void vmx_write_mem(struct CPUState *cpu, target_ulong gva, void *data, int bytes if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); } else { - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, - data, copy, 1); + address_space_write(&address_space_memory, gpa, + MEMTXATTRS_UNSPECIFIED, data, copy); } bytes -= copy; @@ -259,8 +259,8 @@ void vmx_read_mem(struct CPUState *cpu, void *data, target_ulong gva, int bytes) if (!mmu_gva_to_gpa(cpu, gva, &gpa)) { VM_PANIC_EX("%s: mmu_gva_to_gpa %llx failed\n", __func__, gva); } - address_space_rw(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, - data, copy, 0); + address_space_read(&address_space_memory, gpa, MEMTXATTRS_UNSPECIFIED, + data, copy); bytes -= copy; gva += copy; diff --git a/scripts/coccinelle/as_rw_const.cocci b/scripts/coccinelle/as_rw_const.cocci new file mode 100644 index 00000000000..30da707701b --- /dev/null +++ b/scripts/coccinelle/as_rw_const.cocci @@ -0,0 +1,30 @@ +// Avoid uses of address_space_rw() with a constant is_write argument. +// Usage: +// spatch --sp-file as-rw-const.spatch --dir . --in-place + +@@ +expression E1, E2, E3, E4, E5; +symbol false; +@@ + +- address_space_rw(E1, E2, E3, E4, E5, false) ++ address_space_read(E1, E2, E3, E4, E5) +@@ +expression E1, E2, E3, E4, E5; +@@ + +- address_space_rw(E1, E2, E3, E4, E5, 0) ++ address_space_read(E1, E2, E3, E4, E5) +@@ +expression E1, E2, E3, E4, E5; +symbol true; +@@ + +- address_space_rw(E1, E2, E3, E4, E5, true) ++ address_space_write(E1, E2, E3, E4, E5) +@@ +expression E1, E2, E3, E4, E5; +@@ + +- address_space_rw(E1, E2, E3, E4, E5, 1) ++ address_space_write(E1, E2, E3, E4, E5)
The address_space_rw() function allows either reads or writes depending on the is_write argument passed to it; this is useful when the direction of the access is determined programmatically (as for instance when handling the KVM_EXIT_MMIO exit reason). Under the hood it just calls either address_space_write() or address_space_read_full(). We also use it a lot with a constant is_write argument, though, which has two issues: * when reading "address_space_rw(..., 1)" this is less immediately clear to the reader as being a write than "address_space_write(...)" * calling address_space_rw() bypasses the optimization in address_space_read() that fast-paths reads of a fixed length This commit was produced with the included Coccinelle script scripts/coccinelle/as-rw-const.patch. Two lines in hw/net/dp8393x.c that Coccinelle produced that were over 80 characters were re-wrapped by hand. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- I could break this down into separate patches by submaintainer, but the patch is not that large and I would argue that it's better for the project if we can try to avoid introducing too much friction into the process of doing 'safe' tree-wide minor refactorings. v1->v2: put the coccinelle script in scripts/coccinelle rather than just in the commit message. --- accel/kvm/kvm-all.c | 6 +-- dma-helpers.c | 4 +- exec.c | 4 +- hw/dma/xlnx-zdma.c | 11 ++--- hw/net/dp8393x.c | 68 ++++++++++++++-------------- hw/net/i82596.c | 25 +++++----- hw/net/lasi_i82596.c | 5 +- hw/ppc/pnv_lpc.c | 8 ++-- hw/s390x/css.c | 12 ++--- qtest.c | 52 ++++++++++----------- target/i386/hvf/x86_mmu.c | 12 ++--- scripts/coccinelle/as_rw_const.cocci | 30 ++++++++++++ 12 files changed, 133 insertions(+), 104 deletions(-) create mode 100644 scripts/coccinelle/as_rw_const.cocci