Re: [Qemu-devel] [PATCH v3 08/10] Convert address_space_rw to use MemoryAccessType

[David Gibson]

On Tue, Jul 19, 2016 at 10:02:58PM -0700, Andrey Smirnov wrote:
> Convert address_space_rw() to use MemoryAccessType following the
> conversion of cpu_memory_rw_debug().
> 
> Signed-off-by: Andrey Smirnov <address@hidden>

Reviewed-by: David Gibson <address@hidden>

> ---
>  exec.c                   | 14 +++++++++-----
>  include/exec/memory.h    |  7 +++++--
>  kvm-all.c                |  8 +++++---
>  scripts/coverity-model.c |  9 +++++++--
>  4 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 995ff60..5557cc9 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2726,12 +2726,16 @@ MemTxResult address_space_read_full(AddressSpace *as, 
> hwaddr addr,
>  }
>  
>  MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
> -                             uint8_t *buf, int len, bool is_write)
> +                             uint8_t *buf, int len,
> +                             MemoryAccessType access_type)
>  {
> -    if (is_write) {
> +    switch (access_type) {
> +    case MEM_DATA_STORE:
>          return address_space_write(as, addr, attrs, buf, len);
> -    } else {
> +    case MEM_DATA_LOAD:
>          return address_space_read(as, addr, attrs, buf, len);
> +    default:
> +        abort();
>      }
>  }
>  
> @@ -2739,7 +2743,7 @@ void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
>                              int len, int is_write)
>  {
>      address_space_rw(&address_space_memory, addr, MEMTXATTRS_UNSPECIFIED,
> -                     buf, len, is_write);
> +                     buf, len, is_write ? MEM_DATA_STORE : MEM_DATA_LOAD);
>  }
>  
>  enum write_rom_type {
> @@ -3651,7 +3655,7 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong 
> addr,
>          } else {
>              address_space_rw(cpu->cpu_ases[asidx].as, phys_addr,
>                               MEMTXATTRS_UNSPECIFIED,
> -                             buf, l, 0);
> +                             buf, l, access_type);
>          }
>          len -= l;
>          buf += l;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 7851ca9..da56c0e 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -22,6 +22,7 @@
>  #define DIRTY_MEMORY_NUM       3        /* num of dirty bits */
>  
>  #include "exec/cpu-common.h"
> +#include "qom/cpu.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "exec/hwaddr.h"
>  #endif
> @@ -1284,11 +1285,13 @@ void address_space_destroy(AddressSpace *as);
>   * @addr: address within that address space
>   * @attrs: memory transaction attributes
>   * @buf: buffer with the data transferred
> - * @is_write: indicates the transfer direction
> + * @access_type: indicates the transfer direction (only valid values
> + * are MEM_DATA_LOAD for data reads and MEM_DATA_STORE for data
> + * writes)
>   */
>  MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
>                               MemTxAttrs attrs, uint8_t *buf,
> -                             int len, bool is_write);
> +                             int len, MemoryAccessType access_type);
>  
>  /**
>   * address_space_write: write to address space.
> diff --git a/kvm-all.c b/kvm-all.c
> index a88f917..ed45a95 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1767,11 +1767,12 @@ static void kvm_handle_io(uint16_t port, MemTxAttrs 
> attrs, void *data, int direc
>  {
>      int i;
>      uint8_t *ptr = data;
> +    MemoryAccessType access_type =
> +        (direction == KVM_EXIT_IO_OUT) ? MEM_DATA_STORE : MEM_DATA_LOAD;
>  
>      for (i = 0; i < count; i++) {
>          address_space_rw(&address_space_io, port, attrs,
> -                         ptr, size,
> -                         direction == KVM_EXIT_IO_OUT);
> +                         ptr, size, access_type);
>          ptr += size;
>      }
>  }
> @@ -1947,7 +1948,8 @@ int kvm_cpu_exec(CPUState *cpu)
>                               run->mmio.phys_addr, attrs,
>                               run->mmio.data,
>                               run->mmio.len,
> -                             run->mmio.is_write);
> +                             run->mmio.is_write ?
> +                             MEM_DATA_STORE : MEM_DATA_LOAD);
>              ret = 0;
>              break;
>          case KVM_EXIT_IRQ_WINDOW_OPEN:
> diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
> index ee5bf9d..c5603b5 100644
> --- a/scripts/coverity-model.c
> +++ b/scripts/coverity-model.c
> @@ -68,13 +68,18 @@ static void __bufread(uint8_t *buf, ssize_t len)
>  }
>  
>  MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
> -                             uint8_t *buf, int len, bool is_write)
> +                             uint8_t *buf, int len,
> +                             MemoryAccessType access_type)
>  {
>      MemTxResult result;
>  
>      // TODO: investigate impact of treating reads as producing
>      // tainted data, with __coverity_tainted_data_argument__(buf).
> -    if (is_write) __bufread(buf, len); else __bufwrite(buf, len);
> +    if (access_type == MEM_DATA_STORE) {
> +        __bufread(buf, len);
> +    } else {
> +        __bufwrite(buf, len);
> +    }
>  
>      return result;
>  }

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