On Fri, Sep 02, 2016 at 10:59:21AM +0200, Paolo Bonzini wrote: > > > On 01/09/2016 21:34, Eduardo Habkost wrote: > > 1) vhost_user_set_mem_table() fails because dev->mem->nregions is 0 > > 2) dev->mem->nregions is supposed to get new entries based on the > > memory listener callbacks > > 3) vhost_region_add() gets called properly, and calls > > vhost_set_memory(), but: > > 4) vhost_set_memory() forces add=false if > > memory_region_get_dirty_log_mask(section->mr) & ~(1 << > > DIRTY_MEMORY_MIGRATION) > > (I have no idea why) > > DIRTY_MEMORY_MIGRATION is special-cased because, when it is activated, > it is reported to the MemoryListener in two different ways: through > log_global_start/stop and through log_start/stop. vhost only supports > the former. > > > 5) memory_region_init_ram_from_file() sets: > > mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; > > (I don't understand what are the consequences of this) > > TCG requires precise dirty page tracking for all memory, in order to > figure out when to recompile code. It is the same as migration in that > it is global, but it is not the same in that it must be precise at all > times---it cannot use for example memory_region_sync_dirty_bitmap (which > calls log_sync on the MemoryListener to fetch the dirty bitmap from the > vhost server). So TCG is quite fundamentally incompatible with vhost.
Thanks for the explanation. I just don't understand one part: why exactly vhost needs to ignore regions that are being logged? > > > 6) The tcg_enabled() check above is broken if the memory region > > is created before configure_accelerator() is called. My patch > > moves memory backend initialization after > > configure_accelerator() > > > > I'm very confused. My patch seems to fix the dirty_log_mask > > initialization at (5) by accident? But for some reason this > > breaks vhost-user and makes it ignore all memory regions if using > > TCG? (vhost-user-test forces accel=tcg, BTW) > > Yes, your patch fixes a bug actually. Perhaps you can add an > x-i-know-what-i-am-doing flag for vhost-user that is used by the test, > or we can just restrict vhost-user-test to KVM and hence x86 Linux hosts. So, it looks like the patch only breaks vhost-user-test, but shouldn't break actual vhost-user use cases. That sounds better. Now, why exactly vhost-user-test needs accel=tcg and can't use accel=qtest? -- Eduardo
