Avi Kivity <a...@redhat.com> writes:
> Instead of calling a global function on coalesced mmio changes, which
> routes the call to kvm if enabled, add coalesced mmio hooks to
> MemoryListener and make kvm use that instead.
>
> The motivation is support for multiple address spaces (which means we
> we need to filter the call on the right address space) but the result
> is cleaner as well.
>
> Signed-off-by: Avi Kivity <a...@redhat.com>
> ---
> exec.c | 13 -------------
> kvm-all.c | 20 ++++++++++----------
> kvm-stub.c | 10 ----------
> kvm.h | 2 --
> memory.c | 24 ++++++++++++++++++++----
> memory.h | 12 +++++++++++-
> 6 files changed, 41 insertions(+), 40 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 1fd6a10..51a32e7 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2313,19 +2313,6 @@ void
> cpu_register_physical_memory_log(MemoryRegionSection *section,
> }
> }
>
> -
> -void qemu_register_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size)
> -{
> - if (kvm_enabled())
> - kvm_coalesce_mmio_region(addr, size);
> -}
> -
> -void qemu_unregister_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size)
> -{
> - if (kvm_enabled())
> - kvm_uncoalesce_mmio_region(addr, size);
> -}
> -
> void qemu_flush_coalesced_mmio_buffer(void)
> {
> if (kvm_enabled())
> diff --git a/kvm-all.c b/kvm-all.c
> index 5e9215d..25ca202 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -454,9 +454,10 @@ static int
> kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
> return ret;
> }
>
> -int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
> +static void kvm_coalesce_mmio_region(MemoryListener *listener,
> + MemoryRegionSection *secion,
> + target_phys_addr_t start, ram_addr_t
> size)
> {
> - int ret = -ENOSYS;
> KVMState *s = kvm_state;
>
> if (s->coalesced_mmio) {
> @@ -466,15 +467,14 @@ int kvm_coalesce_mmio_region(target_phys_addr_t start,
> ram_addr_t size)
> zone.size = size;
> zone.pad = 0;
>
> - ret = kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
> + (void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
g_assert on error instead of ignoring it.
> }
> -
> - return ret;
> }
>
> -int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
> +static void kvm_uncoalesce_mmio_region(MemoryListener *listener,
> + MemoryRegionSection *secion,
> + target_phys_addr_t start, ram_addr_t
> size)
> {
> - int ret = -ENOSYS;
> KVMState *s = kvm_state;
>
> if (s->coalesced_mmio) {
> @@ -484,10 +484,8 @@ int kvm_uncoalesce_mmio_region(target_phys_addr_t start,
> ram_addr_t size)
> zone.size = size;
> zone.pad = 0;
>
> - ret = kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
> + (void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
> }
> -
> - return ret;
> }
>
> int kvm_check_extension(KVMState *s, unsigned int extension)
> @@ -818,6 +816,8 @@ static void kvm_io_ioeventfd_del(MemoryListener *listener,
> .log_global_stop = kvm_log_global_stop,
> .eventfd_add = kvm_mem_ioeventfd_add,
> .eventfd_del = kvm_mem_ioeventfd_del,
> + .coalesced_mmio_add = kvm_coalesce_mmio_region,
> + .coalesced_mmio_del = kvm_uncoalesce_mmio_region,
> .priority = 10,
> };
>
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 3c52eb5..a3455e2 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -29,16 +29,6 @@ int kvm_init_vcpu(CPUArchState *env)
> return -ENOSYS;
> }
>
> -int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
> -{
> - return -ENOSYS;
> -}
> -
> -int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
> -{
> - return -ENOSYS;
> -}
> -
> int kvm_init(void)
> {
> return -ENOSYS;
> diff --git a/kvm.h b/kvm.h
> index dea2998..eefcb49 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -129,8 +129,6 @@ void *kvm_vmalloc(ram_addr_t size);
> void *kvm_arch_vmalloc(ram_addr_t size);
> void kvm_setup_guest_memory(void *start, size_t size);
>
> -int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
> -int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
> void kvm_flush_coalesced_mmio_buffer(void);
> #endif
>
> diff --git a/memory.c b/memory.c
> index efefcb8..eb75349 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1130,11 +1130,19 @@ static void
> memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
> FlatRange *fr;
> CoalescedMemoryRange *cmr;
> AddrRange tmp;
> + MemoryRegionSection section;
>
> FOR_EACH_FLAT_RANGE(fr, as->current_map) {
> if (fr->mr == mr) {
> - qemu_unregister_coalesced_mmio(int128_get64(fr->addr.start),
> - int128_get64(fr->addr.size));
> + section = (MemoryRegionSection) {
> + .address_space = as->root,
> + .offset_within_address_space = int128_get64(fr->addr.start),
> + .size = int128_get64(fr->addr.size),
> + };
I think this is a bit too clever. You can move the definition of
section into this block with a zero initializer, and then just set the
fields directly. You'll end up losing 2 lines of code in the process
too.
Regards,
Anthony Liguori
> +
> + MEMORY_LISTENER_CALL(coalesced_mmio_del, Reverse, §ion,
> + int128_get64(fr->addr.start),
> + int128_get64(fr->addr.size));
> QTAILQ_FOREACH(cmr, &mr->coalesced, link) {
> tmp = addrrange_shift(cmr->addr,
> int128_sub(fr->addr.start,
> @@ -1143,8 +1151,9 @@ static void
> memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
> continue;
> }
> tmp = addrrange_intersection(tmp, fr->addr);
> - qemu_register_coalesced_mmio(int128_get64(tmp.start),
> - int128_get64(tmp.size));
> + MEMORY_LISTENER_CALL(coalesced_mmio_add, Forward, §ion,
> + int128_get64(tmp.start),
> + int128_get64(tmp.size));
> }
> }
> }
> @@ -1529,6 +1538,13 @@ void memory_listener_default_eventfd(MemoryListener
> *listener,
> {
> }
>
> +void memory_listener_default_coalesced_mmio(MemoryListener *listener,
> + MemoryRegionSection *section,
> + target_phys_addr_t addr,
> + target_phys_addr_t len)
> +{
> +}
> +
> void address_space_init(AddressSpace *as, MemoryRegion *root)
> {
> memory_region_transaction_begin();
> diff --git a/memory.h b/memory.h
> index 0ef95cb..5f50bce 100644
> --- a/memory.h
> +++ b/memory.h
> @@ -217,6 +217,10 @@ struct MemoryListener {
> bool match_data, uint64_t data, EventNotifier *e);
> void (*eventfd_del)(MemoryListener *listener, MemoryRegionSection
> *section,
> bool match_data, uint64_t data, EventNotifier *e);
> + void (*coalesced_mmio_add)(MemoryListener *listener, MemoryRegionSection
> *section,
> + target_phys_addr_t addr, target_phys_addr_t
> len);
> + void (*coalesced_mmio_del)(MemoryListener *listener, MemoryRegionSection
> *section,
> + target_phys_addr_t addr, target_phys_addr_t
> len);
> /* Lower = earlier (during add), later (during del) */
> unsigned priority;
> MemoryRegion *address_space_filter;
> @@ -235,7 +239,9 @@ struct MemoryListener {
> .log_global_start = memory_listener_default_global, \
> .log_global_stop = memory_listener_default_global, \
> .eventfd_add = memory_listener_default_eventfd, \
> - .eventfd_del = memory_listener_default_eventfd \
> + .eventfd_del = memory_listener_default_eventfd, \
> + .coalesced_mmio_add = memory_listener_default_coalesced_mmio, \
> + .coalesced_mmio_del = memory_listener_default_coalesced_mmio \
>
> void memory_listener_default_global(MemoryListener *listener);
> void memory_listener_default_section(MemoryListener *listener,
> @@ -243,6 +249,10 @@ void memory_listener_default_section(MemoryListener
> *listener,
> void memory_listener_default_eventfd(MemoryListener *listener,
> MemoryRegionSection *section,
> bool match_data, uint64_t data,
> EventNotifier *e);
> +void memory_listener_default_coalesced_mmio(MemoryListener *listener,
> + MemoryRegionSection *section,
> + target_phys_addr_t addr,
> + target_phys_addr_t len);
>
> /**
> * memory_region_init: Initialize a memory region
> --
> 1.7.12
