Re: [Qemu-devel] [PATCH 1/9] Avoid needless calls to address_space_rw()

[David Gibson]

On Thu, Jul 07, 2016 at 04:33:35PM -0700, Andrey Smirnov wrote:
> Avoid calling address_space_rw() when direction of the transfer is
> constant and known at compile time and replace them with explicit calls
> to address_space_read()/address_space_write().
> 
> Signed-off-by: Andrey Smirnov <address@hidden>

Reviewed-by: David Gibson <address@hidden>

> ---
>  dma-helpers.c    |  4 ++--
>  exec.c           | 10 +++++-----
>  hw/net/dp8393x.c | 56 
> ++++++++++++++++++++++++++++----------------------------
>  3 files changed, 35 insertions(+), 35 deletions(-)
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index b521d84..96a6d6e 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 0122ef7..93611f1 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3224,7 +3224,7 @@ uint32_t address_space_ldub(AddressSpace *as, hwaddr 
> addr,
>      uint8_t val;
>      MemTxResult r;
>  
> -    r = address_space_rw(as, addr, attrs, &val, 1, 0);
> +    r = address_space_read(as, addr, attrs, &val, 1);
>      if (result) {
>          *result = r;
>      }
> @@ -3474,7 +3474,7 @@ void address_space_stb(AddressSpace *as, hwaddr addr, 
> uint32_t val,
>      uint8_t v = val;
>      MemTxResult r;
>  
> -    r = address_space_rw(as, addr, attrs, &v, 1, 1);
> +    r = address_space_write(as, addr, attrs, &v, 1);
>      if (result) {
>          *result = r;
>      }
> @@ -3582,7 +3582,7 @@ void address_space_stq(AddressSpace *as, hwaddr addr, 
> uint64_t val,
>  {
>      MemTxResult r;
>      val = tswap64(val);
> -    r = address_space_rw(as, addr, attrs, (void *) &val, 8, 1);
> +    r = address_space_write(as, addr, attrs, (void *) &val, 8);
>      if (result) {
>          *result = r;
>      }
> @@ -3593,7 +3593,7 @@ void address_space_stq_le(AddressSpace *as, hwaddr 
> addr, uint64_t val,
>  {
>      MemTxResult r;
>      val = cpu_to_le64(val);
> -    r = address_space_rw(as, addr, attrs, (void *) &val, 8, 1);
> +    r = address_space_write(as, addr, attrs, (void *) &val, 8);
>      if (result) {
>          *result = r;
>      }
> @@ -3603,7 +3603,7 @@ void address_space_stq_be(AddressSpace *as, hwaddr 
> addr, uint64_t val,
>  {
>      MemTxResult r;
>      val = cpu_to_be64(val);
> -    r = address_space_rw(as, addr, attrs, (void *) &val, 8, 1);
> +    r = address_space_write(as, addr, attrs, (void *) &val, 8);
>      if (result) {
>          *result = r;
>      }
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 0fa652c..2849542 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -203,9 +203,9 @@ static void dp8393x_do_load_cam(dp8393xState *s)
>  
>      while (s->regs[SONIC_CDC] & 0x1f) {
>          /* Fill current entry */
> -        address_space_rw(&s->as,
> +        address_space_read(&s->as,
>              (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
> -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>          s->cam[index][0] = data[1 * width] & 0xff;
>          s->cam[index][1] = data[1 * width] >> 8;
>          s->cam[index][2] = data[2 * width] & 0xff;
> @@ -222,9 +222,9 @@ static void dp8393x_do_load_cam(dp8393xState *s)
>      }
>  
>      /* Read CAM enable */
> -    address_space_rw(&s->as,
> +    address_space_read(&s->as,
>          (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
> -        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>      s->regs[SONIC_CE] = data[0 * width];
>      DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]);
>  
> @@ -242,9 +242,9 @@ 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,
> +    address_space_read(&s->as,
>          (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP],
> -        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>  
>      /* Update SONIC registers */
>      s->regs[SONIC_CRBA0] = data[0 * width];
> @@ -360,9 +360,9 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
>                  (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_CTDA]);
>          size = sizeof(uint16_t) * 6 * width;
>          s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA];
> -        address_space_rw(&s->as,
> +        address_space_read(&s->as,
>              ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + 
> sizeof(uint16_t) * width,
> -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>          tx_len = 0;
>  
>          /* Update registers */
> @@ -386,18 +386,18 @@ 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,
> +            address_space_read(&s->as,
>                  (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0],
> -                MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0);
> +                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,
> +                address_space_read(&s->as,
>                      ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + 
> sizeof(uint16_t) * (4 + 3 * i) * width,
> -                    MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
> +                    MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>                  s->regs[SONIC_TSA0] = data[0 * width];
>                  s->regs[SONIC_TSA1] = data[1 * width];
>                  s->regs[SONIC_TFS] = data[2 * width];
> @@ -429,16 +429,16 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
>          /* Write status */
>          data[0 * width] = s->regs[SONIC_TCR] & 0x0fff; /* status */
>          size = sizeof(uint16_t) * width;
> -        address_space_rw(&s->as,
> +        address_space_write(&s->as,
>              (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA],
> -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>  
>          if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) {
>              /* Read footer of packet */
>              size = sizeof(uint16_t) * width;
> -            address_space_rw(&s->as,
> +            address_space_read(&s->as,
>                  ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + 
> sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width,
> -                MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
> +                MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>              s->regs[SONIC_CTDA] = data[0 * width] & ~0x1;
>              if (data[0 * width] & 0x1) {
>                  /* EOL detected */
> @@ -701,8 +701,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 = ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + 
> sizeof(uint16_t) * 5 * width;
> -        address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED,
> -                         (uint8_t *)data, size, 0);
> +        address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED,
> +                           (uint8_t *)data, size);
>          if (data[0 * width] & 0x1) {
>              /* Still EOL ; stop reception */
>              return -1;
> @@ -721,11 +721,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, 
> const uint8_t * buf,
>      /* Put packet into RBA */
>      DPRINTF("Receive packet at %08x\n", (s->regs[SONIC_CRBA1] << 16) | 
> s->regs[SONIC_CRBA0]);
>      address = (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0];
> -    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;
> @@ -753,23 +753,23 @@ static ssize_t dp8393x_receive(NetClientState *nc, 
> const uint8_t * buf,
>      data[3 * width] = s->regs[SONIC_TRBA1]; /* pkt_ptr1 */
>      data[4 * width] = s->regs[SONIC_RSC]; /* seq_no */
>      size = sizeof(uint16_t) * 5 * width;
> -    address_space_rw(&s->as, (s->regs[SONIC_URDA] << 16) | 
> s->regs[SONIC_CRDA],
> -        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
> +    address_space_write(&s->as, (s->regs[SONIC_URDA] << 16) | 
> s->regs[SONIC_CRDA],
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>  
>      /* Move to next descriptor */
>      size = sizeof(uint16_t) * width;
> -    address_space_rw(&s->as,
> +    address_space_read(&s->as,
>          ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + 
> sizeof(uint16_t) * 5 * width,
> -        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
>      s->regs[SONIC_LLFA] = data[0 * width];
>      if (s->regs[SONIC_LLFA] & 0x1) {
>          /* EOL detected */
>          s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
>      } else {
>          data[0 * width] = 0; /* in_use */
> -        address_space_rw(&s->as,
> +        address_space_write(&s->as,
>              ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + 
> sizeof(uint16_t) * 6 * width,
> -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, sizeof(uint16_t), 1);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)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);

-- 
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

signature.asc
Description: PGP signature