Considering the following when the same seabios code snippet:
pci_config_writeb(0x31): bdf: 0x0000 pam: 0x0000005b
is executed to mark an pc.ram area 0xc0000 as readonly:
Entering vhost_begin >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Entering vhost_region_del section: 0x7fd037a4bb60 offset_within_region: 0xc0000 size: 2146697216 readonly: 0 vhost_region_del: is_rom: 0, rom_device: 0 vhost_region_del: readable: 1 vhost_region_del: ram_addr 0x0, addr: 0x0 size: 2147483648 vhost_region_del: name: pc.ram Entering vhost_set_memory, section: 0x7fd037a4bb60 add: 0, dev->started: 1 vhost_set_memory: Setting dev->memory_changed = true for start_addr: 0xc0000 Entering vhost_region_add section: 0x7fd037a4baa0 offset_within_region: 0xc0000 size: 32768 readonly: 1 vhost_region_add is readonly !!!!!!!!!!!!!!!!!!! vhost_region_add: is_rom: 0, rom_device: 0 vhost_region_add: readable: 1 vhost_region_add: ram_addr 0x0000000000000000, addr: 0x 0 size: 2147483648 vhost_region_add: name: pc.ram Entering vhost_set_memory, section: 0x7fd037a4baa0 add: 1, dev->started: 1 vhost_dev_assign_memory(); >>>>>>>>>>>>>>>>>>>>>>>>>>>> reg->guest_phys_addr: 0xc0000 vhost_set_memory: Setting dev->memory_changed = true for start_addr: 0xc0000 Entering vhost_region_add section: 0x7fd037a4baa0 offset_within_region: 0xc8000 size: 2146664448 readonly: 0 vhost_region_add: is_rom: 0, rom_device: 0 vhost_region_add: readable: 1 vhost_region_add: ram_addr 0x0000000000000000, addr: 0x 0 size: 2147483648 vhost_region_add: name: pc.ram Entering vhost_set_memory, section: 0x7fd037a4baa0 add: 1, dev->started: 1 vhost_set_memory: Setting dev->memory_changed = true for start_addr: 0xc8000 phys_page_find got PHYS_MAP_NODE_NIL >>>>>>>>>>>>>>>>>>>>>>.. Entering vhost_commit >>>>>>>>>>>>>>>>>>>>>>>>>>>
Note that originally we'd see the cpu_physical_memory_map() failure in vhost_verify_ring_mappings() after the first ->region_del() above.
So, does using a ->commit callback for MemoryListener mean that vhost_verify_ring_mappings() is OK to be called only from the final ->commit callback, and not from each ->region_del + ->region_add callback..? Eg: I seem to recall something about vhost_verify_ring_mappings() being called during each ->region_del() when dev->started == true was important, no..?
It is important in the case there are only some deleted regions, and no added region, or in general when the last callback is a ->region_del().
But it is even better to just call vhost_verify_ring_mappings() once, from the ->region_commit() callback.
If this OK, then it seems a matter of keeping an updated bit for each of the regions in vhost_dev->mem_sections[] and performing the vhost_verify_ring_mappings() on all three above during the final ->commit() call, right..?
Or even better, what about just invoking vhost_verify_ring_mappings() once from ->commit without section start_addr+size + drop the existing !rings_overlap check..?
Either drop the ranges_overlap check, or keep the start_addr/end_addr up-to-date. Compared to Michael's prototype patch, that would be like this:
vhost_begin() { dev->mem_changed_end_addr = 0; dev->mem_changed_start_addr = -1; }
vhost_set_memory() { ... dev->mem_changed_start_addr = MIN(dev->mem_changed_start_addr, start_addr); dev->mem_changed_end_addr = MAX(dev->mem_changed_end_addr, start_addr + size - 1); }
vhost_commit() { if (dev->mem_changed_start_addr > dev->mem_changed_end_addr) { return; } start_addr = dev->mem_changed_start_addr; size = dev->mem_changed_end_addr - dev->mem_changed_start_addr + 1; ... }
Paolo
Does the following look like you'd expect for marking 0xc0000 area as read-only case..?
Entering vhost_begin >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Entering vhost_region_del section: 0x7f16d3a1db60 offset_within_region: 0xc0000 size: 2146697216 readonly: 0 vhost_region_del: is_rom: 0, rom_device: 0 vhost_region_del: readable: 1 vhost_region_del: ram_addr 0x0, addr: 0x0 size: 2147483648 vhost_region_del: name: pc.ram Entering vhost_set_memory, section: 0x7f16d3a1db60 add: 0, dev->started: 1 vhost_set_memory: Setting dev->memory_changed = true for start_addr: 0xc0000 Entering vhost_region_add section: 0x7f16d3a1daa0 offset_within_region: 0xc0000 size: 32768 readonly: 1 vhost_region_add is readonly !!!!!!!!!!!!!!!!!!! vhost_region_add: is_rom: 0, rom_device: 0 vhost_region_add: readable: 1 vhost_region_add: ram_addr 0x0000000000000000, addr: 0x 0 size: 2147483648 vhost_region_add: name: pc.ram Entering vhost_set_memory, section: 0x7f16d3a1daa0 add: 1, dev->started: 1 vhost_dev_assign_memory(); >>>>>>>>>>>>>>>>>>>>>>>>>>>> reg->guest_phys_addr: 0xc0000 vhost_set_memory: Setting dev->memory_changed = true for start_addr: 0xc0000 Entering vhost_region_add section: 0x7f16d3a1daa0 offset_within_region: 0xc8000 size: 2146664448 readonly: 0 vhost_region_add: is_rom: 0, rom_device: 0 vhost_region_add: readable: 1 vhost_region_add: ram_addr 0x0000000000000000, addr: 0x 0 size: 2147483648 vhost_region_add: name: pc.ram Entering vhost_set_memory, section: 0x7f16d3a1daa0 add: 1, dev->started: 1 vhost_set_memory: Setting dev->memory_changed = true for start_addr: 0xc8000 phys_page_find got PHYS_MAP_NODE_NIL >>>>>>>>>>>>>>>>>>>>>>.. Entering vhost_commit >>>>>>>>>>>>>>>>>>>>>>>>>>> vhost_commit, skipping vhost_verify_ring_mappings without start_addr !!!!!!!!!!!!! Entering verify_ring_mappings: start_addr 0x0000000000000000 size: 0 verify_ring_mappings: ring_phys 0x0 ring_size: 0 verify_ring_mappings: ring_phys 0x0 ring_size: 0 verify_ring_mappings: ring_phys 0xed000 ring_size: 5124 verify_ring_mappings: calling cpu_physical_memory_map ring_phys: 0xed000 l: 5124 address_space_map: addr: 0xed000, plen: 5124 address_space_map: l: 4096, len: 5124 address_space_map: section: 0x7f16d92c6020 memory_region_is_ram: 1 readonly: 0 address_space_map: section: 0x7f16d92c6020 offset_within_region: 0xc8000 section size: 2146664448 address_space_map: l: 4096, len: 1028 address_space_map: section: 0x7f16d92c6020 memory_region_is_ram: 1 readonly: 0 address_space_map: section: 0x7f16d92c6020 offset_within_region: 0xc8000 section size: 2146664448 address_space_map: Calling qemu_ram_ptr_length: raddr: 0x ed000 rlen: 5124 address_space_map: After qemu_ram_ptr_length: raddr: 0x ed000 rlen: 5124