Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom

[Corey Minyard <minyard@acm.org>]

On 11/8/18 12:03 PM, Peter Maydell wrote:
> On 8 November 2018 at 17:58, Corey Minyard <cminyard@mvista.com> wrote:
>> On 11/8/18 8:08 AM, Peter Maydell wrote:
>>> This doesn't do anything for migration of the actual data contents.
>>> The current users of this device (hw/arm/aspeed.c and the
>>> smbus_eeprom_init() function) doesn't do anything
>>> to migrate the contents. For that matter, "user of the device
>>> passes a pointer to a random lump of memory via a device property"
>>> is a bit funky as an interface. The device should allocate its
>>> own memory and migrate it, and the user should just pass the
>>> initial required contents (default being "zero-initialize",
>>> which is what everybody except the mips_fulong2e, mips_malta
>>> and sam460ex want).
>> I debated on this, and it depends on what the eeprom is used for.  If
>> it's a DRAM eeprom, it shouldn't need to be transferred.
> It's guest-visible data; the guest can write it and read it back.
> So it needs to be migrated. Otherwise behaviour after migration
> will not be the same as if the guest had never migrated.


I looked at adding it, but I ran into an issue.  The value is a

DEFINE_PROP_PTR("data", SMBusEEPROMDevice, data)

and that means the data has to be void *, but to transfer it it must be 
a uint8_t *.
The pointer property seems to be a cheap hack, I'm not sure what it will 
take
to fix it.

I was hoping this would be easy.  I guess transferring the eeprom is not
that important, the state of pm_smbus is a lot more critical.  But it would
be nice to fix it since I am messing with that code.

Thanks for your help.

-corey

>>> Does this also break migration from an old QEMU to a new one?
>>> (For Aspeed that's probably ok, but we should flag it up in the
>>> commit message if so. x86 uses need care...)
>>>
>> There is no transfer before, so I don't see why it would break anything.
>> Am I missing something?
> I forget what the behaviour is where the source QEMU didn't
> have a vmstate at all but the destination QEMU does expect
> one. David can remind me...
>
> thanks
> - PMM