Re: [Qemu-devel] [PATCH 0/3] Add new utility function memory_region_allocate_aux_memory()

[Paolo Bonzini]

On 06/07/2017 16:52, Peter Maydell wrote:
> On 5 July 2017 at 13:21, Paolo Bonzini <address@hidden> wrote:
>>
>>
>> On 04/07/2017 19:02, Peter Maydell wrote:
>>> Many board models and several devices need to create auxiliary
>>> regions of RAM (in addition to the main lump of 'system' memory),
>>> to model static RAMs, video memory, ROMs, etc. Currently they do
>>> this with a sequence like:
>>>        memory_region_init_ram(sram, NULL, "sram", 0x10000, &error_fatal);
>>>        vmstate_register_ram_global(sram);
>>
>> Instead of vmstate_register_ram_global, you should use
>>
>>     vmstate_register_ram(mr, owner);
>>
>> You should even do it for all memory regions, probably.
> 
> This sounds like a good thing, but it's awkward for migration
> compatibility, because these callers to memory_region_init_ram()
> don't call vmstate_register_ram():
> 
> hw/arm/highbank.c (a bug)
> hw/mips/mips_malta.c (region is ro)
> hw/net/dp3893x.c (prom, ro, contains mac address)
> hw/pci-host/xilinx-pcie.c (dummy region; migrating wouldn't hurt)
> backends/hostmem-ram.c (bug, or is migration handled elsewhere?)

It's handled by memory_region_allocate_system_memory.

> and if we add an implicit call then we break migration compat
> for those boards/devices.

Forward migration should still work.  The backwards-incompatible part
would be that unused memory backend objects would have to be present on
the destination as well when migrating.  I think it's acceptable.

>> Only memory_region_init_ram_device_ptr (which sets mr->ram_device) must
>> not call vmstate_register_ram.  This is a bit ugly because it requires
>> inlining memory_region_init_ram_ptr into it.
>>
>> memory_region_init_ram_from_fd probably needs to be excluded, as well,
>> based on its sole user.
> 
> Callers of memory_region_init_ram_from_file() which don't
> call vmstate_register_ram():
> backends/hostmem-ram.c

Same as above.

> Callers of memory_region_init_ram_ptr() which don't call
> vmstate_register_ram():
> hw/misc/mmio_interface.c
>  -- seems to be an implementation detail of the exceute-from-device
>     support so maybe it doesn't need migration support ??
I think so...

Paolo