| Message ID | 20200218112457.22712-1-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| 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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
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) -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
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 -- 2.20.1