Am 04.01.2013 20:44, schrieb Jason Baron:
> From: Jason Baron <jba...@redhat.com>
>
> I've tested these patches by migrating Windows 7 and Fedora 17 guests (while
> uunder i/o) on both piix with ahci attached and on q35 (which has a built-in
> ahci controller).
>
> Changes from v2:
> -migrate all relevant ahci fields
> -flush any pending i/o in 'post_load'
>
> Changes from v1:
> -extend Andreas Färber's patch
>
> Signed-off-by: Jason Baron <jba...@redhat.com>
> Signed-off-by: Andreas Färber <afaer...@suse.de>
> Cc: Alexander Graf <ag...@suse.de>
> Cc: Kevin Wolf <kw...@redhat.com>
> Cc: Juan Quintela <quint...@redhat.com>
> Cc: Igor Mitsyanko <i.mitsya...@samsung.com>
I have a few comments below, but in general this seems to get migration
right for AHCI in its current state.
Unfortunately I noticed only now that AHCI completely ignores -drive
werror/rerror=stop. Once we introduce this, we'll probably get some more
state that needs to be transferred. We'll have to introduce a subsection
then, which isn't nice, but I guess good enough that it shouldn't block
this patch.
> ---
> hw/ide/ahci.c | 80
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> hw/ide/ahci.h | 10 +++++++
> hw/ide/ich.c | 11 +++++--
> 3 files changed, 97 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 72cd1c8..96f224b 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1199,6 +1199,81 @@ void ahci_reset(AHCIState *s)
> }
> }
>
> +static const VMStateDescription vmstate_ahci_device = {
> + .name = "ahci port",
> + .version_id = 1,
> + .fields = (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),
> + VMSTATE_INT32(done_atapi_packet, AHCIDevice),
You should change the type of the struct field from int to int32_t then,
I guess.
> + VMSTATE_INT32(busy_slot, AHCIDevice),
> + VMSTATE_INT32(init_d2h_sent, AHCIDevice),
For these two as well.
> + VMSTATE_END_OF_LIST()
> + },
> +};
Fields from the struct not being saved are:
- dma: Immutable, ok
- port_no: Immutable, ok
- hba: Immutable, ok
- check_bh: Handled in post_load, ok
- lst: Handled in post_load, ok
- res_fis: Handled in post_load, ok
- cur_cmd: Handled in post_load by check_cmd(), probably ok
- ncq_tfs: AIO is flushed before migration, so it's unused, ok
> +
> +static int ahci_state_post_load(void *opaque, int version_id)
> +{
> + int i;
> + struct AHCIDevice *ad;
> + AHCIState *s = opaque;
> +
> + for (i = 0; i < s->ports; i++) {
> + ad = &s->dev[i];
> + AHCIPortRegs *pr = &ad->port_regs;
> +
> + map_page(&ad->lst,
> + ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 1024);
> + map_page(&ad->res_fis,
> + ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256);
> + /*
> + * All pending i/o should be flushed out on a migrate. However,
> + * we might not have cleared the busy_slot since this is done
> + * in a bh. Also, issue i/o against any slots that are pending.
> + */
> + if (ad->busy_slot != -1) {
The original condition in ahci_check_cmd_bh was:
if ((ad->busy_slot != -1) &&
!(ad->port.ifs[0].status & (BUSY_STAT|DRQ_STAT))) {
Under the assumption that no I/O is in flight, I guess omitting the
condition isn't wrong. If it doesn't hurt, I'd prefer to keep it around,
though, because I think the assumption won't hold true in the long term
when werror/rerror support is introduced.
> + pr->cmd_issue &= ~(1 << ad->busy_slot);
> + ad->busy_slot = -1;
> + }
> + check_cmd(s, i);
> + }
> +
> + return 0;
> +}
> +
> +const VMStateDescription vmstate_ahci = {
> + .name = "ahci",
> + .version_id = 1,
> + .post_load = ahci_state_post_load,
> + .fields = (VMStateField []) {
> + VMSTATE_STRUCT_VARRAY_POINTER_INT32(dev, AHCIState, ports,
> + vmstate_ahci_device, AHCIDevice),
> + 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_INT32(ports, AHCIState),
Another int -> int32_t
> + VMSTATE_END_OF_LIST()
> + },
Fields from the struct not being saved are:
- mem, idp: Immutable, ok
- idp_offset: Immutable, ok
- irq: Immutable, ok
- dma_context: Immutable, ok
> +};
> +
> typedef struct SysbusAHCIState {
> SysBusDevice busdev;
> AHCIState ahci;
> @@ -1207,7 +1282,10 @@ typedef struct SysbusAHCIState {
>
> static const VMStateDescription vmstate_sysbus_ahci = {
> .name = "sysbus-ahci",
> - .unmigratable = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_AHCI(ahci, AHCIPCIState),
> + VMSTATE_END_OF_LIST()
> + },
> };
>
> static void sysbus_ahci_reset(DeviceState *dev)
Kevin
