From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41967) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T7TZj-0003Bq-07 for qemu-devel@nongnu.org; Fri, 31 Aug 2012 11:56:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T7TZe-0007Cv-5t for qemu-devel@nongnu.org; Fri, 31 Aug 2012 11:56:02 -0400 Received: from cantor2.suse.de ([195.135.220.15]:50836 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T7TZd-0007Cp-Rw for qemu-devel@nongnu.org; Fri, 31 Aug 2012 11:55:58 -0400 Message-ID: <5040DE81.7090305@suse.de> Date: Fri, 31 Aug 2012 17:55:45 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <201208301800.q7UI04R9009678@int-mx12.intmail.prod.int.phx2.redhat.com> In-Reply-To: <201208301800.q7UI04R9009678@int-mx12.intmail.prod.int.phx2.redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] ahci: add migration support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Baron Cc: kwolf@redhat.com, aliguori@us.ibm.com, Juan Quintela , jan.kiszka@siemens.com, agraf@suse.de, qemu-devel@nongnu.org, yamahata@valinux.co.jp, alex.williamson@redhat.com Am 30.08.2012 20:00, schrieb Jason Baron: > Add support for ahci migration. This patch builds upon the patches post= ed > previously by Andreas Faerber: >=20 > http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01538.html >=20 > (I hope I am giving Andreas proper credit for his work.) Not quite. :) You should adopt the Signed-off-by line(s) from the patch you pick up, add a v3 marker and include a change log to the previous version(s) below "---" or in the cover letter. A link to previous discussion threads is then not necessary. The change log would be even more interesting since this does not seem to be my patch plus your diff from the link. `git commit --amend -s -a` would've even got you my name in UTF-8 the easy way, assuming previous `git-am my.patch` for testing. > I've tested these patches by migrating Windows 7 and Fedora 16 guests o= n > both piix with ahci attached and on q35 (which has a built-in ahci cont= roller). This is good for us to know, but in general sentences with "I" don't need to go into the commit message; once more people handle it up- and downstream (submaintainers, committers, stable branches, SLE/RHEL) it becomes less clear who "I" is. >=20 > Signed-off-by: Jason Baron > --- > hw/ide/ahci.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++= +++++++- > hw/ide/ahci.h | 10 +++++++++ > hw/ide/ich.c | 11 +++++++-- > 3 files changed, 81 insertions(+), 4 deletions(-) >=20 > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index b53c757..e94509b 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -1204,6 +1204,65 @@ void ahci_reset(AHCIState *s) > } > } > =20 > +static const VMStateDescription vmstate_ahci_device =3D { > + .name =3D "ahci port", > + .version_id =3D 1, > + .fields =3D (VMStateField []) { > + VMSTATE_IDE_BUS(port, AHCIDevice), > + VMSTATE_UINT32(port_state, AHCIDevice), > + VMSTATE_UINT32(finished, AHCIDevice), > + VMSTATE_UINT32(port_regs.lst_addr, AHCIDevice), > + VMSTATE_UINT32(port_regs.lst_addr_hi, AHCIDevice), > + VMSTATE_UINT32(port_regs.fis_addr, AHCIDevice), > + VMSTATE_UINT32(port_regs.fis_addr_hi, AHCIDevice), > + VMSTATE_UINT32(port_regs.irq_stat, AHCIDevice), > + VMSTATE_UINT32(port_regs.irq_mask, AHCIDevice), > + VMSTATE_UINT32(port_regs.cmd, AHCIDevice), > + VMSTATE_UINT32(port_regs.tfdata, AHCIDevice), > + VMSTATE_UINT32(port_regs.sig, AHCIDevice), > + VMSTATE_UINT32(port_regs.scr_stat, AHCIDevice), > + VMSTATE_UINT32(port_regs.scr_ctl, AHCIDevice), > + VMSTATE_UINT32(port_regs.scr_err, AHCIDevice), > + VMSTATE_UINT32(port_regs.scr_act, AHCIDevice), > + VMSTATE_UINT32(port_regs.cmd_issue, AHCIDevice), Didn't your diff add port_no to this VMSD? Did that turn out unnecessary? (Did not get around to look into this yet and probably won't before the release since Kevin considered this 1.3 material.) > + VMSTATE_END_OF_LIST() > + }, > +}; > + > +static int ahci_state_post_load(void *opaque, int version_id) > +{ > + int i; > + AHCIState *s =3D opaque; > + > + for (i =3D 0; i < s->ports; i++) { > + AHCIPortRegs *pr =3D &s->dev[i].port_regs; > + > + map_page(&s->dev[i].lst, > + ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 102= 4); > + map_page(&s->dev[i].res_fis, > + ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256= ); > + } > + > + return 0; > +} > + > +const VMStateDescription vmstate_ahci =3D { > + .name =3D "ahci", > + .version_id =3D 1, > + .post_load =3D ahci_state_post_load, > + .fields =3D (VMStateField []) { > + VMSTATE_STRUCT_VARRAY_POINTER_INT32(dev, AHCIState, ports, > + vmstate_ahci_device, AHCIDevice), Where did the declaration of this new macro go? I would expect this to be a series of two patches, first introducing that (so that Juan can ack that part) and then using it here for ahci. Regards, Andreas > + VMSTATE_UINT32(control_regs.cap, AHCIState), > + VMSTATE_UINT32(control_regs.ghc, AHCIState), > + VMSTATE_UINT32(control_regs.irqstatus, AHCIState), > + VMSTATE_UINT32(control_regs.impl, AHCIState), > + VMSTATE_UINT32(control_regs.version, AHCIState), > + VMSTATE_UINT32(idp_index, AHCIState), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > typedef struct SysbusAHCIState { > SysBusDevice busdev; > AHCIState ahci; > @@ -1212,7 +1271,10 @@ typedef struct SysbusAHCIState { > =20 > static const VMStateDescription vmstate_sysbus_ahci =3D { > .name =3D "sysbus-ahci", > - .unmigratable =3D 1, > + .fields =3D (VMStateField []) { > + VMSTATE_AHCI(ahci, AHCIPCIState), > + VMSTATE_END_OF_LIST() > + }, > }; > =20 > static void sysbus_ahci_reset(DeviceState *dev) > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h > index 1200a56..7719dbf 100644 > --- a/hw/ide/ahci.h > +++ b/hw/ide/ahci.h > @@ -307,6 +307,16 @@ typedef struct AHCIPCIState { > AHCIState ahci; > } AHCIPCIState; > =20 > +extern const VMStateDescription vmstate_ahci; > + > +#define VMSTATE_AHCI(_field, _state) { \ > + .name =3D (stringify(_field)), = \ > + .size =3D sizeof(AHCIState), = \ > + .vmsd =3D &vmstate_ahci, = \ > + .flags =3D VMS_STRUCT, = \ > + .offset =3D vmstate_offset_value(_state, _field, AHCIState), = \ > +} > + > typedef struct NCQFrame { > uint8_t fis_type; > uint8_t c; > diff --git a/hw/ide/ich.c b/hw/ide/ich.c > index 272b773..ae6f56f 100644 > --- a/hw/ide/ich.c > +++ b/hw/ide/ich.c > @@ -79,9 +79,14 @@ > #define ICH9_IDP_INDEX 0x10 > #define ICH9_IDP_INDEX_LOG2 0x04 > =20 > -static const VMStateDescription vmstate_ahci =3D { > +static const VMStateDescription vmstate_ich9_ahci =3D { > .name =3D "ahci", > - .unmigratable =3D 1, > + .version_id =3D 1, > + .fields =3D (VMStateField []) { > + VMSTATE_PCI_DEVICE(card, AHCIPCIState), > + VMSTATE_AHCI(ahci, AHCIPCIState), > + VMSTATE_END_OF_LIST() > + }, > }; > =20 > static void pci_ich9_reset(DeviceState *dev) > @@ -152,7 +157,7 @@ static void ich_ahci_class_init(ObjectClass *klass,= void *data) > k->device_id =3D PCI_DEVICE_ID_INTEL_82801IR; > k->revision =3D 0x02; > k->class_id =3D PCI_CLASS_STORAGE_SATA; > - dc->vmsd =3D &vmstate_ahci; > + dc->vmsd =3D &vmstate_ich9_ahci; > dc->reset =3D pci_ich9_reset; > } > =20 >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg