On Tue, Aug 16, 2022 at 06:12:49AM -0400, Emanuele Giuseppe Esposito wrote:
> kvm listeners now need ->commit callback in order to actually send
> the ioctl to the hypervisor. Therefore, add missing callers around
> address_space_set_flatview(), which in turn calls
> address_space_update_topology_pass() which calls ->region_* and
> ->log_* callbacks.
>
> Using MEMORY_LISTENER_CALL_GLOBAL is a little bit an overkill,
> but it is harmless, considering that other listeners that are not
> invoked in address_space_update_topology_pass() won't do anything,
> since they won't have anything to commit.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@xxxxxxxxxx>
> ---
> softmmu/memory.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 7ba2048836..1afd3f9703 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1076,7 +1076,9 @@ static void address_space_update_topology(AddressSpace *as)
> if (!g_hash_table_lookup(flat_views, physmr)) {
> generate_memory_topology(physmr);
> }
> + MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
> address_space_set_flatview(as);
> + MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
Should the pair be with MEMORY_LISTENER_CALL() rather than the global
version? Since it's only updating one address space.
Besides the perf implication (walking per-as list should be faster than
walking global memory listener list?), I think it feels broken too since
we'll call begin() then commit() (with no region_add()/region_del()/..) for
all the listeners that are not registered against this AS. IIUC it will
empty all regions with those listeners?
Thanks,
--
Peter Xu
