[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eepr
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom |
Date: |
Thu, 8 Nov 2018 14:08:31 +0000 |
On 7 November 2018 at 15:54, <address@hidden> wrote:
> From: Corey Minyard <address@hidden>
>
> This was if the eeprom is accessed during a state transfer, the
> transfer will be reliable.
>
> Signed-off-by: Corey Minyard <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Michael S. Tsirkin <address@hidden>
> Cc: Dr. David Alan Gilbert <address@hidden>
> ---
> hw/i2c/smbus_eeprom.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
> index f18aa3de35..d4430b0c5d 100644
> --- a/hw/i2c/smbus_eeprom.c
> +++ b/hw/i2c/smbus_eeprom.c
> @@ -29,6 +29,8 @@
>
> //#define DEBUG
>
> +#define TYPE_SMBUS_EEPROM_DEVICE "smbus-eeprom"
The part of this patch which is adding the usual QOM
macros is good, but we should provide also the
cast-macro (the one that's an OBJECT_CHECK(...)).
And this should be separate from adding the vmstate.
> +
> typedef struct SMBusEEPROMDevice {
> SMBusDevice smbusdev;
> void *data;
> @@ -97,6 +99,17 @@ static uint8_t eeprom_read_data(SMBusDevice *dev, uint8_t
> cmd, int n)
> return eeprom_receive_byte(dev);
> }
>
> +static const VMStateDescription vmstate_smbus_eeprom = {
> + .name = TYPE_SMBUS_EEPROM_DEVICE,
Generally we don't use the TYPE_FOO constant for the vmstate
name, because we can usually freely change the TYPE_FOO string without
breaking back-compat, but we can't change the vmstate name.
So we decouple them to make it more obvious when a change might
be changing the migration on-the-wire format.
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_SMBUS_DEVICE(smbusdev, SMBusEEPROMDevice),
> + VMSTATE_UINT8(offset, SMBusEEPROMDevice),
> + VMSTATE_END_OF_LIST()
> + }
> +};
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).
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...)
> +
> static void smbus_eeprom_realize(DeviceState *dev, Error **errp)
> {
> SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *)dev;
> @@ -121,12 +134,13 @@ static void smbus_eeprom_class_initfn(ObjectClass
> *klass, void *data)
> sc->write_data = eeprom_write_data;
> sc->read_data = eeprom_read_data;
> dc->props = smbus_eeprom_properties;
> + dc->vmsd = &vmstate_smbus_eeprom;
> /* Reason: pointer property "data" */
> dc->user_creatable = false;
> }
>
> static const TypeInfo smbus_eeprom_info = {
> - .name = "smbus-eeprom",
> + .name = TYPE_SMBUS_EEPROM_DEVICE,
> .parent = TYPE_SMBUS_DEVICE,
> .instance_size = sizeof(SMBusEEPROMDevice),
> .class_init = smbus_eeprom_class_initfn,
> --
> 2.17.1
thanks
-- PMM
- [Qemu-devel] [PATCH 0/3] Fix/add vmstate handling in some I2C code, minyard, 2018/11/07
- [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, minyard, 2018/11/07
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom,
Peter Maydell <=
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Corey Minyard, 2018/11/08
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Peter Maydell, 2018/11/08
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Corey Minyard, 2018/11/09
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Peter Maydell, 2018/11/09
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Corey Minyard, 2018/11/09
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Peter Maydell, 2018/11/09
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Dr. David Alan Gilbert, 2018/11/12
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Peter Maydell, 2018/11/12
- Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom, Dr. David Alan Gilbert, 2018/11/12
[Qemu-devel] [PATCH 1/3] i2c:pm_smbus: Fix state transfer, minyard, 2018/11/07