Re: [PATCH V5 05/25] as_flat_walk

[Steven Sistare <steven.sistare@oracle.com>]

Will do for all - steve

On 7/8/2021 9:49 AM, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 7, 2021 at 9:28 PM Steve Sistare <steven.sistare@oracle.com <mailto:steven.sistare@oracle.com>> wrote:
> 
>     Add an iterator over the sections of a flattened address space.
> 
>     Signed-off-by: Steve Sistare <steven.sistare@oracle.com <mailto:steven.sistare@oracle.com>>
>     ---
>      include/exec/memory.h | 17 +++++++++++++++++
>      softmmu/memory.c      | 18 ++++++++++++++++++
>      2 files changed, 35 insertions(+)
> 
>     diff --git a/include/exec/memory.h b/include/exec/memory.h
>     index 7ad63f8..a030aef 100644
>     --- a/include/exec/memory.h
>     +++ b/include/exec/memory.h
>     @@ -2023,6 +2023,23 @@ bool memory_region_present(MemoryRegion *container, hwaddr addr);
>       */
>      bool memory_region_is_mapped(MemoryRegion *mr);
> 
>     +typedef int (*qemu_flat_walk_cb)(MemoryRegionSection *s,
>     +                                 void *handle,
>     +                                 Error **errp);
> 
> 
> Please document the callback type, especially returned values. (see for example flatview_cb)
> 
> Usually, the user pointer is called "opaque".
> 
> Could it be named memory_region_section_cb instead ?
> 
>     +
>     +/**
>     + * as_flat_walk: walk the ranges in the address space flat view and call @func
>     + * for each.  Return 0 on success, else return non-zero with a message in
>     + * @errp.
> 
> 
> Suggest address_space_flat_for_each_section() name ?
> 
>  
> 
>     + *
>     + * @as: target address space
>     + * @func: callback function
>     + * @handle: passed to @func
> 
> 
> opaque
> 
>     + * @errp: passed to @func
>     + */
>     +int as_flat_walk(AddressSpace *as, qemu_flat_walk_cb func,
>     +                 void *handle, Error **errp);
>     +
>      /**
>       * memory_region_find: translate an address/size relative to a
>       * MemoryRegion into a #MemoryRegionSection.
>     diff --git a/softmmu/memory.c b/softmmu/memory.c
>     index e9536bc..1ec1e25 100644
>     --- a/softmmu/memory.c
>     +++ b/softmmu/memory.c
>     @@ -2577,6 +2577,24 @@ bool memory_region_is_mapped(MemoryRegion *mr)
>          return mr->container ? true : false;
>      }
> 
>     +int as_flat_walk(AddressSpace *as, qemu_flat_walk_cb func,
>     +                 void *handle, Error **errp)
>     +{
>     +    FlatView *view = address_space_get_flatview(as);
>     +    FlatRange *fr;
>     +    int ret;
>     +
>     +    FOR_EACH_FLAT_RANGE(fr, view) {
>     +        MemoryRegionSection section = section_from_flat_range(fr, view);
>     +        ret = func(&section, handle, errp);
>     +        if (ret) {
>     +            return ret;
>     +        }
>     +    }
>     +
>     +    return 0;
>     +}
>     +
>      /* Same as memory_region_find, but it does not add a reference to the
>       * returned region.  It must be called from an RCU critical section.
>       */
>     -- 
>     1.8.3.1
> 
> 
> 
> 
> -- 
> Marc-André Lureau