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

[Paolo Bonzini]

On 02/09/2016 16:04, Eduardo Habkost wrote:
> 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?

Because there's no way for it to tell to QEMU that writes happened in
those region.  It can do so for migration because it uses
memory_region_sync_dirty_bitmap *and* is active for all memory.

It could do so for VGA only at the cost of enabling dirty tracking for
all memory, and it could not do it at all for TCG (DIRTY_MEMORY_CODE)
because it doesn't use memory_region_sync_dirty_bitmap.

>> 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?

Because it uses iPXE to exercise the NIC.

Paolo