On 07/23/2017 04:58 PM, Peter Maydell wrote:
On 22 July 2017 at 05:47, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
I'm seeing memleaks using the malta machine, they come from the
smbus_eeprom_init() in hw/i2c/smbus_eeprom.c which does:
uint8_t *eeprom_buf = g_malloc0(8 * 256); /* XXX: make this persistent
*/
(This isn't actually a leak because the eeprom device
can't be hotplugged/unplugged, so once created it will
live for the life of the simulation.)
Question for 2.11: is this device a candidate to use one of the function you
enumerated? My guess is memory_region_init_rom_device()
A quick look at that device suggests that it's doing some
pretty weird stuff which probably should be being
cleaned up to use a memory region to handle the backing
store for the EEPROM (although there are other ways to do it;
since there's only 2K of data there it could also be
handled by a byte-array in the device migrated via
VMState).
Do you know what the intention is of the code which
passes the device an offset into the eeprom_buf buffer
rather than just passing the start of the buffer ?
Question for 2.10: does that mean than the malta machine is not migratable?
Depends whether the guest is actually writing to the EEPROM :-)
The smbus_eeprom device doesn't have any support for migration
at all -- it is neither migrating the data, nor the internal
state (the offset). But if the guest doesn't ever write to
the EEPROM and it doesn't happen to be relying on the offset
having a particular value at the point of migration, you
won't notice.
In particular, the pc_piix and pc_q35 machines both create
one of these devices, so this is an issue for the pc
machine migration too... (and any fix we make to
smbus_eeprom needs to handle migration compat as a result)
Thank for your clear explanation, it seems quite some work to fix this
device (due to the migration part I'm not common yet) so I'll just add
this in my TODO list for 2.13 and will probably come back to you about
that in a far future.
Regards,
Phil.
