On Wed, 4 May 2016 16:52:14 +1000 Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
> When a new memory listener is registered, listener_add_address_space() > is called and which in turn calls region_add() callbacks of memory regions. > However when unregistering the memory listener, it is just removed from > the listening chain and no region_del() is called. > > This adds listener_del_address_space() and uses it in > memory_listener_unregister(). listener_add_address_space() was used as > a template with the following changes: > s/log_global_start/log_global_stop/ > s/log_start/log_stop/ > s/region_add/region_del/ > > This will allow the following patches to add/remove DMA windows > dynamically from VFIO's PCI address space's region_add()/region_del(). Following patch 1 comments, it would be a bug if the kernel actually needed this to do cleanup, we must release everything if QEMU gets shot with a SIGKILL anyway. So what does this cleanup facilitate in QEMU? Having QEMU trigger an unmap for each region_del is not going to be as efficient as just dropping the container and letting the kernel handle the cleanup all in one go. Thanks, Alex > > Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru> > --- > memory.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/memory.c b/memory.c > index f76f85d..f762a34 100644 > --- a/memory.c > +++ b/memory.c > @@ -2185,6 +2185,49 @@ static void listener_add_address_space(MemoryListener > *listener, > flatview_unref(view); > } > > +static void listener_del_address_space(MemoryListener *listener, > + AddressSpace *as) > +{ > + FlatView *view; > + FlatRange *fr; > + > + if (listener->address_space_filter > + && listener->address_space_filter != as) { > + return; > + } > + > + if (listener->begin) { > + listener->begin(listener); > + } > + if (global_dirty_log) { > + if (listener->log_global_stop) { > + listener->log_global_stop(listener); > + } > + } > + > + view = address_space_get_flatview(as); > + FOR_EACH_FLAT_RANGE(fr, view) { > + MemoryRegionSection section = { > + .mr = fr->mr, > + .address_space = as, > + .offset_within_region = fr->offset_in_region, > + .size = fr->addr.size, > + .offset_within_address_space = int128_get64(fr->addr.start), > + .readonly = fr->readonly, > + }; > + if (fr->dirty_log_mask && listener->log_stop) { > + listener->log_stop(listener, §ion, 0, fr->dirty_log_mask); > + } > + if (listener->region_del) { > + listener->region_del(listener, §ion); > + } > + } > + if (listener->commit) { > + listener->commit(listener); > + } > + flatview_unref(view); > +} > + > void memory_listener_register(MemoryListener *listener, AddressSpace *filter) > { > MemoryListener *other = NULL; > @@ -2211,6 +2254,11 @@ void memory_listener_register(MemoryListener > *listener, AddressSpace *filter) > > void memory_listener_unregister(MemoryListener *listener) > { > + AddressSpace *as; > + > + QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { > + listener_del_address_space(listener, as); > + } > QTAILQ_REMOVE(&memory_listeners, listener, link); > } >