On 03/06/2015 14:22, Igor Mammedov wrote:
> QEMU assert in vhost due to hitting vhost bachend limit
> on number of supported memory regions.
>
> Instead of increasing limit in backends, describe all hotlugged
> memory as one continuos range to vhost that implements linear
> 1:1 HVA->GPA mapping in backend.
> It allows to avoid increasing VHOST_MEMORY_MAX_NREGIONS limit
> in kernel and refactoring current region lookup algorithm
> to a faster/scalable datastructure. The same applies to
> vhost user which has even lower limit.
>
> Signed-off-by: Igor Mammedov <[email protected]>
> ---
> hw/i386/pc.c | 4 ++--
> hw/virtio/vhost.c | 15 ++++++++++++---
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 1eb1db0..c722339 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1306,8 +1306,8 @@ FWCfgState *pc_memory_init(MachineState *machine,
> exit(EXIT_FAILURE);
> }
>
> - memory_region_init(&pcms->hotplug_memory, OBJECT(pcms),
> - "hotplug-memory", hotplug_mem_size);
> + memory_region_init_rsvd_hva(&pcms->hotplug_memory, OBJECT(pcms),
> + "hotplug-memory", hotplug_mem_size);
> memory_region_add_subregion(system_memory, pcms->hotplug_memory_base,
> &pcms->hotplug_memory);
> }
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 54851b7..44cfaab 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -380,6 +380,7 @@ static void vhost_set_memory(MemoryListener *listener,
> bool log_dirty = memory_region_is_logging(section->mr);
> int s = offsetof(struct vhost_memory, regions) +
> (dev->mem->nregions + 1) * sizeof dev->mem->regions[0];
> + MemoryRegionSection rsvd_hva;
> void *ram;
>
> dev->mem = g_realloc(dev->mem, s);
> @@ -388,17 +389,25 @@ static void vhost_set_memory(MemoryListener *listener,
> add = false;
> }
>
> + rsvd_hva = memory_region_find_rsvd_hva(section->mr);
> + if (rsvd_hva.mr) {
> + start_addr = rsvd_hva.offset_within_address_space;
> + size = int128_get64(rsvd_hva.size);
> + ram = memory_region_get_ram_ptr(rsvd_hva.mr);
> + } else {
> + ram = memory_region_get_ram_ptr(section->mr) +
> section->offset_within_region;
> + }
I don't think this is needed.
What _could_ be useful is to merge adjacent ranges even if they are
partly unmapped, but your patch doesn't do that.
But converting to a "reserved HVA" range should have been done already
in memory_region_add_subregion_common.
Am I missing something? (And I see now that my request about
memory_region_get_ram_ptr is linked to this bit of your patch).
Paolo
> assert(size);
>
> /* Optimize no-change case. At least cirrus_vga does this a lot at this
> time. */
> - ram = memory_region_get_ram_ptr(section->mr) +
> section->offset_within_region;
> if (add) {
> - if (!vhost_dev_cmp_memory(dev, start_addr, size, (uintptr_t)ram)) {
> + if (!rsvd_hva.mr && !vhost_dev_cmp_memory(dev, start_addr, size,
> (uintptr_t)ram)) {
> /* Region exists with same address. Nothing to do. */
> return;
> }
> } else {
> - if (!vhost_dev_find_reg(dev, start_addr, size)) {
> + if (!rsvd_hva.mr && !vhost_dev_find_reg(dev, start_addr, size)) {
> /* Removing region that we don't access. Nothing to do. */
> return;
> }
>
