On Mon, Aug 19, 2019 at 04:30:45PM +0200, Paolo Bonzini wrote: > On 17/08/19 11:32, Peter Xu wrote: > > The previous has_coalesced_range counter has a problem in that it only > > works for additions of coalesced mmio ranges but not deletions. The > > reason is that has_coalesced_range information can be lost when the > > FlatView updates the topology again when the updated region is not > > covering the coalesced regions. When that happens, due to > > flatrange_equal() is not checking against has_coalesced_range, the new > > FlatRange will be seen as the same one as the old and the new > > instance (whose has_coalesced_range will be zero) will replace the old > > instance (whose has_coalesced_range _could_ be non-zero). > > > > To fix it, we don't cache has_coalesced_range at all in the FlatRange. > > Instead we introduce two flags to make sure the coalesced_io_{add|del} > > will only be called once for every FlatRange instance. This will even > > work if another FlatRange replaces current one. > > It's still a bit ugly that coalesced_mmio_add_done ends up not being set > on the new (but equal) FlatRange. > > Would something like this work too? > > diff --git a/memory.c b/memory.c > index edd0c13..fc91f06 100644 > --- a/memory.c > +++ b/memory.c > @@ -939,6 +939,7 @@ static void > address_space_update_topology_pass(AddressSpace *as, > /* In both and unchanged (except logging may have changed) */ > > if (adding) { > + frnew->has_coalesced_range = frold->has_coalesced_range; > MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, > region_nop); > if (frnew->dirty_log_mask & ~frold->dirty_log_mask) { > MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, > log_start,
This seems to be a much better (and, shorter) idea. :-) I'll verify it and repost if it goes well. Regards, -- Peter Xu