Re: [Qemu-stable] [Qemu-devel] [PATCH] vl: Delay initialization of memory backends

[Eduardo Habkost]

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