Re: [Qemu-devel] [PATCH v2 00/12] RFC: Fix/add vmstate handling in some I2C code

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Corey,

On 15/11/18 20:24, minyard@acm.org wrote:
> These changes allow SMBus access while doing a state transfer.
> Seems like a good idea to me in general.
> 
> I have these queued for the SMBus IPMI driver work, of course.
> 
> I had submitted this before and then lost track of the work since I
> started finding all kinds of broken things in the I2C code.  I
> have fixed the broken things I found first, and then added the
> previous patches.
> 
> I have tested this in q35 and it works without issue.  On piix4 the
> pm_smbus code is broken on a migration, however. The device disappears
> from the PCI bus on a migration, from what I can tell.  It's not the
> fault of this code, something more fundamental is going on.  The
> following comment in piix4.c may have something to do with it:
> 
> /* qemu-kvm 1.2 uses version 3 but advertised as 2
>   * To support incoming qemu-kvm 1.2 migration, change version_id
>   * and minimum_version_id to 2 below (which breaks migration from
>   * qemu 1.2).
> 
> Anyway, I need to chase that down.
> 
> I'm primarily submitting this to make sure I'm doing the backwards
> compatability with .needed correctly.  I'm adding a new field in
> the machine class and setting it in the initialization code for
> older versions.  David, is this what you wanted?  It will have to
> be adjusted for the proper version if/when it really goes in, of
> course.  You can see those in the following commits:
>    boards.h: Ignore migration for SMBus devices on
>    i2c:pm_smbus: Fix state transfer
>    i2c: Add vmstate handling to the smbus eeprom
> I thought about adding a field to the pm_smbus code to only transfer
> if it was accessed, but I'm assuming that most modern OSes will
> at least initialized the device based on its presence on the PCI
> bus.  So that didn't seem like it would add any value.
> 
> I'm also submitting to see if all the fixes and cleanups look ok.
> That's the first 5 commits.

$ git diff origin/master --summary
  delete mode 100644 hw/i2c/smbus.c
  create mode 100644 hw/i2c/smbus_master.c
  create mode 100644 hw/i2c/smbus_slave.c
  create mode 100644 include/hw/i2c/smbus_eeprom.h
  rename include/hw/i2c/{smbus.h => smbus_master.h} (56%)
  create mode 100644 include/hw/i2c/smbus_slave.h

Can you add the following files in the MAINTAINERS file:
- hw/i2c/smbus_master.c
- hw/i2c/smbus_slave.c
- include/hw/i2c/smbus_eeprom.h
- include/hw/i2c/smbus_master.h
- include/hw/i2c/smbus_slave.h

Thanks,

Phil.