Re: [PATCH 1/2] exec/memory: Let address_space_read/write_cached() propagate MemTxResult

[Paolo Bonzini <pbonzini@redhat.com>]

On 17/05/20 18:48, Philippe Mathieu-Daudé wrote:
> Both address_space_read_cached_slow() and
> address_space_write_cached_slow() return a MemTxResult type.
> Do not discard it, return it to the caller.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/exec/memory.h | 19 +++++++++++--------
>  exec.c                | 16 ++++++++--------
>  2 files changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index e000bd2f97..5e8c009169 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2343,10 +2343,11 @@ void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
>  
>  /* Internal functions, part of the implementation of address_space_read_cached
>   * and address_space_write_cached.  */
> -void address_space_read_cached_slow(MemoryRegionCache *cache,
> -                                    hwaddr addr, void *buf, hwaddr len);
> -void address_space_write_cached_slow(MemoryRegionCache *cache,
> -                                     hwaddr addr, const void *buf, hwaddr len);
> +MemTxResult address_space_read_cached_slow(MemoryRegionCache *cache,
> +                                           hwaddr addr, void *buf, hwaddr len);
> +MemTxResult address_space_write_cached_slow(MemoryRegionCache *cache,
> +                                            hwaddr addr, const void *buf,
> +                                            hwaddr len);
>  
>  static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
>  {
> @@ -2411,15 +2412,16 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
>   * @buf: buffer with the data transferred
>   * @len: length of the data transferred
>   */
> -static inline void
> +static inline MemTxResult
>  address_space_read_cached(MemoryRegionCache *cache, hwaddr addr,
>                            void *buf, hwaddr len)
>  {
>      assert(addr < cache->len && len <= cache->len - addr);
>      if (likely(cache->ptr)) {
>          memcpy(buf, cache->ptr + addr, len);
> +        return MEMTX_OK;
>      } else {
> -        address_space_read_cached_slow(cache, addr, buf, len);
> +        return address_space_read_cached_slow(cache, addr, buf, len);
>      }
>  }
>  
> @@ -2431,15 +2433,16 @@ address_space_read_cached(MemoryRegionCache *cache, hwaddr addr,
>   * @buf: buffer with the data transferred
>   * @len: length of the data transferred
>   */
> -static inline void
> +static inline MemTxResult
>  address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
>                             const void *buf, hwaddr len)
>  {
>      assert(addr < cache->len && len <= cache->len - addr);
>      if (likely(cache->ptr)) {
>          memcpy(cache->ptr + addr, buf, len);
> +        return MEMTX_OK;
>      } else {
> -        address_space_write_cached_slow(cache, addr, buf, len);
> +        return address_space_write_cached_slow(cache, addr, buf, len);
>      }
>  }
>  
> diff --git a/exec.c b/exec.c
> index 5162f0d12f..877b51cc5c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3716,7 +3716,7 @@ static inline MemoryRegion *address_space_translate_cached(
>  /* Called from RCU critical section. address_space_read_cached uses this
>   * out of line function when the target is an MMIO or IOMMU region.
>   */
> -void
> +MemTxResult
>  address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr,
>                                     void *buf, hwaddr len)
>  {
> @@ -3726,15 +3726,15 @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr,
>      l = len;
>      mr = address_space_translate_cached(cache, addr, &addr1, &l, false,
>                                          MEMTXATTRS_UNSPECIFIED);
> -    flatview_read_continue(cache->fv,
> -                           addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> -                           addr1, l, mr);
> +    return flatview_read_continue(cache->fv,
> +                                  addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> +                                  addr1, l, mr);
>  }
>  
>  /* Called from RCU critical section. address_space_write_cached uses this
>   * out of line function when the target is an MMIO or IOMMU region.
>   */
> -void
> +MemTxResult
>  address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr,
>                                      const void *buf, hwaddr len)
>  {
> @@ -3744,9 +3744,9 @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr,
>      l = len;
>      mr = address_space_translate_cached(cache, addr, &addr1, &l, true,
>                                          MEMTXATTRS_UNSPECIFIED);
> -    flatview_write_continue(cache->fv,
> -                            addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> -                            addr1, l, mr);
> +    return flatview_write_continue(cache->fv,
> +                                   addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> +                                   addr1, l, mr);
>  }
>  
>  #define ARG1_DECL                MemoryRegionCache *cache
> 

Queued patch 1, thanks.

Paolo