Re: [Qemu-devel] [PATCH v3 1/8] memory: address_space_iterate

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 12 Dec 2017 00:50:51 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 11/12/2017 20:46, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Iterate through an address space calling a function for each
> > section.  The iteration is done in order.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>  
> 
> It seems to me that you can achieve the same effect by implementing the
> region_add and region_nop callbacks, and leaving out region_del.  Am I
> missing something?
region_del would also be need to track references correctly.

Initially I've also suggested to reuse region_add|del but
compared to David's approach it will be multiple calls for
a memory transaction vs a pass over flatview at commit time.

> 
> Thanks,
> 
> Paolo
> 
> > ---
> >  include/exec/memory.h | 23 +++++++++++++++++++++++
> >  memory.c              | 22 ++++++++++++++++++++++
> >  2 files changed, 45 insertions(+)
> > 
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index 5ed4042f87..f5a9df642e 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -1987,6 +1987,29 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
> >      address_space_write(cache->as, cache->xlat + addr, MEMTXATTRS_UNSPECIFIED, buf, len);
> >  }
> >  
> > +/**
> > + * ASIterateCallback: Function type called by address_space_iterate
> > + *
> > + * Return 0 on success or a negative error code.
> > + *
> > + * @mrs: Memory region section for this range
> > + * @opaque: The opaque value passed in to the iterator.
> > + */
> > +typedef int (*ASIterateCallback)(MemoryRegionSection *mrs, void *opaque);
> > +
> > +/**
> > + * address_space_iterate: Call the function for each address range in the
> > + *                        AddressSpace, in sorted order.
> > + *
> > + * Return 0 on success or a negative error code.
> > + *
> > + * @as: Address space to iterate over
> > + * @cb: Function to call.  If the function returns none-0 the iteration will
> > + *     stop.
> > + * @opaque: Value to pass to the function
> > + */
> > +int
> > +address_space_iterate(AddressSpace *as, ASIterateCallback cb, void *opaque);
> >  #endif
> >  
> >  #endif
> > diff --git a/memory.c b/memory.c
> > index e26e5a3b1d..f45137f25e 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -2810,6 +2810,28 @@ void address_space_destroy(AddressSpace *as)
> >      call_rcu(as, do_address_space_destroy, rcu);
> >  }
> >  
> > +int address_space_iterate(AddressSpace *as, ASIterateCallback cb,
> > +                          void *opaque)
> > +{
> > +    int res = 0;
> > +    FlatView *fv = address_space_to_flatview(as);
> > +    FlatRange *range;
> > +
> > +    flatview_ref(fv);
> > +
> > +    FOR_EACH_FLAT_RANGE(range, fv) {
> > +        MemoryRegionSection mrs = section_from_flat_range(range, fv);
> > +        res = cb(&mrs, opaque);
> > +        if (res) {
> > +            break;
> > +        }
> > +    }
> > +
> > +    flatview_unref(fv);
> > +
> > +    return res;
> > +}
> > +
> >  static const char *memory_region_type(MemoryRegion *mr)
> >  {
> >      if (memory_region_is_ram_device(mr)) {
> >   
>