Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()

[Laurent Vivier <laurent@vivier.eu>]

Le 15/04/2021 à 12:04, Philippe Mathieu-Daudé a écrit :
> dma_memory_set() does a DMA barrier, set the address space with
> a constant value. The constant value filling code is not specific
> to DMA and can be used for AddressSpace. Extract it as a new
> helper: address_space_set().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/exec/memory.h | 16 ++++++++++++++++
>  softmmu/dma-helpers.c | 16 +---------------
>  softmmu/physmem.c     | 19 +++++++++++++++++++
>  3 files changed, 36 insertions(+), 15 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 5728a681b27..192139af58e 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2568,6 +2568,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
>      }
>  }
>  
> +/**
> + * address_space_set: Fill address space with a constant byte.
> + *
> + * Return a MemTxResult indicating whether the operation succeeded
> + * or failed (eg unassigned memory, device rejected the transaction,
> + * IOMMU fault).
> + *
> + * @as: #AddressSpace to be accessed
> + * @addr: address within that address space
> + * @c: constant byte to fill the memory
> + * @len: the number of bytes to fill with the constant byte
> + * @attrs: memory transaction attributes
> + */
> +MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
> +                              uint8_t c, hwaddr len, MemTxAttrs attrs);
> +
>  #ifdef NEED_CPU_H
>  /* enum device_endian to MemOp.  */
>  static inline MemOp devend_memop(enum device_endian end)
> diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
> index 7d766a5e89a..8e1e7ad5320 100644
> --- a/softmmu/dma-helpers.c
> +++ b/softmmu/dma-helpers.c
> @@ -23,21 +23,7 @@ MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr,
>  {
>      dma_barrier(as, DMA_DIRECTION_FROM_DEVICE);
>  
> -#define FILLBUF_SIZE 512
> -    uint8_t fillbuf[FILLBUF_SIZE];
> -    int l;
> -    MemTxResult error = MEMTX_OK;
> -
> -    memset(fillbuf, c, FILLBUF_SIZE);
> -    while (len > 0) {
> -        l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
> -        error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED,
> -                                     fillbuf, l);
> -        len -= l;
> -        addr += l;
> -    }
> -
> -    return error;
> +    return address_space_set(as, addr, c, len, MEMTXATTRS_UNSPECIFIED);
>  }
>  
>  void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 85034d9c11e..c9117527ae7 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2891,6 +2891,25 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
>      }
>  }
>  
> +MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
> +                              uint8_t c, hwaddr len, MemTxAttrs attrs)
> +{
> +#define FILLBUF_SIZE 512
> +    uint8_t fillbuf[FILLBUF_SIZE];
> +    int l;
> +    MemTxResult error = MEMTX_OK;
> +
> +    memset(fillbuf, c, FILLBUF_SIZE);
> +    while (len > 0) {
> +        l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
> +        error |= address_space_write(as, addr, attrs, fillbuf, l);
> +        len -= l;
> +        addr += l;
> +    }
> +
> +    return error;
> +}
> +
>  void cpu_physical_memory_rw(hwaddr addr, void *buf,
>                              hwaddr len, bool is_write)
>  {
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>