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

[Corey Minyard]

On 11/8/18 12:03 PM, Peter Maydell wrote:
> On 8 November 2018 at 17:58, Corey Minyard <address@hidden> 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