qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [PATCH v2 00/12] RFC: Fix/add vmstate handling in some I2C code
Date: Fri, 16 Nov 2018 00:01:21 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0

Hi Corey,

On 15/11/18 20:24, address@hidden 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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]