| Message ID | 20170529131716.94338-1-pasic@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
* Halil Pasic (pasic@linux.vnet.ibm.com) wrote: > Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for > flexibility (extending using subsections) and for fun. > > To achieve this we need to hack the config_vector, which is VirtIODevice > (that is common virtio) state, in the middle of the VirtioCcwDevice state > representation. This somewhat ugly, but we have no choice because the > stream format needs to be preserved. I don't think the hack you have here is any different to the existing code really; the flow is pretty much the same. > Almost no changes in behavior. Exception is everything that comes with > vmstate like extra bookkeeping about what's in the stream, and maybe some > extra checks and better error reporting. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> Without actually understanding any of the s390 magic, it does look like a faithful conversion, so: Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > > This patch is related to the series 'migration: s390x css migration', > in a sense that it's basically the compatibility preserving changes > split out as requested by Dave. > > Note: This patch can be split in two parts, where the first part is > limited to getting rid of the qemu_put's and qemu_gets from > subch_device_save and subch_device_load. With that the scope of the > second part would be concerned with the virtio_ccw_save_config and > virtio_ccw_load_config. Would it significantly benefit readability > -- probably not. > --- > hw/intc/s390_flic.c | 28 ++++ > hw/s390x/ccw-device.c | 10 ++ > hw/s390x/ccw-device.h | 4 + > hw/s390x/css.c | 361 ++++++++++++++++++++++++++----------------- > hw/s390x/virtio-ccw.c | 154 +++++++++--------- > include/hw/s390x/css.h | 12 +- > include/hw/s390x/s390_flic.h | 5 + > 7 files changed, 354 insertions(+), 220 deletions(-) > > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c > index 711c11454f..7e7546a576 100644 > --- a/hw/intc/s390_flic.c > +++ b/hw/intc/s390_flic.c > @@ -18,6 +18,7 @@ > #include "trace.h" > #include "hw/qdev.h" > #include "qapi/error.h" > +#include "hw/s390x/s390-virtio-ccw.h" > > S390FLICState *s390_get_flic(void) > { > @@ -137,3 +138,30 @@ static void qemu_s390_flic_register_types(void) > } > > type_init(qemu_s390_flic_register_types) > + > +const VMStateDescription vmstate_adapter_info = { > + .name = "s390_adapter_info", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT64(ind_offset, AdapterInfo), > + /* > + * We do not have to migrate neither the id nor the addresses. > + * The id is set by css_register_io_adapter and the addresses > + * are set based on the IndAddr objects after those get mapped. > + */ > + VMSTATE_END_OF_LIST() > + }, > +}; > + > +const VMStateDescription vmstate_adapter_routes = { > + > + .name = "s390_adapter_routes", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT(adapter, AdapterRoutes, 1, vmstate_adapter_info, \ > + AdapterInfo), > + VMSTATE_END_OF_LIST() > + } > +}; > diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c > index fb8d640a7e..f9bfa154d6 100644 > --- a/hw/s390x/ccw-device.c > +++ b/hw/s390x/ccw-device.c > @@ -50,6 +50,16 @@ static void ccw_device_class_init(ObjectClass *klass, void *data) > dc->props = ccw_device_properties; > } > > +const VMStateDescription vmstate_ccw_dev = { > + .name = "s390_ccw_dev", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT_POINTER(sch, CcwDevice, vmstate_subch_dev, SubchDev), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const TypeInfo ccw_device_info = { > .name = TYPE_CCW_DEVICE, > .parent = TYPE_DEVICE, > diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h > index 89c8e5dff7..4e6af287e7 100644 > --- a/hw/s390x/ccw-device.h > +++ b/hw/s390x/ccw-device.h > @@ -27,6 +27,10 @@ typedef struct CcwDevice { > CssDevId subch_id; > } CcwDevice; > > +extern const VMStateDescription vmstate_ccw_dev; > +#define VMSTATE_CCW_DEVICE(_field, _state) \ > + VMSTATE_STRUCT(_field, _state, 1, vmstate_ccw_dev, CcwDevice) > + > typedef struct CCWDeviceClass { > DeviceClass parent_class; > void (*unplug)(HotplugHandler *, DeviceState *, Error **); > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 15c4f4b249..15517a16e4 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -20,6 +20,7 @@ > #include "hw/s390x/css.h" > #include "trace.h" > #include "hw/s390x/s390_flic.h" > +#include "hw/s390x/s390-virtio-ccw.h" > > typedef struct CrwContainer { > CRW crw; > @@ -38,6 +39,177 @@ typedef struct SubchSet { > unsigned long devnos_used[BITS_TO_LONGS(MAX_SCHID + 1)]; > } SubchSet; > > +static const VMStateDescription vmstate_scsw = { > + .name = "s390_scsw", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT16(flags, SCSW), > + VMSTATE_UINT16(ctrl, SCSW), > + VMSTATE_UINT32(cpa, SCSW), > + VMSTATE_UINT8(dstat, SCSW), > + VMSTATE_UINT8(cstat, SCSW), > + VMSTATE_UINT16(count, SCSW), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_pmcw = { > + .name = "s390_pmcw", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(intparm, PMCW), > + VMSTATE_UINT16(flags, PMCW), > + VMSTATE_UINT16(devno, PMCW), > + VMSTATE_UINT8(lpm, PMCW), > + VMSTATE_UINT8(pnom, PMCW), > + VMSTATE_UINT8(lpum, PMCW), > + VMSTATE_UINT8(pim, PMCW), > + VMSTATE_UINT16(mbi, PMCW), > + VMSTATE_UINT8(pom, PMCW), > + VMSTATE_UINT8(pam, PMCW), > + VMSTATE_UINT8_ARRAY(chpid, PMCW, 8), > + VMSTATE_UINT32(chars, PMCW), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_schib = { > + .name = "s390_schib", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT(pmcw, SCHIB, 0, vmstate_pmcw, PMCW), > + VMSTATE_STRUCT(scsw, SCHIB, 0, vmstate_scsw, SCSW), > + VMSTATE_UINT64(mba, SCHIB), > + VMSTATE_UINT8_ARRAY(mda, SCHIB, 4), > + VMSTATE_END_OF_LIST() > + } > +}; > + > + > +static const VMStateDescription vmstate_ccw1 = { > + .name = "s390_ccw1", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(cmd_code, CCW1), > + VMSTATE_UINT8(flags, CCW1), > + VMSTATE_UINT16(count, CCW1), > + VMSTATE_UINT32(cda, CCW1), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_ciw = { > + .name = "s390_ciw", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(type, CIW), > + VMSTATE_UINT8(command, CIW), > + VMSTATE_UINT16(count, CIW), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_sense_id = { > + .name = "s390_sense_id", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(reserved, SenseId), > + VMSTATE_UINT16(cu_type, SenseId), > + VMSTATE_UINT8(cu_model, SenseId), > + VMSTATE_UINT16(dev_type, SenseId), > + VMSTATE_UINT8(dev_model, SenseId), > + VMSTATE_UINT8(unused, SenseId), > + VMSTATE_STRUCT_ARRAY(ciw, SenseId, MAX_CIWS, 0, vmstate_ciw, CIW), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static int subch_dev_post_load(void *opaque, int version_id); > +static void subch_dev_pre_save(void *opaque); > + > +const VMStateDescription vmstate_subch_dev = { > + .name = "s390_subch_dev", > + .version_id = 1, > + .minimum_version_id = 1, > + .post_load = subch_dev_post_load, > + .pre_save = subch_dev_pre_save, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8_EQUAL(cssid, SubchDev), > + VMSTATE_UINT8_EQUAL(ssid, SubchDev), > + VMSTATE_UINT16(migrated_schid, SubchDev), > + VMSTATE_UINT16(devno, SubchDev), > + VMSTATE_BOOL(thinint_active, SubchDev), > + VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB), > + VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32), > + VMSTATE_UINT64(channel_prog, SubchDev), > + VMSTATE_STRUCT(last_cmd, SubchDev, 0, vmstate_ccw1, CCW1), > + VMSTATE_BOOL(last_cmd_valid, SubchDev), > + VMSTATE_STRUCT(id, SubchDev, 0, vmstate_sense_id, SenseId), > + VMSTATE_BOOL(ccw_fmt_1, SubchDev), > + VMSTATE_UINT8(ccw_no_data_cnt, SubchDev), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +typedef struct IndAddrPtrTmp { > + IndAddr **parent; > + uint64_t addr; > + int32_t len; > +} IndAddrPtrTmp; > + > +static int post_load_ind_addr(void *opaque, int version_id) > +{ > + IndAddrPtrTmp *ptmp = opaque; > + IndAddr **ind_addr = ptmp->parent; > + > + if (ptmp->len != 0) { > + *ind_addr = get_indicator(ptmp->addr, ptmp->len); > + } else { > + *ind_addr = NULL; > + } > + return 0; > +} > + > +static void pre_save_ind_addr(void *opaque) > +{ > + IndAddrPtrTmp *ptmp = opaque; > + IndAddr *ind_addr = *(ptmp->parent); > + > + if (ind_addr != NULL) { > + ptmp->len = ind_addr->len; > + ptmp->addr = ind_addr->addr; > + } else { > + ptmp->len = 0; > + ptmp->addr = 0L; > + } > +} > + > +const VMStateDescription vmstate_ind_addr_tmp = { > + .name = "s390_ind_addr_tmp", > + .pre_save = pre_save_ind_addr, > + .post_load = post_load_ind_addr, > + > + .fields = (VMStateField[]) { > + VMSTATE_INT32(len, IndAddrPtrTmp), > + VMSTATE_UINT64(addr, IndAddrPtrTmp), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +const VMStateDescription vmstate_ind_addr = { > + .name = "s390_ind_addr_tmp", > + .fields = (VMStateField[]) { > + VMSTATE_WITH_TMP(IndAddr*, IndAddrPtrTmp, vmstate_ind_addr_tmp), > + VMSTATE_END_OF_LIST() > + } > +}; > + > typedef struct CssImage { > SubchSet *sch_set[MAX_SSID + 1]; > ChpInfo chpids[MAX_CHPID + 1]; > @@ -75,6 +247,52 @@ static ChannelSubSys channel_subsys = { > QTAILQ_HEAD_INITIALIZER(channel_subsys.indicator_addresses), > }; > > +static void subch_dev_pre_save(void *opaque) > +{ > + SubchDev *s = opaque; > + > + /* Prepare remote_schid for save */ > + s->migrated_schid = s->schid; > +} > + > +static int subch_dev_post_load(void *opaque, int version_id) > +{ > + > + SubchDev *s = opaque; > + > + /* Re-assign the subchannel to remote_schid if necessary */ > + if (s->migrated_schid != s->schid) { > + if (css_find_subch(true, s->cssid, s->ssid, s->schid) == s) { > + /* > + * Cleanup the slot before moving to s->migrated_schid provided > + * it still belongs to us, i.e. it was not changed by previous > + * invocation of this function. > + */ > + css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, NULL); > + } > + /* It's OK to re-assign without a prior de-assign. */ > + s->schid = s->migrated_schid; > + css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s); > + } > + > + /* > + * Hack alert. If we don't migrate the channel subsystem status > + * we still need to find out if the guest enabled mss/mcss-e. > + * If the subchannel is enabled, it certainly was able to access it, > + * so adjust the max_ssid/max_cssid values for relevant ssid/cssid > + * values. This is not watertight, but better than nothing. > + */ > + if (s->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA) { > + if (s->ssid) { > + channel_subsys.max_ssid = MAX_SSID; > + } > + if (s->cssid != channel_subsys.default_cssid) { > + channel_subsys.max_cssid = MAX_CSSID; > + } > + } > + return 0; > +} > + > IndAddr *get_indicator(hwaddr ind_addr, int len) > { > IndAddr *indicator; > @@ -1662,149 +1880,6 @@ int css_enable_mss(void) > return 0; > } > > -void subch_device_save(SubchDev *s, QEMUFile *f) > -{ > - int i; > - > - qemu_put_byte(f, s->cssid); > - qemu_put_byte(f, s->ssid); > - qemu_put_be16(f, s->schid); > - qemu_put_be16(f, s->devno); > - qemu_put_byte(f, s->thinint_active); > - /* SCHIB */ > - /* PMCW */ > - qemu_put_be32(f, s->curr_status.pmcw.intparm); > - qemu_put_be16(f, s->curr_status.pmcw.flags); > - qemu_put_be16(f, s->curr_status.pmcw.devno); > - qemu_put_byte(f, s->curr_status.pmcw.lpm); > - qemu_put_byte(f, s->curr_status.pmcw.pnom); > - qemu_put_byte(f, s->curr_status.pmcw.lpum); > - qemu_put_byte(f, s->curr_status.pmcw.pim); > - qemu_put_be16(f, s->curr_status.pmcw.mbi); > - qemu_put_byte(f, s->curr_status.pmcw.pom); > - qemu_put_byte(f, s->curr_status.pmcw.pam); > - qemu_put_buffer(f, s->curr_status.pmcw.chpid, 8); > - qemu_put_be32(f, s->curr_status.pmcw.chars); > - /* SCSW */ > - qemu_put_be16(f, s->curr_status.scsw.flags); > - qemu_put_be16(f, s->curr_status.scsw.ctrl); > - qemu_put_be32(f, s->curr_status.scsw.cpa); > - qemu_put_byte(f, s->curr_status.scsw.dstat); > - qemu_put_byte(f, s->curr_status.scsw.cstat); > - qemu_put_be16(f, s->curr_status.scsw.count); > - qemu_put_be64(f, s->curr_status.mba); > - qemu_put_buffer(f, s->curr_status.mda, 4); > - /* end SCHIB */ > - qemu_put_buffer(f, s->sense_data, 32); > - qemu_put_be64(f, s->channel_prog); > - /* last cmd */ > - qemu_put_byte(f, s->last_cmd.cmd_code); > - qemu_put_byte(f, s->last_cmd.flags); > - qemu_put_be16(f, s->last_cmd.count); > - qemu_put_be32(f, s->last_cmd.cda); > - qemu_put_byte(f, s->last_cmd_valid); > - qemu_put_byte(f, s->id.reserved); > - qemu_put_be16(f, s->id.cu_type); > - qemu_put_byte(f, s->id.cu_model); > - qemu_put_be16(f, s->id.dev_type); > - qemu_put_byte(f, s->id.dev_model); > - qemu_put_byte(f, s->id.unused); > - for (i = 0; i < ARRAY_SIZE(s->id.ciw); i++) { > - qemu_put_byte(f, s->id.ciw[i].type); > - qemu_put_byte(f, s->id.ciw[i].command); > - qemu_put_be16(f, s->id.ciw[i].count); > - } > - qemu_put_byte(f, s->ccw_fmt_1); > - qemu_put_byte(f, s->ccw_no_data_cnt); > -} > - > -int subch_device_load(SubchDev *s, QEMUFile *f) > -{ > - SubchDev *old_s; > - uint16_t old_schid = s->schid; > - int i; > - > - s->cssid = qemu_get_byte(f); > - s->ssid = qemu_get_byte(f); > - s->schid = qemu_get_be16(f); > - s->devno = qemu_get_be16(f); > - /* Re-assign subch. */ > - if (old_schid != s->schid) { > - old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid]; > - /* > - * (old_s != s) means that some other device has its correct > - * subchannel already assigned (in load). > - */ > - if (old_s == s) { > - css_subch_assign(s->cssid, s->ssid, old_schid, s->devno, NULL); > - } > - /* It's OK to re-assign without a prior de-assign. */ > - css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s); > - } > - s->thinint_active = qemu_get_byte(f); > - /* SCHIB */ > - /* PMCW */ > - s->curr_status.pmcw.intparm = qemu_get_be32(f); > - s->curr_status.pmcw.flags = qemu_get_be16(f); > - s->curr_status.pmcw.devno = qemu_get_be16(f); > - s->curr_status.pmcw.lpm = qemu_get_byte(f); > - s->curr_status.pmcw.pnom = qemu_get_byte(f); > - s->curr_status.pmcw.lpum = qemu_get_byte(f); > - s->curr_status.pmcw.pim = qemu_get_byte(f); > - s->curr_status.pmcw.mbi = qemu_get_be16(f); > - s->curr_status.pmcw.pom = qemu_get_byte(f); > - s->curr_status.pmcw.pam = qemu_get_byte(f); > - qemu_get_buffer(f, s->curr_status.pmcw.chpid, 8); > - s->curr_status.pmcw.chars = qemu_get_be32(f); > - /* SCSW */ > - s->curr_status.scsw.flags = qemu_get_be16(f); > - s->curr_status.scsw.ctrl = qemu_get_be16(f); > - s->curr_status.scsw.cpa = qemu_get_be32(f); > - s->curr_status.scsw.dstat = qemu_get_byte(f); > - s->curr_status.scsw.cstat = qemu_get_byte(f); > - s->curr_status.scsw.count = qemu_get_be16(f); > - s->curr_status.mba = qemu_get_be64(f); > - qemu_get_buffer(f, s->curr_status.mda, 4); > - /* end SCHIB */ > - qemu_get_buffer(f, s->sense_data, 32); > - s->channel_prog = qemu_get_be64(f); > - /* last cmd */ > - s->last_cmd.cmd_code = qemu_get_byte(f); > - s->last_cmd.flags = qemu_get_byte(f); > - s->last_cmd.count = qemu_get_be16(f); > - s->last_cmd.cda = qemu_get_be32(f); > - s->last_cmd_valid = qemu_get_byte(f); > - s->id.reserved = qemu_get_byte(f); > - s->id.cu_type = qemu_get_be16(f); > - s->id.cu_model = qemu_get_byte(f); > - s->id.dev_type = qemu_get_be16(f); > - s->id.dev_model = qemu_get_byte(f); > - s->id.unused = qemu_get_byte(f); > - for (i = 0; i < ARRAY_SIZE(s->id.ciw); i++) { > - s->id.ciw[i].type = qemu_get_byte(f); > - s->id.ciw[i].command = qemu_get_byte(f); > - s->id.ciw[i].count = qemu_get_be16(f); > - } > - s->ccw_fmt_1 = qemu_get_byte(f); > - s->ccw_no_data_cnt = qemu_get_byte(f); > - /* > - * Hack alert. We don't migrate the channel subsystem status (no > - * device!), but we need to find out if the guest enabled mss/mcss-e. > - * If the subchannel is enabled, it certainly was able to access it, > - * so adjust the max_ssid/max_cssid values for relevant ssid/cssid > - * values. This is not watertight, but better than nothing. > - */ > - if (s->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA) { > - if (s->ssid) { > - channel_subsys.max_ssid = MAX_SSID; > - } > - if (s->cssid != channel_subsys.default_cssid) { > - channel_subsys.max_cssid = MAX_CSSID; > - } > - } > - return 0; > -} > - > void css_reset_sch(SubchDev *sch) > { > PMCW *p = &sch->curr_status.pmcw; > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index e7167e3d05..a75eedeffb 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -34,9 +34,87 @@ > #include "virtio-ccw.h" > #include "trace.h" > #include "hw/s390x/css-bridge.h" > +#include "hw/s390x/s390-virtio-ccw.h" > > #define NR_CLASSIC_INDICATOR_BITS 64 > > +static int virtio_ccw_dev_post_load(void *opaque, int version_id) > +{ > + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(opaque); > + CcwDevice *ccw_dev = CCW_DEVICE(dev); > + CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); > + > + ccw_dev->sch->driver_data = dev; > + if (CCW_DEVICE(dev)->sch->thinint_active) { > + dev->routes.adapter.adapter_id = css_get_adapter_id( > + CSS_IO_ADAPTER_VIRTIO, > + dev->thinint_isc); > + } > + /* Re-fill subch_id after loading the subchannel states.*/ > + if (ck->refill_ids) { > + ck->refill_ids(ccw_dev); > + } > + return 0; > +} > + > +typedef struct VirtioCcwDeviceTmp { > + VirtioCcwDevice *parent; > + uint16_t config_vector; > +} VirtioCcwDeviceTmp; > + > +static void virtio_ccw_dev_tmp_pre_save(void *opaque) > +{ > + VirtioCcwDeviceTmp *tmp = opaque; > + VirtioCcwDevice *dev = tmp->parent; > + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); > + > + tmp->config_vector = vdev->config_vector; > +} > + > +static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id) > +{ > + VirtioCcwDeviceTmp *tmp = opaque; > + VirtioCcwDevice *dev = tmp->parent; > + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); > + > + vdev->config_vector = tmp->config_vector; > + return 0; > +} > + > +const VMStateDescription vmstate_virtio_ccw_dev_tmp = { > + .name = "s390_virtio_ccw_dev_tmp", > + .pre_save = virtio_ccw_dev_tmp_pre_save, > + .post_load = virtio_ccw_dev_tmp_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +const VMStateDescription vmstate_virtio_ccw_dev = { > + .name = "s390_virtio_ccw_dev", > + .version_id = 1, > + .minimum_version_id = 1, > + .post_load = virtio_ccw_dev_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice), > + VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), > + VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), > + VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), > + /* > + * Ugly hack because VirtIODevice does not migrate itself. > + * This also makes legacy via vmstate_save_state possible. > + */ > + VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp, > + vmstate_virtio_ccw_dev_tmp), > + VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \ > + AdapterRoutes), > + VMSTATE_UINT8(thinint_isc, VirtioCcwDevice), > + VMSTATE_INT32(revision, VirtioCcwDevice), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size, > VirtioCcwDevice *dev); > > @@ -1234,85 +1312,13 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f) > static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) > { > VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > - CcwDevice *ccw_dev = CCW_DEVICE(d); > - SubchDev *s = ccw_dev->sch; > - VirtIODevice *vdev = virtio_ccw_get_vdev(s); > - > - subch_device_save(s, f); > - if (dev->indicators != NULL) { > - qemu_put_be32(f, dev->indicators->len); > - qemu_put_be64(f, dev->indicators->addr); > - } else { > - qemu_put_be32(f, 0); > - qemu_put_be64(f, 0UL); > - } > - if (dev->indicators2 != NULL) { > - qemu_put_be32(f, dev->indicators2->len); > - qemu_put_be64(f, dev->indicators2->addr); > - } else { > - qemu_put_be32(f, 0); > - qemu_put_be64(f, 0UL); > - } > - if (dev->summary_indicator != NULL) { > - qemu_put_be32(f, dev->summary_indicator->len); > - qemu_put_be64(f, dev->summary_indicator->addr); > - } else { > - qemu_put_be32(f, 0); > - qemu_put_be64(f, 0UL); > - } > - qemu_put_be16(f, vdev->config_vector); > - qemu_put_be64(f, dev->routes.adapter.ind_offset); > - qemu_put_byte(f, dev->thinint_isc); > - qemu_put_be32(f, dev->revision); > + vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL); > } > > static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) > { > VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > - CcwDevice *ccw_dev = CCW_DEVICE(d); > - CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); > - SubchDev *s = ccw_dev->sch; > - VirtIODevice *vdev = virtio_ccw_get_vdev(s); > - int len; > - > - s->driver_data = dev; > - subch_device_load(s, f); > - /* Re-fill subch_id after loading the subchannel states.*/ > - if (ck->refill_ids) { > - ck->refill_ids(ccw_dev); > - } > - len = qemu_get_be32(f); > - if (len != 0) { > - dev->indicators = get_indicator(qemu_get_be64(f), len); > - } else { > - qemu_get_be64(f); > - dev->indicators = NULL; > - } > - len = qemu_get_be32(f); > - if (len != 0) { > - dev->indicators2 = get_indicator(qemu_get_be64(f), len); > - } else { > - qemu_get_be64(f); > - dev->indicators2 = NULL; > - } > - len = qemu_get_be32(f); > - if (len != 0) { > - dev->summary_indicator = get_indicator(qemu_get_be64(f), len); > - } else { > - qemu_get_be64(f); > - dev->summary_indicator = NULL; > - } > - qemu_get_be16s(f, &vdev->config_vector); > - dev->routes.adapter.ind_offset = qemu_get_be64(f); > - dev->thinint_isc = qemu_get_byte(f); > - dev->revision = qemu_get_be32(f); > - if (s->thinint_active) { > - dev->routes.adapter.adapter_id = css_get_adapter_id( > - CSS_IO_ADAPTER_VIRTIO, > - dev->thinint_isc); > - } > - > - return 0; > + return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1); > } > > static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) > diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h > index e61fa74d9b..d74e44d312 100644 > --- a/include/hw/s390x/css.h > +++ b/include/hw/s390x/css.h > @@ -88,6 +88,7 @@ struct SubchDev { > bool ccw_fmt_1; > bool thinint_active; > uint8_t ccw_no_data_cnt; > + uint16_t migrated_schid; /* used for missmatch detection */ > /* transport-provided data: */ > int (*ccw_cb) (SubchDev *, CCW1); > void (*disable_cb)(SubchDev *); > @@ -95,22 +96,27 @@ struct SubchDev { > void *driver_data; > }; > > +extern const VMStateDescription vmstate_subch_dev; > + > typedef struct IndAddr { > hwaddr addr; > uint64_t map; > unsigned long refcnt; > - int len; > + int32_t len; > QTAILQ_ENTRY(IndAddr) sibling; > } IndAddr; > > +extern const VMStateDescription vmstate_ind_addr; > + > +#define VMSTATE_PTR_TO_IND_ADDR(_f, _s) \ > + VMSTATE_STRUCT(_f, _s, 1, vmstate_ind_addr, IndAddr*) > + > IndAddr *get_indicator(hwaddr ind_addr, int len); > void release_indicator(AdapterInfo *adapter, IndAddr *indicator); > int map_indicator(AdapterInfo *adapter, IndAddr *indicator); > > typedef SubchDev *(*css_subch_cb_func)(uint8_t m, uint8_t cssid, uint8_t ssid, > uint16_t schid); > -void subch_device_save(SubchDev *s, QEMUFile *f); > -int subch_device_load(SubchDev *s, QEMUFile *f); > int css_create_css_image(uint8_t cssid, bool default_image); > bool css_devno_used(uint8_t cssid, uint8_t ssid, uint16_t devno); > void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, > diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h > index f9e6890c90..caa6fc608d 100644 > --- a/include/hw/s390x/s390_flic.h > +++ b/include/hw/s390x/s390_flic.h > @@ -31,6 +31,11 @@ typedef struct AdapterRoutes { > int gsi[ADAPTER_ROUTES_MAX_GSI]; > } AdapterRoutes; > > +extern const VMStateDescription vmstate_adapter_routes; > + > +#define VMSTATE_ADAPTER_ROUTES(_f, _s) \ > + VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes) > + > #define TYPE_S390_FLIC_COMMON "s390-flic" > #define S390_FLIC_COMMON(obj) \ > OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON) > -- > 2.11.2 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 05/31/2017 06:00 PM, Dr. David Alan Gilbert wrote: > * Halil Pasic (pasic@linux.vnet.ibm.com) wrote: >> Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for >> flexibility (extending using subsections) and for fun. >> >> To achieve this we need to hack the config_vector, which is VirtIODevice >> (that is common virtio) state, in the middle of the VirtioCcwDevice state >> representation. This somewhat ugly, but we have no choice because the s/This somewhat/This is somewhat >> stream format needs to be preserved. > > I don't think the hack you have here is any different to the existing > code really; the flow is pretty much the same. > I agree, it's the best we can do, but still a qdev device migrating the state of some other qdev device is ugly to me. I prefer keeping the explanation, as neither what has to be done, nor how it is done is really straight forward. If you recommend dropping the sentence I can do that to. >> Almost no changes in behavior. Exception is everything that comes with >> vmstate like extra bookkeeping about what's in the stream, and maybe some >> extra checks and better error reporting. >> >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > > Without actually understanding any of the s390 magic, it does look > like a faithful conversion, so: > > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Thanks a lot! I guess Connie is going to have to take the responsibility and review the s390 magic. This has to go trough her tree anyway. The eyes of an migration expert are highly appreciated (even if its s390 only code)! Will wait for Connie and probably re-spin with your r-b and the above typo fixed. Cheers, Halil
Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for > flexibility (extending using subsections) and for fun. > > To achieve this we need to hack the config_vector, which is VirtIODevice > (that is common virtio) state, in the middle of the VirtioCcwDevice state > representation. This somewhat ugly, but we have no choice because the > stream format needs to be preserved. > > Almost no changes in behavior. Exception is everything that comes with > vmstate like extra bookkeeping about what's in the stream, and maybe some > extra checks and better error reporting. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> Reviewed-by: Juan Quintela <quintela@redhat.com> > +static void subch_dev_pre_save(void *opaque) > +{ > + SubchDev *s = opaque; > + > + /* Prepare remote_schid for save */ > + s->migrated_schid = s->schid; > +} > + > +static int subch_dev_post_load(void *opaque, int version_id) > +{ > + > + SubchDev *s = opaque; > + > + /* Re-assign the subchannel to remote_schid if necessary */ > + if (s->migrated_schid != s->schid) { > + if (css_find_subch(true, s->cssid, s->ssid, s->schid) == s) { I am assuming this is somehow similar to old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid]; > - qemu_put_be32(f, s->curr_status.pmcw.intparm); > - qemu_put_be16(f, s->curr_status.pmcw.flags); > - qemu_put_be16(f, s->curr_status.pmcw.devno); > - qemu_put_byte(f, s->curr_status.pmcw.lpm); > - qemu_put_byte(f, s->curr_status.pmcw.pnom); > - qemu_put_byte(f, s->curr_status.pmcw.lpum); > - qemu_put_byte(f, s->curr_status.pmcw.pim); > - qemu_put_be16(f, s->curr_status.pmcw.mbi); > - qemu_put_byte(f, s->curr_status.pmcw.pom); > - qemu_put_byte(f, s->curr_status.pmcw.pam); I hope it somehow makes sense, I am having trouble following that you have fields named: pim, pam, pom, pnom, lpm, lpum, mda, mba ..... looks like hell for reviewing O:-) And I thought that x86 was weird because it used all three letters acronyms O:-) Later, Juan.
On 05/31/2017 08:13 PM, Juan Quintela wrote: > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >> Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for >> flexibility (extending using subsections) and for fun. >> >> To achieve this we need to hack the config_vector, which is VirtIODevice >> (that is common virtio) state, in the middle of the VirtioCcwDevice state >> representation. This somewhat ugly, but we have no choice because the >> stream format needs to be preserved. >> >> Almost no changes in behavior. Exception is everything that comes with >> vmstate like extra bookkeeping about what's in the stream, and maybe some >> extra checks and better error reporting. >> >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > > Reviewed-by: Juan Quintela <quintela@redhat.com> > >> +static void subch_dev_pre_save(void *opaque) >> +{ >> + SubchDev *s = opaque; >> + >> + /* Prepare remote_schid for save */ >> + s->migrated_schid = s->schid; >> +} >> + >> +static int subch_dev_post_load(void *opaque, int version_id) >> +{ >> + >> + SubchDev *s = opaque; >> + >> + /* Re-assign the subchannel to remote_schid if necessary */ >> + if (s->migrated_schid != s->schid) { >> + if (css_find_subch(true, s->cssid, s->ssid, s->schid) == s) { > > I am assuming this is somehow similar to > old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid]; > That's right. A quick glance at the return statement(s) of css_find_subch makes it very obvious. But css_find_subch does some null checks and may differently for cssid == 0 (which does not matter here). >> - qemu_put_be32(f, s->curr_status.pmcw.intparm); >> - qemu_put_be16(f, s->curr_status.pmcw.flags); >> - qemu_put_be16(f, s->curr_status.pmcw.devno); >> - qemu_put_byte(f, s->curr_status.pmcw.lpm); >> - qemu_put_byte(f, s->curr_status.pmcw.pnom); >> - qemu_put_byte(f, s->curr_status.pmcw.lpum); >> - qemu_put_byte(f, s->curr_status.pmcw.pim); >> - qemu_put_be16(f, s->curr_status.pmcw.mbi); >> - qemu_put_byte(f, s->curr_status.pmcw.pom); >> - qemu_put_byte(f, s->curr_status.pmcw.pam); > > I hope it somehow makes sense, I am having trouble following that you > have fields named: pim, pam, pom, pnom, lpm, lpum, mda, mba ..... looks > like hell for reviewing O:-) > > And I thought that x86 was weird because it used all three letters > acronyms > > O:-) nod > > Later, Juan. > Many thanks for the review and the r-b! Regards, Halil
On Mon, 29 May 2017 15:17:16 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for > flexibility (extending using subsections) and for fun. You have interesting ideas of fun :) > > To achieve this we need to hack the config_vector, which is VirtIODevice > (that is common virtio) state, in the middle of the VirtioCcwDevice state > representation. This somewhat ugly, but we have no choice because the > stream format needs to be preserved. > > Almost no changes in behavior. Exception is everything that comes with > vmstate like extra bookkeeping about what's in the stream, and maybe some > extra checks and better error reporting. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > --- > > This patch is related to the series 'migration: s390x css migration', > in a sense that it's basically the compatibility preserving changes > split out as requested by Dave. > > Note: This patch can be split in two parts, where the first part is > limited to getting rid of the qemu_put's and qemu_gets from > subch_device_save and subch_device_load. With that the scope of the > second part would be concerned with the virtio_ccw_save_config and > virtio_ccw_load_config. Would it significantly benefit readability > -- probably not. Agreed, probably not. You still have to dig through a zoo of acronyms... > --- > hw/intc/s390_flic.c | 28 ++++ > hw/s390x/ccw-device.c | 10 ++ > hw/s390x/ccw-device.h | 4 + > hw/s390x/css.c | 361 ++++++++++++++++++++++++++----------------- > hw/s390x/virtio-ccw.c | 154 +++++++++--------- > include/hw/s390x/css.h | 12 +- > include/hw/s390x/s390_flic.h | 5 + > 7 files changed, 354 insertions(+), 220 deletions(-) > > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c > index 711c11454f..7e7546a576 100644 > --- a/hw/intc/s390_flic.c > +++ b/hw/intc/s390_flic.c > @@ -18,6 +18,7 @@ > #include "trace.h" > #include "hw/qdev.h" > #include "qapi/error.h" > +#include "hw/s390x/s390-virtio-ccw.h" > > S390FLICState *s390_get_flic(void) > { > @@ -137,3 +138,30 @@ static void qemu_s390_flic_register_types(void) > } > > type_init(qemu_s390_flic_register_types) > + > +const VMStateDescription vmstate_adapter_info = { > + .name = "s390_adapter_info", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT64(ind_offset, AdapterInfo), > + /* > + * We do not have to migrate neither the id nor the addresses. > + * The id is set by css_register_io_adapter and the addresses > + * are set based on the IndAddr objects after those get mapped. > + */ > + VMSTATE_END_OF_LIST() > + }, > +}; > + > +const VMStateDescription vmstate_adapter_routes = { > + > + .name = "s390_adapter_routes", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT(adapter, AdapterRoutes, 1, vmstate_adapter_info, \ > + AdapterInfo), > + VMSTATE_END_OF_LIST() > + } > +}; > diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c > index fb8d640a7e..f9bfa154d6 100644 > --- a/hw/s390x/ccw-device.c > +++ b/hw/s390x/ccw-device.c > @@ -50,6 +50,16 @@ static void ccw_device_class_init(ObjectClass *klass, void *data) > dc->props = ccw_device_properties; > } > > +const VMStateDescription vmstate_ccw_dev = { > + .name = "s390_ccw_dev", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT_POINTER(sch, CcwDevice, vmstate_subch_dev, SubchDev), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const TypeInfo ccw_device_info = { > .name = TYPE_CCW_DEVICE, > .parent = TYPE_DEVICE, > diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h > index 89c8e5dff7..4e6af287e7 100644 > --- a/hw/s390x/ccw-device.h > +++ b/hw/s390x/ccw-device.h > @@ -27,6 +27,10 @@ typedef struct CcwDevice { > CssDevId subch_id; > } CcwDevice; > > +extern const VMStateDescription vmstate_ccw_dev; > +#define VMSTATE_CCW_DEVICE(_field, _state) \ > + VMSTATE_STRUCT(_field, _state, 1, vmstate_ccw_dev, CcwDevice) > + > typedef struct CCWDeviceClass { > DeviceClass parent_class; > void (*unplug)(HotplugHandler *, DeviceState *, Error **); > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 15c4f4b249..15517a16e4 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -20,6 +20,7 @@ > #include "hw/s390x/css.h" > #include "trace.h" > #include "hw/s390x/s390_flic.h" > +#include "hw/s390x/s390-virtio-ccw.h" > > typedef struct CrwContainer { > CRW crw; > @@ -38,6 +39,177 @@ typedef struct SubchSet { > unsigned long devnos_used[BITS_TO_LONGS(MAX_SCHID + 1)]; > } SubchSet; > > +static const VMStateDescription vmstate_scsw = { > + .name = "s390_scsw", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT16(flags, SCSW), > + VMSTATE_UINT16(ctrl, SCSW), > + VMSTATE_UINT32(cpa, SCSW), > + VMSTATE_UINT8(dstat, SCSW), > + VMSTATE_UINT8(cstat, SCSW), > + VMSTATE_UINT16(count, SCSW), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_pmcw = { > + .name = "s390_pmcw", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(intparm, PMCW), > + VMSTATE_UINT16(flags, PMCW), > + VMSTATE_UINT16(devno, PMCW), > + VMSTATE_UINT8(lpm, PMCW), > + VMSTATE_UINT8(pnom, PMCW), > + VMSTATE_UINT8(lpum, PMCW), > + VMSTATE_UINT8(pim, PMCW), > + VMSTATE_UINT16(mbi, PMCW), > + VMSTATE_UINT8(pom, PMCW), > + VMSTATE_UINT8(pam, PMCW), > + VMSTATE_UINT8_ARRAY(chpid, PMCW, 8), > + VMSTATE_UINT32(chars, PMCW), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_schib = { > + .name = "s390_schib", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT(pmcw, SCHIB, 0, vmstate_pmcw, PMCW), > + VMSTATE_STRUCT(scsw, SCHIB, 0, vmstate_scsw, SCSW), > + VMSTATE_UINT64(mba, SCHIB), > + VMSTATE_UINT8_ARRAY(mda, SCHIB, 4), > + VMSTATE_END_OF_LIST() > + } > +}; > + > + > +static const VMStateDescription vmstate_ccw1 = { > + .name = "s390_ccw1", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(cmd_code, CCW1), > + VMSTATE_UINT8(flags, CCW1), > + VMSTATE_UINT16(count, CCW1), > + VMSTATE_UINT32(cda, CCW1), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_ciw = { > + .name = "s390_ciw", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(type, CIW), > + VMSTATE_UINT8(command, CIW), > + VMSTATE_UINT16(count, CIW), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_sense_id = { > + .name = "s390_sense_id", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(reserved, SenseId), > + VMSTATE_UINT16(cu_type, SenseId), > + VMSTATE_UINT8(cu_model, SenseId), > + VMSTATE_UINT16(dev_type, SenseId), > + VMSTATE_UINT8(dev_model, SenseId), > + VMSTATE_UINT8(unused, SenseId), This is strictly due to stream format preservation, right? > + VMSTATE_STRUCT_ARRAY(ciw, SenseId, MAX_CIWS, 0, vmstate_ciw, CIW), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static int subch_dev_post_load(void *opaque, int version_id); > +static void subch_dev_pre_save(void *opaque); > + > +const VMStateDescription vmstate_subch_dev = { > + .name = "s390_subch_dev", > + .version_id = 1, > + .minimum_version_id = 1, > + .post_load = subch_dev_post_load, > + .pre_save = subch_dev_pre_save, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8_EQUAL(cssid, SubchDev), > + VMSTATE_UINT8_EQUAL(ssid, SubchDev), > + VMSTATE_UINT16(migrated_schid, SubchDev), > + VMSTATE_UINT16(devno, SubchDev), > + VMSTATE_BOOL(thinint_active, SubchDev), > + VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB), > + VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32), > + VMSTATE_UINT64(channel_prog, SubchDev), > + VMSTATE_STRUCT(last_cmd, SubchDev, 0, vmstate_ccw1, CCW1), > + VMSTATE_BOOL(last_cmd_valid, SubchDev), > + VMSTATE_STRUCT(id, SubchDev, 0, vmstate_sense_id, SenseId), > + VMSTATE_BOOL(ccw_fmt_1, SubchDev), > + VMSTATE_UINT8(ccw_no_data_cnt, SubchDev), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +typedef struct IndAddrPtrTmp { > + IndAddr **parent; > + uint64_t addr; > + int32_t len; > +} IndAddrPtrTmp; > + > +static int post_load_ind_addr(void *opaque, int version_id) > +{ > + IndAddrPtrTmp *ptmp = opaque; > + IndAddr **ind_addr = ptmp->parent; > + > + if (ptmp->len != 0) { > + *ind_addr = get_indicator(ptmp->addr, ptmp->len); > + } else { > + *ind_addr = NULL; > + } > + return 0; > +} > + > +static void pre_save_ind_addr(void *opaque) > +{ > + IndAddrPtrTmp *ptmp = opaque; > + IndAddr *ind_addr = *(ptmp->parent); > + > + if (ind_addr != NULL) { > + ptmp->len = ind_addr->len; > + ptmp->addr = ind_addr->addr; > + } else { > + ptmp->len = 0; > + ptmp->addr = 0L; > + } > +} > + > +const VMStateDescription vmstate_ind_addr_tmp = { > + .name = "s390_ind_addr_tmp", > + .pre_save = pre_save_ind_addr, > + .post_load = post_load_ind_addr, > + > + .fields = (VMStateField[]) { > + VMSTATE_INT32(len, IndAddrPtrTmp), > + VMSTATE_UINT64(addr, IndAddrPtrTmp), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +const VMStateDescription vmstate_ind_addr = { > + .name = "s390_ind_addr_tmp", > + .fields = (VMStateField[]) { > + VMSTATE_WITH_TMP(IndAddr*, IndAddrPtrTmp, vmstate_ind_addr_tmp), > + VMSTATE_END_OF_LIST() > + } > +}; > + > typedef struct CssImage { > SubchSet *sch_set[MAX_SSID + 1]; > ChpInfo chpids[MAX_CHPID + 1]; > @@ -75,6 +247,52 @@ static ChannelSubSys channel_subsys = { > QTAILQ_HEAD_INITIALIZER(channel_subsys.indicator_addresses), > }; > > +static void subch_dev_pre_save(void *opaque) > +{ > + SubchDev *s = opaque; > + > + /* Prepare remote_schid for save */ > + s->migrated_schid = s->schid; > +} > + > +static int subch_dev_post_load(void *opaque, int version_id) > +{ > + > + SubchDev *s = opaque; > + > + /* Re-assign the subchannel to remote_schid if necessary */ > + if (s->migrated_schid != s->schid) { > + if (css_find_subch(true, s->cssid, s->ssid, s->schid) == s) { > + /* > + * Cleanup the slot before moving to s->migrated_schid provided > + * it still belongs to us, i.e. it was not changed by previous > + * invocation of this function. > + */ > + css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, NULL); > + } > + /* It's OK to re-assign without a prior de-assign. */ > + s->schid = s->migrated_schid; > + css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s); > + } > + > + /* > + * Hack alert. If we don't migrate the channel subsystem status > + * we still need to find out if the guest enabled mss/mcss-e. > + * If the subchannel is enabled, it certainly was able to access it, > + * so adjust the max_ssid/max_cssid values for relevant ssid/cssid > + * values. This is not watertight, but better than nothing. > + */ > + if (s->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA) { > + if (s->ssid) { > + channel_subsys.max_ssid = MAX_SSID; > + } > + if (s->cssid != channel_subsys.default_cssid) { > + channel_subsys.max_cssid = MAX_CSSID; > + } > + } > + return 0; > +} > + > IndAddr *get_indicator(hwaddr ind_addr, int len) > { > IndAddr *indicator; > @@ -1662,149 +1880,6 @@ int css_enable_mss(void) > return 0; > } > > -void subch_device_save(SubchDev *s, QEMUFile *f) > -{ > - int i; > - > - qemu_put_byte(f, s->cssid); > - qemu_put_byte(f, s->ssid); > - qemu_put_be16(f, s->schid); > - qemu_put_be16(f, s->devno); > - qemu_put_byte(f, s->thinint_active); > - /* SCHIB */ > - /* PMCW */ > - qemu_put_be32(f, s->curr_status.pmcw.intparm); > - qemu_put_be16(f, s->curr_status.pmcw.flags); > - qemu_put_be16(f, s->curr_status.pmcw.devno); > - qemu_put_byte(f, s->curr_status.pmcw.lpm); > - qemu_put_byte(f, s->curr_status.pmcw.pnom); > - qemu_put_byte(f, s->curr_status.pmcw.lpum); > - qemu_put_byte(f, s->curr_status.pmcw.pim); > - qemu_put_be16(f, s->curr_status.pmcw.mbi); > - qemu_put_byte(f, s->curr_status.pmcw.pom); > - qemu_put_byte(f, s->curr_status.pmcw.pam); > - qemu_put_buffer(f, s->curr_status.pmcw.chpid, 8); > - qemu_put_be32(f, s->curr_status.pmcw.chars); > - /* SCSW */ > - qemu_put_be16(f, s->curr_status.scsw.flags); > - qemu_put_be16(f, s->curr_status.scsw.ctrl); > - qemu_put_be32(f, s->curr_status.scsw.cpa); > - qemu_put_byte(f, s->curr_status.scsw.dstat); > - qemu_put_byte(f, s->curr_status.scsw.cstat); > - qemu_put_be16(f, s->curr_status.scsw.count); > - qemu_put_be64(f, s->curr_status.mba); > - qemu_put_buffer(f, s->curr_status.mda, 4); > - /* end SCHIB */ > - qemu_put_buffer(f, s->sense_data, 32); > - qemu_put_be64(f, s->channel_prog); > - /* last cmd */ > - qemu_put_byte(f, s->last_cmd.cmd_code); > - qemu_put_byte(f, s->last_cmd.flags); > - qemu_put_be16(f, s->last_cmd.count); > - qemu_put_be32(f, s->last_cmd.cda); > - qemu_put_byte(f, s->last_cmd_valid); > - qemu_put_byte(f, s->id.reserved); > - qemu_put_be16(f, s->id.cu_type); > - qemu_put_byte(f, s->id.cu_model); > - qemu_put_be16(f, s->id.dev_type); > - qemu_put_byte(f, s->id.dev_model); > - qemu_put_byte(f, s->id.unused); > - for (i = 0; i < ARRAY_SIZE(s->id.ciw); i++) { > - qemu_put_byte(f, s->id.ciw[i].type); > - qemu_put_byte(f, s->id.ciw[i].command); > - qemu_put_be16(f, s->id.ciw[i].count); > - } > - qemu_put_byte(f, s->ccw_fmt_1); > - qemu_put_byte(f, s->ccw_no_data_cnt); > -} > - > -int subch_device_load(SubchDev *s, QEMUFile *f) > -{ > - SubchDev *old_s; > - uint16_t old_schid = s->schid; > - int i; > - > - s->cssid = qemu_get_byte(f); > - s->ssid = qemu_get_byte(f); > - s->schid = qemu_get_be16(f); > - s->devno = qemu_get_be16(f); > - /* Re-assign subch. */ > - if (old_schid != s->schid) { > - old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid]; > - /* > - * (old_s != s) means that some other device has its correct > - * subchannel already assigned (in load). > - */ > - if (old_s == s) { > - css_subch_assign(s->cssid, s->ssid, old_schid, s->devno, NULL); > - } > - /* It's OK to re-assign without a prior de-assign. */ > - css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s); > - } > - s->thinint_active = qemu_get_byte(f); > - /* SCHIB */ > - /* PMCW */ > - s->curr_status.pmcw.intparm = qemu_get_be32(f); > - s->curr_status.pmcw.flags = qemu_get_be16(f); > - s->curr_status.pmcw.devno = qemu_get_be16(f); > - s->curr_status.pmcw.lpm = qemu_get_byte(f); > - s->curr_status.pmcw.pnom = qemu_get_byte(f); > - s->curr_status.pmcw.lpum = qemu_get_byte(f); > - s->curr_status.pmcw.pim = qemu_get_byte(f); > - s->curr_status.pmcw.mbi = qemu_get_be16(f); > - s->curr_status.pmcw.pom = qemu_get_byte(f); > - s->curr_status.pmcw.pam = qemu_get_byte(f); > - qemu_get_buffer(f, s->curr_status.pmcw.chpid, 8); > - s->curr_status.pmcw.chars = qemu_get_be32(f); > - /* SCSW */ > - s->curr_status.scsw.flags = qemu_get_be16(f); > - s->curr_status.scsw.ctrl = qemu_get_be16(f); > - s->curr_status.scsw.cpa = qemu_get_be32(f); > - s->curr_status.scsw.dstat = qemu_get_byte(f); > - s->curr_status.scsw.cstat = qemu_get_byte(f); > - s->curr_status.scsw.count = qemu_get_be16(f); > - s->curr_status.mba = qemu_get_be64(f); > - qemu_get_buffer(f, s->curr_status.mda, 4); > - /* end SCHIB */ > - qemu_get_buffer(f, s->sense_data, 32); > - s->channel_prog = qemu_get_be64(f); > - /* last cmd */ > - s->last_cmd.cmd_code = qemu_get_byte(f); > - s->last_cmd.flags = qemu_get_byte(f); > - s->last_cmd.count = qemu_get_be16(f); > - s->last_cmd.cda = qemu_get_be32(f); > - s->last_cmd_valid = qemu_get_byte(f); > - s->id.reserved = qemu_get_byte(f); > - s->id.cu_type = qemu_get_be16(f); > - s->id.cu_model = qemu_get_byte(f); > - s->id.dev_type = qemu_get_be16(f); > - s->id.dev_model = qemu_get_byte(f); > - s->id.unused = qemu_get_byte(f); > - for (i = 0; i < ARRAY_SIZE(s->id.ciw); i++) { > - s->id.ciw[i].type = qemu_get_byte(f); > - s->id.ciw[i].command = qemu_get_byte(f); > - s->id.ciw[i].count = qemu_get_be16(f); > - } > - s->ccw_fmt_1 = qemu_get_byte(f); > - s->ccw_no_data_cnt = qemu_get_byte(f); > - /* > - * Hack alert. We don't migrate the channel subsystem status (no > - * device!), but we need to find out if the guest enabled mss/mcss-e. > - * If the subchannel is enabled, it certainly was able to access it, > - * so adjust the max_ssid/max_cssid values for relevant ssid/cssid > - * values. This is not watertight, but better than nothing. > - */ > - if (s->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA) { > - if (s->ssid) { > - channel_subsys.max_ssid = MAX_SSID; > - } > - if (s->cssid != channel_subsys.default_cssid) { > - channel_subsys.max_cssid = MAX_CSSID; > - } > - } > - return 0; > -} > - > void css_reset_sch(SubchDev *sch) > { > PMCW *p = &sch->curr_status.pmcw; > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index e7167e3d05..a75eedeffb 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -34,9 +34,87 @@ > #include "virtio-ccw.h" > #include "trace.h" > #include "hw/s390x/css-bridge.h" > +#include "hw/s390x/s390-virtio-ccw.h" > > #define NR_CLASSIC_INDICATOR_BITS 64 > > +static int virtio_ccw_dev_post_load(void *opaque, int version_id) > +{ > + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(opaque); > + CcwDevice *ccw_dev = CCW_DEVICE(dev); > + CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); > + > + ccw_dev->sch->driver_data = dev; > + if (CCW_DEVICE(dev)->sch->thinint_active) { Please use ccw_dev instead of CCW_DEVICE(dev). > + dev->routes.adapter.adapter_id = css_get_adapter_id( > + CSS_IO_ADAPTER_VIRTIO, > + dev->thinint_isc); > + } > + /* Re-fill subch_id after loading the subchannel states.*/ > + if (ck->refill_ids) { > + ck->refill_ids(ccw_dev); > + } > + return 0; > +} > + > +typedef struct VirtioCcwDeviceTmp { > + VirtioCcwDevice *parent; > + uint16_t config_vector; > +} VirtioCcwDeviceTmp; > + > +static void virtio_ccw_dev_tmp_pre_save(void *opaque) > +{ > + VirtioCcwDeviceTmp *tmp = opaque; > + VirtioCcwDevice *dev = tmp->parent; > + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); > + > + tmp->config_vector = vdev->config_vector; > +} > + > +static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id) > +{ > + VirtioCcwDeviceTmp *tmp = opaque; > + VirtioCcwDevice *dev = tmp->parent; > + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); > + > + vdev->config_vector = tmp->config_vector; > + return 0; > +} > + > +const VMStateDescription vmstate_virtio_ccw_dev_tmp = { > + .name = "s390_virtio_ccw_dev_tmp", > + .pre_save = virtio_ccw_dev_tmp_pre_save, > + .post_load = virtio_ccw_dev_tmp_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +const VMStateDescription vmstate_virtio_ccw_dev = { > + .name = "s390_virtio_ccw_dev", > + .version_id = 1, > + .minimum_version_id = 1, > + .post_load = virtio_ccw_dev_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice), > + VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), > + VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), > + VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), > + /* > + * Ugly hack because VirtIODevice does not migrate itself. > + * This also makes legacy via vmstate_save_state possible. > + */ > + VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp, > + vmstate_virtio_ccw_dev_tmp), > + VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \ > + AdapterRoutes), > + VMSTATE_UINT8(thinint_isc, VirtioCcwDevice), > + VMSTATE_INT32(revision, VirtioCcwDevice), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size, > VirtioCcwDevice *dev); > > @@ -1234,85 +1312,13 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f) > static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) > { > VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > - CcwDevice *ccw_dev = CCW_DEVICE(d); > - SubchDev *s = ccw_dev->sch; > - VirtIODevice *vdev = virtio_ccw_get_vdev(s); > - > - subch_device_save(s, f); > - if (dev->indicators != NULL) { > - qemu_put_be32(f, dev->indicators->len); > - qemu_put_be64(f, dev->indicators->addr); > - } else { > - qemu_put_be32(f, 0); > - qemu_put_be64(f, 0UL); > - } > - if (dev->indicators2 != NULL) { > - qemu_put_be32(f, dev->indicators2->len); > - qemu_put_be64(f, dev->indicators2->addr); > - } else { > - qemu_put_be32(f, 0); > - qemu_put_be64(f, 0UL); > - } > - if (dev->summary_indicator != NULL) { > - qemu_put_be32(f, dev->summary_indicator->len); > - qemu_put_be64(f, dev->summary_indicator->addr); > - } else { > - qemu_put_be32(f, 0); > - qemu_put_be64(f, 0UL); > - } > - qemu_put_be16(f, vdev->config_vector); > - qemu_put_be64(f, dev->routes.adapter.ind_offset); > - qemu_put_byte(f, dev->thinint_isc); > - qemu_put_be32(f, dev->revision); > + vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL); > } > > static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) > { > VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > - CcwDevice *ccw_dev = CCW_DEVICE(d); > - CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); > - SubchDev *s = ccw_dev->sch; > - VirtIODevice *vdev = virtio_ccw_get_vdev(s); > - int len; > - > - s->driver_data = dev; > - subch_device_load(s, f); > - /* Re-fill subch_id after loading the subchannel states.*/ > - if (ck->refill_ids) { > - ck->refill_ids(ccw_dev); > - } > - len = qemu_get_be32(f); > - if (len != 0) { > - dev->indicators = get_indicator(qemu_get_be64(f), len); > - } else { > - qemu_get_be64(f); > - dev->indicators = NULL; > - } > - len = qemu_get_be32(f); > - if (len != 0) { > - dev->indicators2 = get_indicator(qemu_get_be64(f), len); > - } else { > - qemu_get_be64(f); > - dev->indicators2 = NULL; > - } > - len = qemu_get_be32(f); > - if (len != 0) { > - dev->summary_indicator = get_indicator(qemu_get_be64(f), len); > - } else { > - qemu_get_be64(f); > - dev->summary_indicator = NULL; > - } > - qemu_get_be16s(f, &vdev->config_vector); > - dev->routes.adapter.ind_offset = qemu_get_be64(f); > - dev->thinint_isc = qemu_get_byte(f); > - dev->revision = qemu_get_be32(f); > - if (s->thinint_active) { > - dev->routes.adapter.adapter_id = css_get_adapter_id( > - CSS_IO_ADAPTER_VIRTIO, > - dev->thinint_isc); > - } > - > - return 0; > + return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1); > } > > static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) > diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h > index e61fa74d9b..d74e44d312 100644 > --- a/include/hw/s390x/css.h > +++ b/include/hw/s390x/css.h > @@ -88,6 +88,7 @@ struct SubchDev { > bool ccw_fmt_1; > bool thinint_active; > uint8_t ccw_no_data_cnt; > + uint16_t migrated_schid; /* used for missmatch detection */ > /* transport-provided data: */ > int (*ccw_cb) (SubchDev *, CCW1); > void (*disable_cb)(SubchDev *); > @@ -95,22 +96,27 @@ struct SubchDev { > void *driver_data; > }; > > +extern const VMStateDescription vmstate_subch_dev; > + > typedef struct IndAddr { > hwaddr addr; > uint64_t map; > unsigned long refcnt; > - int len; > + int32_t len; > QTAILQ_ENTRY(IndAddr) sibling; > } IndAddr; > > +extern const VMStateDescription vmstate_ind_addr; > + > +#define VMSTATE_PTR_TO_IND_ADDR(_f, _s) \ > + VMSTATE_STRUCT(_f, _s, 1, vmstate_ind_addr, IndAddr*) > + > IndAddr *get_indicator(hwaddr ind_addr, int len); > void release_indicator(AdapterInfo *adapter, IndAddr *indicator); > int map_indicator(AdapterInfo *adapter, IndAddr *indicator); > > typedef SubchDev *(*css_subch_cb_func)(uint8_t m, uint8_t cssid, uint8_t ssid, > uint16_t schid); > -void subch_device_save(SubchDev *s, QEMUFile *f); > -int subch_device_load(SubchDev *s, QEMUFile *f); > int css_create_css_image(uint8_t cssid, bool default_image); > bool css_devno_used(uint8_t cssid, uint8_t ssid, uint16_t devno); > void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, > diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h > index f9e6890c90..caa6fc608d 100644 > --- a/include/hw/s390x/s390_flic.h > +++ b/include/hw/s390x/s390_flic.h > @@ -31,6 +31,11 @@ typedef struct AdapterRoutes { > int gsi[ADAPTER_ROUTES_MAX_GSI]; > } AdapterRoutes; > > +extern const VMStateDescription vmstate_adapter_routes; > + > +#define VMSTATE_ADAPTER_ROUTES(_f, _s) \ > + VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes) > + > #define TYPE_S390_FLIC_COMMON "s390-flic" > #define S390_FLIC_COMMON(obj) \ > OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON) Other than the nit above, looks good to me. Especially the migration of the pmcw & co. is now more readable. So, with the nit fixed, Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> Christian, can you please take this?
On 06/01/2017 12:52 PM, Cornelia Huck wrote: > On Mon, 29 May 2017 15:17:16 +0200 > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > >> Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for >> flexibility (extending using subsections) and for fun. > > You have interesting ideas of fun :) > >> >> To achieve this we need to hack the config_vector, which is VirtIODevice >> (that is common virtio) state, in the middle of the VirtioCcwDevice state >> representation. This somewhat ugly, but we have no choice because the >> stream format needs to be preserved. >> >> Almost no changes in behavior. Exception is everything that comes with >> vmstate like extra bookkeeping about what's in the stream, and maybe some >> extra checks and better error reporting. >> >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> >> --- [..] >> --- a/hw/s390x/css.c >> +++ b/hw/s390x/css.c [..] >> +static const VMStateDescription vmstate_sense_id = { >> + .name = "s390_sense_id", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT8(reserved, SenseId), >> + VMSTATE_UINT16(cu_type, SenseId), >> + VMSTATE_UINT8(cu_model, SenseId), >> + VMSTATE_UINT16(dev_type, SenseId), >> + VMSTATE_UINT8(dev_model, SenseId), >> + VMSTATE_UINT8(unused, SenseId), > > This is strictly due to stream format preservation, right? > Yes! [..] >> --- a/hw/s390x/virtio-ccw.c >> +++ b/hw/s390x/virtio-ccw.c >> @@ -34,9 +34,87 @@ >> #include "virtio-ccw.h" >> #include "trace.h" >> #include "hw/s390x/css-bridge.h" >> +#include "hw/s390x/s390-virtio-ccw.h" >> >> #define NR_CLASSIC_INDICATOR_BITS 64 >> >> +static int virtio_ccw_dev_post_load(void *opaque, int version_id) >> +{ >> + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(opaque); >> + CcwDevice *ccw_dev = CCW_DEVICE(dev); >> + CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >> + >> + ccw_dev->sch->driver_data = dev; >> + if (CCW_DEVICE(dev)->sch->thinint_active) { > > Please use ccw_dev instead of CCW_DEVICE(dev). > Sure. >> + dev->routes.adapter.adapter_id = css_get_adapter_id( >> + CSS_IO_ADAPTER_VIRTIO, >> + dev->thinint_isc); >> + } >> + /* Re-fill subch_id after loading the subchannel states.*/ >> + if (ck->refill_ids) { >> + ck->refill_ids(ccw_dev); >> + } >> + return 0; >> +} >> + >> +typedef struct VirtioCcwDeviceTmp { >> + VirtioCcwDevice *parent; >> + uint16_t config_vector; >> +} VirtioCcwDeviceTmp; >> + >> +static void virtio_ccw_dev_tmp_pre_save(void *opaque) >> +{ >> + VirtioCcwDeviceTmp *tmp = opaque; >> + VirtioCcwDevice *dev = tmp->parent; >> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >> + >> + tmp->config_vector = vdev->config_vector; >> +} >> + >> +static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id) >> +{ >> + VirtioCcwDeviceTmp *tmp = opaque; >> + VirtioCcwDevice *dev = tmp->parent; >> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >> + >> + vdev->config_vector = tmp->config_vector; >> + return 0; >> +} >> + >> +const VMStateDescription vmstate_virtio_ccw_dev_tmp = { >> + .name = "s390_virtio_ccw_dev_tmp", >> + .pre_save = virtio_ccw_dev_tmp_pre_save, >> + .post_load = virtio_ccw_dev_tmp_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +const VMStateDescription vmstate_virtio_ccw_dev = { >> + .name = "s390_virtio_ccw_dev", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .post_load = virtio_ccw_dev_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice), >> + VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), >> + VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), >> + VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), >> + /* >> + * Ugly hack because VirtIODevice does not migrate itself. >> + * This also makes legacy via vmstate_save_state possible. >> + */ >> + VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp, >> + vmstate_virtio_ccw_dev_tmp), >> + VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \ >> + AdapterRoutes), >> + VMSTATE_UINT8(thinint_isc, VirtioCcwDevice), >> + VMSTATE_INT32(revision, VirtioCcwDevice), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size, >> VirtioCcwDevice *dev); >> >> @@ -1234,85 +1312,13 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f) >> static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) >> { >> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >> - CcwDevice *ccw_dev = CCW_DEVICE(d); >> - SubchDev *s = ccw_dev->sch; >> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); >> - >> - subch_device_save(s, f); >> - if (dev->indicators != NULL) { >> - qemu_put_be32(f, dev->indicators->len); >> - qemu_put_be64(f, dev->indicators->addr); >> - } else { >> - qemu_put_be32(f, 0); >> - qemu_put_be64(f, 0UL); >> - } >> - if (dev->indicators2 != NULL) { >> - qemu_put_be32(f, dev->indicators2->len); >> - qemu_put_be64(f, dev->indicators2->addr); >> - } else { >> - qemu_put_be32(f, 0); >> - qemu_put_be64(f, 0UL); >> - } >> - if (dev->summary_indicator != NULL) { >> - qemu_put_be32(f, dev->summary_indicator->len); >> - qemu_put_be64(f, dev->summary_indicator->addr); >> - } else { >> - qemu_put_be32(f, 0); >> - qemu_put_be64(f, 0UL); >> - } >> - qemu_put_be16(f, vdev->config_vector); >> - qemu_put_be64(f, dev->routes.adapter.ind_offset); >> - qemu_put_byte(f, dev->thinint_isc); >> - qemu_put_be32(f, dev->revision); >> + vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL); >> } >> >> static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) >> { >> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >> - CcwDevice *ccw_dev = CCW_DEVICE(d); >> - CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >> - SubchDev *s = ccw_dev->sch; >> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); >> - int len; >> - >> - s->driver_data = dev; >> - subch_device_load(s, f); >> - /* Re-fill subch_id after loading the subchannel states.*/ >> - if (ck->refill_ids) { >> - ck->refill_ids(ccw_dev); >> - } >> - len = qemu_get_be32(f); >> - if (len != 0) { >> - dev->indicators = get_indicator(qemu_get_be64(f), len); >> - } else { >> - qemu_get_be64(f); >> - dev->indicators = NULL; >> - } >> - len = qemu_get_be32(f); >> - if (len != 0) { >> - dev->indicators2 = get_indicator(qemu_get_be64(f), len); >> - } else { >> - qemu_get_be64(f); >> - dev->indicators2 = NULL; >> - } >> - len = qemu_get_be32(f); >> - if (len != 0) { >> - dev->summary_indicator = get_indicator(qemu_get_be64(f), len); >> - } else { >> - qemu_get_be64(f); >> - dev->summary_indicator = NULL; >> - } >> - qemu_get_be16s(f, &vdev->config_vector); >> - dev->routes.adapter.ind_offset = qemu_get_be64(f); >> - dev->thinint_isc = qemu_get_byte(f); >> - dev->revision = qemu_get_be32(f); >> - if (s->thinint_active) { >> - dev->routes.adapter.adapter_id = css_get_adapter_id( >> - CSS_IO_ADAPTER_VIRTIO, >> - dev->thinint_isc); >> - } >> - >> - return 0; >> + return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1); >> } >> >> static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) >> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h >> index e61fa74d9b..d74e44d312 100644 >> --- a/include/hw/s390x/css.h >> +++ b/include/hw/s390x/css.h >> @@ -88,6 +88,7 @@ struct SubchDev { >> bool ccw_fmt_1; >> bool thinint_active; >> uint8_t ccw_no_data_cnt; >> + uint16_t migrated_schid; /* used for missmatch detection */ >> /* transport-provided data: */ >> int (*ccw_cb) (SubchDev *, CCW1); >> void (*disable_cb)(SubchDev *); >> @@ -95,22 +96,27 @@ struct SubchDev { >> void *driver_data; >> }; >> >> +extern const VMStateDescription vmstate_subch_dev; >> + >> typedef struct IndAddr { >> hwaddr addr; >> uint64_t map; >> unsigned long refcnt; >> - int len; >> + int32_t len; >> QTAILQ_ENTRY(IndAddr) sibling; >> } IndAddr; >> >> +extern const VMStateDescription vmstate_ind_addr; >> + >> +#define VMSTATE_PTR_TO_IND_ADDR(_f, _s) \ >> + VMSTATE_STRUCT(_f, _s, 1, vmstate_ind_addr, IndAddr*) >> + >> IndAddr *get_indicator(hwaddr ind_addr, int len); >> void release_indicator(AdapterInfo *adapter, IndAddr *indicator); >> int map_indicator(AdapterInfo *adapter, IndAddr *indicator); >> >> typedef SubchDev *(*css_subch_cb_func)(uint8_t m, uint8_t cssid, uint8_t ssid, >> uint16_t schid); >> -void subch_device_save(SubchDev *s, QEMUFile *f); >> -int subch_device_load(SubchDev *s, QEMUFile *f); >> int css_create_css_image(uint8_t cssid, bool default_image); >> bool css_devno_used(uint8_t cssid, uint8_t ssid, uint16_t devno); >> void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, >> diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h >> index f9e6890c90..caa6fc608d 100644 >> --- a/include/hw/s390x/s390_flic.h >> +++ b/include/hw/s390x/s390_flic.h >> @@ -31,6 +31,11 @@ typedef struct AdapterRoutes { >> int gsi[ADAPTER_ROUTES_MAX_GSI]; >> } AdapterRoutes; >> >> +extern const VMStateDescription vmstate_adapter_routes; >> + >> +#define VMSTATE_ADAPTER_ROUTES(_f, _s) \ >> + VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes) >> + >> #define TYPE_S390_FLIC_COMMON "s390-flic" >> #define S390_FLIC_COMMON(obj) \ >> OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON) > > Other than the nit above, looks good to me. Especially the migration of > the pmcw & co. is now more readable. So, with the nit fixed, > > Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> Thanks! > > Christian, can you please take this? > Should I do a re-spin with the nit's picked and the r-b's added? Regards, Halil
On 06/01/2017 01:02 PM, Halil Pasic wrote: > > > On 06/01/2017 12:52 PM, Cornelia Huck wrote: >> On Mon, 29 May 2017 15:17:16 +0200 >> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >> >>> Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for >>> flexibility (extending using subsections) and for fun. >> >> You have interesting ideas of fun :) >> >>> >>> To achieve this we need to hack the config_vector, which is VirtIODevice >>> (that is common virtio) state, in the middle of the VirtioCcwDevice state >>> representation. This somewhat ugly, but we have no choice because the >>> stream format needs to be preserved. >>> >>> Almost no changes in behavior. Exception is everything that comes with >>> vmstate like extra bookkeeping about what's in the stream, and maybe some >>> extra checks and better error reporting. >>> >>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> >>> --- > [..] >>> --- a/hw/s390x/css.c >>> +++ b/hw/s390x/css.c > [..] >>> +static const VMStateDescription vmstate_sense_id = { >>> + .name = "s390_sense_id", >>> + .version_id = 1, >>> + .minimum_version_id = 1, >>> + .fields = (VMStateField[]) { >>> + VMSTATE_UINT8(reserved, SenseId), >>> + VMSTATE_UINT16(cu_type, SenseId), >>> + VMSTATE_UINT8(cu_model, SenseId), >>> + VMSTATE_UINT16(dev_type, SenseId), >>> + VMSTATE_UINT8(dev_model, SenseId), >>> + VMSTATE_UINT8(unused, SenseId), >> >> This is strictly due to stream format preservation, right? >> > > Yes! > > [..] >>> --- a/hw/s390x/virtio-ccw.c >>> +++ b/hw/s390x/virtio-ccw.c >>> @@ -34,9 +34,87 @@ >>> #include "virtio-ccw.h" >>> #include "trace.h" >>> #include "hw/s390x/css-bridge.h" >>> +#include "hw/s390x/s390-virtio-ccw.h" >>> >>> #define NR_CLASSIC_INDICATOR_BITS 64 >>> >>> +static int virtio_ccw_dev_post_load(void *opaque, int version_id) >>> +{ >>> + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(opaque); >>> + CcwDevice *ccw_dev = CCW_DEVICE(dev); >>> + CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >>> + >>> + ccw_dev->sch->driver_data = dev; >>> + if (CCW_DEVICE(dev)->sch->thinint_active) { >> >> Please use ccw_dev instead of CCW_DEVICE(dev). >> > > Sure. > >>> + dev->routes.adapter.adapter_id = css_get_adapter_id( >>> + CSS_IO_ADAPTER_VIRTIO, >>> + dev->thinint_isc); >>> + } >>> + /* Re-fill subch_id after loading the subchannel states.*/ >>> + if (ck->refill_ids) { >>> + ck->refill_ids(ccw_dev); >>> + } >>> + return 0; >>> +} >>> + >>> +typedef struct VirtioCcwDeviceTmp { >>> + VirtioCcwDevice *parent; >>> + uint16_t config_vector; >>> +} VirtioCcwDeviceTmp; >>> + >>> +static void virtio_ccw_dev_tmp_pre_save(void *opaque) >>> +{ >>> + VirtioCcwDeviceTmp *tmp = opaque; >>> + VirtioCcwDevice *dev = tmp->parent; >>> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >>> + >>> + tmp->config_vector = vdev->config_vector; >>> +} >>> + >>> +static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id) >>> +{ >>> + VirtioCcwDeviceTmp *tmp = opaque; >>> + VirtioCcwDevice *dev = tmp->parent; >>> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >>> + >>> + vdev->config_vector = tmp->config_vector; >>> + return 0; >>> +} >>> + >>> +const VMStateDescription vmstate_virtio_ccw_dev_tmp = { >>> + .name = "s390_virtio_ccw_dev_tmp", >>> + .pre_save = virtio_ccw_dev_tmp_pre_save, >>> + .post_load = virtio_ccw_dev_tmp_post_load, >>> + .fields = (VMStateField[]) { >>> + VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp), >>> + VMSTATE_END_OF_LIST() >>> + } >>> +}; >>> + >>> +const VMStateDescription vmstate_virtio_ccw_dev = { >>> + .name = "s390_virtio_ccw_dev", >>> + .version_id = 1, >>> + .minimum_version_id = 1, >>> + .post_load = virtio_ccw_dev_post_load, >>> + .fields = (VMStateField[]) { >>> + VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice), >>> + VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), >>> + VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), >>> + VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), >>> + /* >>> + * Ugly hack because VirtIODevice does not migrate itself. >>> + * This also makes legacy via vmstate_save_state possible. >>> + */ >>> + VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp, >>> + vmstate_virtio_ccw_dev_tmp), >>> + VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \ >>> + AdapterRoutes), >>> + VMSTATE_UINT8(thinint_isc, VirtioCcwDevice), >>> + VMSTATE_INT32(revision, VirtioCcwDevice), >>> + VMSTATE_END_OF_LIST() >>> + } >>> +}; >>> + >>> static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size, >>> VirtioCcwDevice *dev); >>> >>> @@ -1234,85 +1312,13 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f) >>> static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) >>> { >>> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >>> - CcwDevice *ccw_dev = CCW_DEVICE(d); >>> - SubchDev *s = ccw_dev->sch; >>> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); >>> - >>> - subch_device_save(s, f); >>> - if (dev->indicators != NULL) { >>> - qemu_put_be32(f, dev->indicators->len); >>> - qemu_put_be64(f, dev->indicators->addr); >>> - } else { >>> - qemu_put_be32(f, 0); >>> - qemu_put_be64(f, 0UL); >>> - } >>> - if (dev->indicators2 != NULL) { >>> - qemu_put_be32(f, dev->indicators2->len); >>> - qemu_put_be64(f, dev->indicators2->addr); >>> - } else { >>> - qemu_put_be32(f, 0); >>> - qemu_put_be64(f, 0UL); >>> - } >>> - if (dev->summary_indicator != NULL) { >>> - qemu_put_be32(f, dev->summary_indicator->len); >>> - qemu_put_be64(f, dev->summary_indicator->addr); >>> - } else { >>> - qemu_put_be32(f, 0); >>> - qemu_put_be64(f, 0UL); >>> - } >>> - qemu_put_be16(f, vdev->config_vector); >>> - qemu_put_be64(f, dev->routes.adapter.ind_offset); >>> - qemu_put_byte(f, dev->thinint_isc); >>> - qemu_put_be32(f, dev->revision); >>> + vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL); >>> } >>> >>> static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) >>> { >>> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >>> - CcwDevice *ccw_dev = CCW_DEVICE(d); >>> - CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >>> - SubchDev *s = ccw_dev->sch; >>> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); >>> - int len; >>> - >>> - s->driver_data = dev; >>> - subch_device_load(s, f); >>> - /* Re-fill subch_id after loading the subchannel states.*/ >>> - if (ck->refill_ids) { >>> - ck->refill_ids(ccw_dev); >>> - } >>> - len = qemu_get_be32(f); >>> - if (len != 0) { >>> - dev->indicators = get_indicator(qemu_get_be64(f), len); >>> - } else { >>> - qemu_get_be64(f); >>> - dev->indicators = NULL; >>> - } >>> - len = qemu_get_be32(f); >>> - if (len != 0) { >>> - dev->indicators2 = get_indicator(qemu_get_be64(f), len); >>> - } else { >>> - qemu_get_be64(f); >>> - dev->indicators2 = NULL; >>> - } >>> - len = qemu_get_be32(f); >>> - if (len != 0) { >>> - dev->summary_indicator = get_indicator(qemu_get_be64(f), len); >>> - } else { >>> - qemu_get_be64(f); >>> - dev->summary_indicator = NULL; >>> - } >>> - qemu_get_be16s(f, &vdev->config_vector); >>> - dev->routes.adapter.ind_offset = qemu_get_be64(f); >>> - dev->thinint_isc = qemu_get_byte(f); >>> - dev->revision = qemu_get_be32(f); >>> - if (s->thinint_active) { >>> - dev->routes.adapter.adapter_id = css_get_adapter_id( >>> - CSS_IO_ADAPTER_VIRTIO, >>> - dev->thinint_isc); >>> - } >>> - >>> - return 0; >>> + return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1); >>> } >>> >>> static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) >>> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h >>> index e61fa74d9b..d74e44d312 100644 >>> --- a/include/hw/s390x/css.h >>> +++ b/include/hw/s390x/css.h >>> @@ -88,6 +88,7 @@ struct SubchDev { >>> bool ccw_fmt_1; >>> bool thinint_active; >>> uint8_t ccw_no_data_cnt; >>> + uint16_t migrated_schid; /* used for missmatch detection */ >>> /* transport-provided data: */ >>> int (*ccw_cb) (SubchDev *, CCW1); >>> void (*disable_cb)(SubchDev *); >>> @@ -95,22 +96,27 @@ struct SubchDev { >>> void *driver_data; >>> }; >>> >>> +extern const VMStateDescription vmstate_subch_dev; >>> + >>> typedef struct IndAddr { >>> hwaddr addr; >>> uint64_t map; >>> unsigned long refcnt; >>> - int len; >>> + int32_t len; >>> QTAILQ_ENTRY(IndAddr) sibling; >>> } IndAddr; >>> >>> +extern const VMStateDescription vmstate_ind_addr; >>> + >>> +#define VMSTATE_PTR_TO_IND_ADDR(_f, _s) \ >>> + VMSTATE_STRUCT(_f, _s, 1, vmstate_ind_addr, IndAddr*) >>> + >>> IndAddr *get_indicator(hwaddr ind_addr, int len); >>> void release_indicator(AdapterInfo *adapter, IndAddr *indicator); >>> int map_indicator(AdapterInfo *adapter, IndAddr *indicator); >>> >>> typedef SubchDev *(*css_subch_cb_func)(uint8_t m, uint8_t cssid, uint8_t ssid, >>> uint16_t schid); >>> -void subch_device_save(SubchDev *s, QEMUFile *f); >>> -int subch_device_load(SubchDev *s, QEMUFile *f); >>> int css_create_css_image(uint8_t cssid, bool default_image); >>> bool css_devno_used(uint8_t cssid, uint8_t ssid, uint16_t devno); >>> void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, >>> diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h >>> index f9e6890c90..caa6fc608d 100644 >>> --- a/include/hw/s390x/s390_flic.h >>> +++ b/include/hw/s390x/s390_flic.h >>> @@ -31,6 +31,11 @@ typedef struct AdapterRoutes { >>> int gsi[ADAPTER_ROUTES_MAX_GSI]; >>> } AdapterRoutes; >>> >>> +extern const VMStateDescription vmstate_adapter_routes; >>> + >>> +#define VMSTATE_ADAPTER_ROUTES(_f, _s) \ >>> + VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes) >>> + >>> #define TYPE_S390_FLIC_COMMON "s390-flic" >>> #define S390_FLIC_COMMON(obj) \ >>> OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON) >> >> Other than the nit above, looks good to me. Especially the migration of >> the pmcw & co. is now more readable. So, with the nit fixed, >> >> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> > > Thanks! > >> >> Christian, can you please take this? >> > > Should I do a re-spin with the nit's picked and the r-b's added? Yes please. Can you then give me a confirmation that you have tested a migration between 2.9 and this patch set?
On 06/01/2017 01:19 PM, Christian Borntraeger wrote: > On 06/01/2017 01:02 PM, Halil Pasic wrote: >> >> >> On 06/01/2017 12:52 PM, Cornelia Huck wrote: >>> On Mon, 29 May 2017 15:17:16 +0200 >>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >>> >>>> Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for >>>> flexibility (extending using subsections) and for fun. >>> >>> You have interesting ideas of fun :) >>> >>>> >>>> To achieve this we need to hack the config_vector, which is VirtIODevice >>>> (that is common virtio) state, in the middle of the VirtioCcwDevice state >>>> representation. This somewhat ugly, but we have no choice because the >>>> stream format needs to be preserved. >>>> >>>> Almost no changes in behavior. Exception is everything that comes with >>>> vmstate like extra bookkeeping about what's in the stream, and maybe some >>>> extra checks and better error reporting. >>>> >>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> >>>> --- >> [..] >>>> --- a/hw/s390x/css.c >>>> +++ b/hw/s390x/css.c >> [..] >>>> +static const VMStateDescription vmstate_sense_id = { >>>> + .name = "s390_sense_id", >>>> + .version_id = 1, >>>> + .minimum_version_id = 1, >>>> + .fields = (VMStateField[]) { >>>> + VMSTATE_UINT8(reserved, SenseId), >>>> + VMSTATE_UINT16(cu_type, SenseId), >>>> + VMSTATE_UINT8(cu_model, SenseId), >>>> + VMSTATE_UINT16(dev_type, SenseId), >>>> + VMSTATE_UINT8(dev_model, SenseId), >>>> + VMSTATE_UINT8(unused, SenseId), >>> >>> This is strictly due to stream format preservation, right? >>> >> >> Yes! >> >> [..] >>>> --- a/hw/s390x/virtio-ccw.c >>>> +++ b/hw/s390x/virtio-ccw.c >>>> @@ -34,9 +34,87 @@ >>>> #include "virtio-ccw.h" >>>> #include "trace.h" >>>> #include "hw/s390x/css-bridge.h" >>>> +#include "hw/s390x/s390-virtio-ccw.h" >>>> >>>> #define NR_CLASSIC_INDICATOR_BITS 64 >>>> >>>> +static int virtio_ccw_dev_post_load(void *opaque, int version_id) >>>> +{ >>>> + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(opaque); >>>> + CcwDevice *ccw_dev = CCW_DEVICE(dev); >>>> + CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >>>> + >>>> + ccw_dev->sch->driver_data = dev; >>>> + if (CCW_DEVICE(dev)->sch->thinint_active) { >>> >>> Please use ccw_dev instead of CCW_DEVICE(dev). >>> >> >> Sure. >> >>>> + dev->routes.adapter.adapter_id = css_get_adapter_id( >>>> + CSS_IO_ADAPTER_VIRTIO, >>>> + dev->thinint_isc); >>>> + } >>>> + /* Re-fill subch_id after loading the subchannel states.*/ >>>> + if (ck->refill_ids) { >>>> + ck->refill_ids(ccw_dev); >>>> + } >>>> + return 0; >>>> +} >>>> + >>>> +typedef struct VirtioCcwDeviceTmp { >>>> + VirtioCcwDevice *parent; >>>> + uint16_t config_vector; >>>> +} VirtioCcwDeviceTmp; >>>> + >>>> +static void virtio_ccw_dev_tmp_pre_save(void *opaque) >>>> +{ >>>> + VirtioCcwDeviceTmp *tmp = opaque; >>>> + VirtioCcwDevice *dev = tmp->parent; >>>> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >>>> + >>>> + tmp->config_vector = vdev->config_vector; >>>> +} >>>> + >>>> +static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id) >>>> +{ >>>> + VirtioCcwDeviceTmp *tmp = opaque; >>>> + VirtioCcwDevice *dev = tmp->parent; >>>> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >>>> + >>>> + vdev->config_vector = tmp->config_vector; >>>> + return 0; >>>> +} >>>> + >>>> +const VMStateDescription vmstate_virtio_ccw_dev_tmp = { >>>> + .name = "s390_virtio_ccw_dev_tmp", >>>> + .pre_save = virtio_ccw_dev_tmp_pre_save, >>>> + .post_load = virtio_ccw_dev_tmp_post_load, >>>> + .fields = (VMStateField[]) { >>>> + VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp), >>>> + VMSTATE_END_OF_LIST() >>>> + } >>>> +}; >>>> + >>>> +const VMStateDescription vmstate_virtio_ccw_dev = { >>>> + .name = "s390_virtio_ccw_dev", >>>> + .version_id = 1, >>>> + .minimum_version_id = 1, >>>> + .post_load = virtio_ccw_dev_post_load, >>>> + .fields = (VMStateField[]) { >>>> + VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice), >>>> + VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), >>>> + VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), >>>> + VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), >>>> + /* >>>> + * Ugly hack because VirtIODevice does not migrate itself. >>>> + * This also makes legacy via vmstate_save_state possible. >>>> + */ >>>> + VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp, >>>> + vmstate_virtio_ccw_dev_tmp), >>>> + VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \ >>>> + AdapterRoutes), >>>> + VMSTATE_UINT8(thinint_isc, VirtioCcwDevice), >>>> + VMSTATE_INT32(revision, VirtioCcwDevice), >>>> + VMSTATE_END_OF_LIST() >>>> + } >>>> +}; >>>> + >>>> static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size, >>>> VirtioCcwDevice *dev); >>>> >>>> @@ -1234,85 +1312,13 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f) >>>> static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) >>>> { >>>> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >>>> - CcwDevice *ccw_dev = CCW_DEVICE(d); >>>> - SubchDev *s = ccw_dev->sch; >>>> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); >>>> - >>>> - subch_device_save(s, f); >>>> - if (dev->indicators != NULL) { >>>> - qemu_put_be32(f, dev->indicators->len); >>>> - qemu_put_be64(f, dev->indicators->addr); >>>> - } else { >>>> - qemu_put_be32(f, 0); >>>> - qemu_put_be64(f, 0UL); >>>> - } >>>> - if (dev->indicators2 != NULL) { >>>> - qemu_put_be32(f, dev->indicators2->len); >>>> - qemu_put_be64(f, dev->indicators2->addr); >>>> - } else { >>>> - qemu_put_be32(f, 0); >>>> - qemu_put_be64(f, 0UL); >>>> - } >>>> - if (dev->summary_indicator != NULL) { >>>> - qemu_put_be32(f, dev->summary_indicator->len); >>>> - qemu_put_be64(f, dev->summary_indicator->addr); >>>> - } else { >>>> - qemu_put_be32(f, 0); >>>> - qemu_put_be64(f, 0UL); >>>> - } >>>> - qemu_put_be16(f, vdev->config_vector); >>>> - qemu_put_be64(f, dev->routes.adapter.ind_offset); >>>> - qemu_put_byte(f, dev->thinint_isc); >>>> - qemu_put_be32(f, dev->revision); >>>> + vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL); >>>> } >>>> >>>> static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) >>>> { >>>> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >>>> - CcwDevice *ccw_dev = CCW_DEVICE(d); >>>> - CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >>>> - SubchDev *s = ccw_dev->sch; >>>> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); >>>> - int len; >>>> - >>>> - s->driver_data = dev; >>>> - subch_device_load(s, f); >>>> - /* Re-fill subch_id after loading the subchannel states.*/ >>>> - if (ck->refill_ids) { >>>> - ck->refill_ids(ccw_dev); >>>> - } >>>> - len = qemu_get_be32(f); >>>> - if (len != 0) { >>>> - dev->indicators = get_indicator(qemu_get_be64(f), len); >>>> - } else { >>>> - qemu_get_be64(f); >>>> - dev->indicators = NULL; >>>> - } >>>> - len = qemu_get_be32(f); >>>> - if (len != 0) { >>>> - dev->indicators2 = get_indicator(qemu_get_be64(f), len); >>>> - } else { >>>> - qemu_get_be64(f); >>>> - dev->indicators2 = NULL; >>>> - } >>>> - len = qemu_get_be32(f); >>>> - if (len != 0) { >>>> - dev->summary_indicator = get_indicator(qemu_get_be64(f), len); >>>> - } else { >>>> - qemu_get_be64(f); >>>> - dev->summary_indicator = NULL; >>>> - } >>>> - qemu_get_be16s(f, &vdev->config_vector); >>>> - dev->routes.adapter.ind_offset = qemu_get_be64(f); >>>> - dev->thinint_isc = qemu_get_byte(f); >>>> - dev->revision = qemu_get_be32(f); >>>> - if (s->thinint_active) { >>>> - dev->routes.adapter.adapter_id = css_get_adapter_id( >>>> - CSS_IO_ADAPTER_VIRTIO, >>>> - dev->thinint_isc); >>>> - } >>>> - >>>> - return 0; >>>> + return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1); >>>> } >>>> >>>> static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) >>>> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h >>>> index e61fa74d9b..d74e44d312 100644 >>>> --- a/include/hw/s390x/css.h >>>> +++ b/include/hw/s390x/css.h >>>> @@ -88,6 +88,7 @@ struct SubchDev { >>>> bool ccw_fmt_1; >>>> bool thinint_active; >>>> uint8_t ccw_no_data_cnt; >>>> + uint16_t migrated_schid; /* used for missmatch detection */ >>>> /* transport-provided data: */ >>>> int (*ccw_cb) (SubchDev *, CCW1); >>>> void (*disable_cb)(SubchDev *); >>>> @@ -95,22 +96,27 @@ struct SubchDev { >>>> void *driver_data; >>>> }; >>>> >>>> +extern const VMStateDescription vmstate_subch_dev; >>>> + >>>> typedef struct IndAddr { >>>> hwaddr addr; >>>> uint64_t map; >>>> unsigned long refcnt; >>>> - int len; >>>> + int32_t len; >>>> QTAILQ_ENTRY(IndAddr) sibling; >>>> } IndAddr; >>>> >>>> +extern const VMStateDescription vmstate_ind_addr; >>>> + >>>> +#define VMSTATE_PTR_TO_IND_ADDR(_f, _s) \ >>>> + VMSTATE_STRUCT(_f, _s, 1, vmstate_ind_addr, IndAddr*) >>>> + >>>> IndAddr *get_indicator(hwaddr ind_addr, int len); >>>> void release_indicator(AdapterInfo *adapter, IndAddr *indicator); >>>> int map_indicator(AdapterInfo *adapter, IndAddr *indicator); >>>> >>>> typedef SubchDev *(*css_subch_cb_func)(uint8_t m, uint8_t cssid, uint8_t ssid, >>>> uint16_t schid); >>>> -void subch_device_save(SubchDev *s, QEMUFile *f); >>>> -int subch_device_load(SubchDev *s, QEMUFile *f); >>>> int css_create_css_image(uint8_t cssid, bool default_image); >>>> bool css_devno_used(uint8_t cssid, uint8_t ssid, uint16_t devno); >>>> void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, >>>> diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h >>>> index f9e6890c90..caa6fc608d 100644 >>>> --- a/include/hw/s390x/s390_flic.h >>>> +++ b/include/hw/s390x/s390_flic.h >>>> @@ -31,6 +31,11 @@ typedef struct AdapterRoutes { >>>> int gsi[ADAPTER_ROUTES_MAX_GSI]; >>>> } AdapterRoutes; >>>> >>>> +extern const VMStateDescription vmstate_adapter_routes; >>>> + >>>> +#define VMSTATE_ADAPTER_ROUTES(_f, _s) \ >>>> + VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes) >>>> + >>>> #define TYPE_S390_FLIC_COMMON "s390-flic" >>>> #define S390_FLIC_COMMON(obj) \ >>>> OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON) >>> >>> Other than the nit above, looks good to me. Especially the migration of >>> the pmcw & co. is now more readable. So, with the nit fixed, >>> >>> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> >> >> Thanks! >> >>> >>> Christian, can you please take this? >>> >> >> Should I do a re-spin with the nit's picked and the r-b's added? > > Yes please. Can you then give me a confirmation that you have tested a migration between 2.9 and this patch set? > > I have tested between 2.5 and this (2.5 binary and compat both). I think that should be equivalent with testing with 2.9, but I can give it a couple of spins with 2.9. Though my tests are semi automated at best. Given the nature of changes I do not expect problems, but from the methodology perspective it ain't really satisfactory. It could might make sense to throw some well designed test-suite at it. Cheers, Halil
On 06/01/2017 01:28 PM, Halil Pasic wrote: > > > On 06/01/2017 01:19 PM, Christian Borntraeger wrote: >> On 06/01/2017 01:02 PM, Halil Pasic wrote: >>> >>> >>> On 06/01/2017 12:52 PM, Cornelia Huck wrote: >>>> On Mon, 29 May 2017 15:17:16 +0200 >>>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >>>> >>>>> Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for >>>>> flexibility (extending using subsections) and for fun. >>>> >>>> You have interesting ideas of fun :) >>>> >>>>> >>>>> To achieve this we need to hack the config_vector, which is VirtIODevice >>>>> (that is common virtio) state, in the middle of the VirtioCcwDevice state >>>>> representation. This somewhat ugly, but we have no choice because the >>>>> stream format needs to be preserved. >>>>> >>>>> Almost no changes in behavior. Exception is everything that comes with >>>>> vmstate like extra bookkeeping about what's in the stream, and maybe some >>>>> extra checks and better error reporting. >>>>> >>>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> >>>>> --- >>> [..] >>>>> --- a/hw/s390x/css.c >>>>> +++ b/hw/s390x/css.c >>> [..] >>>>> +static const VMStateDescription vmstate_sense_id = { >>>>> + .name = "s390_sense_id", >>>>> + .version_id = 1, >>>>> + .minimum_version_id = 1, >>>>> + .fields = (VMStateField[]) { >>>>> + VMSTATE_UINT8(reserved, SenseId), >>>>> + VMSTATE_UINT16(cu_type, SenseId), >>>>> + VMSTATE_UINT8(cu_model, SenseId), >>>>> + VMSTATE_UINT16(dev_type, SenseId), >>>>> + VMSTATE_UINT8(dev_model, SenseId), >>>>> + VMSTATE_UINT8(unused, SenseId), >>>> >>>> This is strictly due to stream format preservation, right? >>>> >>> >>> Yes! >>> >>> [..] >>>>> --- a/hw/s390x/virtio-ccw.c >>>>> +++ b/hw/s390x/virtio-ccw.c >>>>> @@ -34,9 +34,87 @@ >>>>> #include "virtio-ccw.h" >>>>> #include "trace.h" >>>>> #include "hw/s390x/css-bridge.h" >>>>> +#include "hw/s390x/s390-virtio-ccw.h" >>>>> >>>>> #define NR_CLASSIC_INDICATOR_BITS 64 >>>>> >>>>> +static int virtio_ccw_dev_post_load(void *opaque, int version_id) >>>>> +{ >>>>> + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(opaque); >>>>> + CcwDevice *ccw_dev = CCW_DEVICE(dev); >>>>> + CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >>>>> + >>>>> + ccw_dev->sch->driver_data = dev; >>>>> + if (CCW_DEVICE(dev)->sch->thinint_active) { >>>> >>>> Please use ccw_dev instead of CCW_DEVICE(dev). >>>> >>> >>> Sure. >>> >>>>> + dev->routes.adapter.adapter_id = css_get_adapter_id( >>>>> + CSS_IO_ADAPTER_VIRTIO, >>>>> + dev->thinint_isc); >>>>> + } >>>>> + /* Re-fill subch_id after loading the subchannel states.*/ >>>>> + if (ck->refill_ids) { >>>>> + ck->refill_ids(ccw_dev); >>>>> + } >>>>> + return 0; >>>>> +} >>>>> + >>>>> +typedef struct VirtioCcwDeviceTmp { >>>>> + VirtioCcwDevice *parent; >>>>> + uint16_t config_vector; >>>>> +} VirtioCcwDeviceTmp; >>>>> + >>>>> +static void virtio_ccw_dev_tmp_pre_save(void *opaque) >>>>> +{ >>>>> + VirtioCcwDeviceTmp *tmp = opaque; >>>>> + VirtioCcwDevice *dev = tmp->parent; >>>>> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >>>>> + >>>>> + tmp->config_vector = vdev->config_vector; >>>>> +} >>>>> + >>>>> +static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id) >>>>> +{ >>>>> + VirtioCcwDeviceTmp *tmp = opaque; >>>>> + VirtioCcwDevice *dev = tmp->parent; >>>>> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >>>>> + >>>>> + vdev->config_vector = tmp->config_vector; >>>>> + return 0; >>>>> +} >>>>> + >>>>> +const VMStateDescription vmstate_virtio_ccw_dev_tmp = { >>>>> + .name = "s390_virtio_ccw_dev_tmp", >>>>> + .pre_save = virtio_ccw_dev_tmp_pre_save, >>>>> + .post_load = virtio_ccw_dev_tmp_post_load, >>>>> + .fields = (VMStateField[]) { >>>>> + VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp), >>>>> + VMSTATE_END_OF_LIST() >>>>> + } >>>>> +}; >>>>> + >>>>> +const VMStateDescription vmstate_virtio_ccw_dev = { >>>>> + .name = "s390_virtio_ccw_dev", >>>>> + .version_id = 1, >>>>> + .minimum_version_id = 1, >>>>> + .post_load = virtio_ccw_dev_post_load, >>>>> + .fields = (VMStateField[]) { >>>>> + VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice), >>>>> + VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), >>>>> + VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), >>>>> + VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), >>>>> + /* >>>>> + * Ugly hack because VirtIODevice does not migrate itself. >>>>> + * This also makes legacy via vmstate_save_state possible. >>>>> + */ >>>>> + VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp, >>>>> + vmstate_virtio_ccw_dev_tmp), >>>>> + VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \ >>>>> + AdapterRoutes), >>>>> + VMSTATE_UINT8(thinint_isc, VirtioCcwDevice), >>>>> + VMSTATE_INT32(revision, VirtioCcwDevice), >>>>> + VMSTATE_END_OF_LIST() >>>>> + } >>>>> +}; >>>>> + >>>>> static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size, >>>>> VirtioCcwDevice *dev); >>>>> >>>>> @@ -1234,85 +1312,13 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f) >>>>> static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) >>>>> { >>>>> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >>>>> - CcwDevice *ccw_dev = CCW_DEVICE(d); >>>>> - SubchDev *s = ccw_dev->sch; >>>>> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); >>>>> - >>>>> - subch_device_save(s, f); >>>>> - if (dev->indicators != NULL) { >>>>> - qemu_put_be32(f, dev->indicators->len); >>>>> - qemu_put_be64(f, dev->indicators->addr); >>>>> - } else { >>>>> - qemu_put_be32(f, 0); >>>>> - qemu_put_be64(f, 0UL); >>>>> - } >>>>> - if (dev->indicators2 != NULL) { >>>>> - qemu_put_be32(f, dev->indicators2->len); >>>>> - qemu_put_be64(f, dev->indicators2->addr); >>>>> - } else { >>>>> - qemu_put_be32(f, 0); >>>>> - qemu_put_be64(f, 0UL); >>>>> - } >>>>> - if (dev->summary_indicator != NULL) { >>>>> - qemu_put_be32(f, dev->summary_indicator->len); >>>>> - qemu_put_be64(f, dev->summary_indicator->addr); >>>>> - } else { >>>>> - qemu_put_be32(f, 0); >>>>> - qemu_put_be64(f, 0UL); >>>>> - } >>>>> - qemu_put_be16(f, vdev->config_vector); >>>>> - qemu_put_be64(f, dev->routes.adapter.ind_offset); >>>>> - qemu_put_byte(f, dev->thinint_isc); >>>>> - qemu_put_be32(f, dev->revision); >>>>> + vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL); >>>>> } >>>>> >>>>> static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) >>>>> { >>>>> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >>>>> - CcwDevice *ccw_dev = CCW_DEVICE(d); >>>>> - CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >>>>> - SubchDev *s = ccw_dev->sch; >>>>> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); >>>>> - int len; >>>>> - >>>>> - s->driver_data = dev; >>>>> - subch_device_load(s, f); >>>>> - /* Re-fill subch_id after loading the subchannel states.*/ >>>>> - if (ck->refill_ids) { >>>>> - ck->refill_ids(ccw_dev); >>>>> - } >>>>> - len = qemu_get_be32(f); >>>>> - if (len != 0) { >>>>> - dev->indicators = get_indicator(qemu_get_be64(f), len); >>>>> - } else { >>>>> - qemu_get_be64(f); >>>>> - dev->indicators = NULL; >>>>> - } >>>>> - len = qemu_get_be32(f); >>>>> - if (len != 0) { >>>>> - dev->indicators2 = get_indicator(qemu_get_be64(f), len); >>>>> - } else { >>>>> - qemu_get_be64(f); >>>>> - dev->indicators2 = NULL; >>>>> - } >>>>> - len = qemu_get_be32(f); >>>>> - if (len != 0) { >>>>> - dev->summary_indicator = get_indicator(qemu_get_be64(f), len); >>>>> - } else { >>>>> - qemu_get_be64(f); >>>>> - dev->summary_indicator = NULL; >>>>> - } >>>>> - qemu_get_be16s(f, &vdev->config_vector); >>>>> - dev->routes.adapter.ind_offset = qemu_get_be64(f); >>>>> - dev->thinint_isc = qemu_get_byte(f); >>>>> - dev->revision = qemu_get_be32(f); >>>>> - if (s->thinint_active) { >>>>> - dev->routes.adapter.adapter_id = css_get_adapter_id( >>>>> - CSS_IO_ADAPTER_VIRTIO, >>>>> - dev->thinint_isc); >>>>> - } >>>>> - >>>>> - return 0; >>>>> + return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1); >>>>> } >>>>> >>>>> static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) >>>>> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h >>>>> index e61fa74d9b..d74e44d312 100644 >>>>> --- a/include/hw/s390x/css.h >>>>> +++ b/include/hw/s390x/css.h >>>>> @@ -88,6 +88,7 @@ struct SubchDev { >>>>> bool ccw_fmt_1; >>>>> bool thinint_active; >>>>> uint8_t ccw_no_data_cnt; >>>>> + uint16_t migrated_schid; /* used for missmatch detection */ >>>>> /* transport-provided data: */ >>>>> int (*ccw_cb) (SubchDev *, CCW1); >>>>> void (*disable_cb)(SubchDev *); >>>>> @@ -95,22 +96,27 @@ struct SubchDev { >>>>> void *driver_data; >>>>> }; >>>>> >>>>> +extern const VMStateDescription vmstate_subch_dev; >>>>> + >>>>> typedef struct IndAddr { >>>>> hwaddr addr; >>>>> uint64_t map; >>>>> unsigned long refcnt; >>>>> - int len; >>>>> + int32_t len; >>>>> QTAILQ_ENTRY(IndAddr) sibling; >>>>> } IndAddr; >>>>> >>>>> +extern const VMStateDescription vmstate_ind_addr; >>>>> + >>>>> +#define VMSTATE_PTR_TO_IND_ADDR(_f, _s) \ >>>>> + VMSTATE_STRUCT(_f, _s, 1, vmstate_ind_addr, IndAddr*) >>>>> + >>>>> IndAddr *get_indicator(hwaddr ind_addr, int len); >>>>> void release_indicator(AdapterInfo *adapter, IndAddr *indicator); >>>>> int map_indicator(AdapterInfo *adapter, IndAddr *indicator); >>>>> >>>>> typedef SubchDev *(*css_subch_cb_func)(uint8_t m, uint8_t cssid, uint8_t ssid, >>>>> uint16_t schid); >>>>> -void subch_device_save(SubchDev *s, QEMUFile *f); >>>>> -int subch_device_load(SubchDev *s, QEMUFile *f); >>>>> int css_create_css_image(uint8_t cssid, bool default_image); >>>>> bool css_devno_used(uint8_t cssid, uint8_t ssid, uint16_t devno); >>>>> void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, >>>>> diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h >>>>> index f9e6890c90..caa6fc608d 100644 >>>>> --- a/include/hw/s390x/s390_flic.h >>>>> +++ b/include/hw/s390x/s390_flic.h >>>>> @@ -31,6 +31,11 @@ typedef struct AdapterRoutes { >>>>> int gsi[ADAPTER_ROUTES_MAX_GSI]; >>>>> } AdapterRoutes; >>>>> >>>>> +extern const VMStateDescription vmstate_adapter_routes; >>>>> + >>>>> +#define VMSTATE_ADAPTER_ROUTES(_f, _s) \ >>>>> + VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes) >>>>> + >>>>> #define TYPE_S390_FLIC_COMMON "s390-flic" >>>>> #define S390_FLIC_COMMON(obj) \ >>>>> OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON) >>>> >>>> Other than the nit above, looks good to me. Especially the migration of >>>> the pmcw & co. is now more readable. So, with the nit fixed, >>>> >>>> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> >>> >>> Thanks! >>> >>>> >>>> Christian, can you please take this? >>>> >>> >>> Should I do a re-spin with the nit's picked and the r-b's added? >> >> Yes please. Can you then give me a confirmation that you have tested a migration between 2.9 and this patch set? >> >> > > I have tested between 2.5 and this (2.5 binary and compat both). I think > that should be equivalent with testing with 2.9, but I can give it a > couple of spins with 2.9. Yes, 2.5 is even better.
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c index 711c11454f..7e7546a576 100644 --- a/hw/intc/s390_flic.c +++ b/hw/intc/s390_flic.c @@ -18,6 +18,7 @@ #include "trace.h" #include "hw/qdev.h" #include "qapi/error.h" +#include "hw/s390x/s390-virtio-ccw.h" S390FLICState *s390_get_flic(void) { @@ -137,3 +138,30 @@ static void qemu_s390_flic_register_types(void) } type_init(qemu_s390_flic_register_types) + +const VMStateDescription vmstate_adapter_info = { + .name = "s390_adapter_info", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT64(ind_offset, AdapterInfo), + /* + * We do not have to migrate neither the id nor the addresses. + * The id is set by css_register_io_adapter and the addresses + * are set based on the IndAddr objects after those get mapped. + */ + VMSTATE_END_OF_LIST() + }, +}; + +const VMStateDescription vmstate_adapter_routes = { + + .name = "s390_adapter_routes", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_STRUCT(adapter, AdapterRoutes, 1, vmstate_adapter_info, \ + AdapterInfo), + VMSTATE_END_OF_LIST() + } +}; diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c index fb8d640a7e..f9bfa154d6 100644 --- a/hw/s390x/ccw-device.c +++ b/hw/s390x/ccw-device.c @@ -50,6 +50,16 @@ static void ccw_device_class_init(ObjectClass *klass, void *data) dc->props = ccw_device_properties; } +const VMStateDescription vmstate_ccw_dev = { + .name = "s390_ccw_dev", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_STRUCT_POINTER(sch, CcwDevice, vmstate_subch_dev, SubchDev), + VMSTATE_END_OF_LIST() + } +}; + static const TypeInfo ccw_device_info = { .name = TYPE_CCW_DEVICE, .parent = TYPE_DEVICE, diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h index 89c8e5dff7..4e6af287e7 100644 --- a/hw/s390x/ccw-device.h +++ b/hw/s390x/ccw-device.h @@ -27,6 +27,10 @@ typedef struct CcwDevice { CssDevId subch_id; } CcwDevice; +extern const VMStateDescription vmstate_ccw_dev; +#define VMSTATE_CCW_DEVICE(_field, _state) \ + VMSTATE_STRUCT(_field, _state, 1, vmstate_ccw_dev, CcwDevice) + typedef struct CCWDeviceClass { DeviceClass parent_class; void (*unplug)(HotplugHandler *, DeviceState *, Error **); diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 15c4f4b249..15517a16e4 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -20,6 +20,7 @@ #include "hw/s390x/css.h" #include "trace.h" #include "hw/s390x/s390_flic.h" +#include "hw/s390x/s390-virtio-ccw.h" typedef struct CrwContainer { CRW crw; @@ -38,6 +39,177 @@ typedef struct SubchSet { unsigned long devnos_used[BITS_TO_LONGS(MAX_SCHID + 1)]; } SubchSet; +static const VMStateDescription vmstate_scsw = { + .name = "s390_scsw", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT16(flags, SCSW), + VMSTATE_UINT16(ctrl, SCSW), + VMSTATE_UINT32(cpa, SCSW), + VMSTATE_UINT8(dstat, SCSW), + VMSTATE_UINT8(cstat, SCSW), + VMSTATE_UINT16(count, SCSW), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_pmcw = { + .name = "s390_pmcw", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(intparm, PMCW), + VMSTATE_UINT16(flags, PMCW), + VMSTATE_UINT16(devno, PMCW), + VMSTATE_UINT8(lpm, PMCW), + VMSTATE_UINT8(pnom, PMCW), + VMSTATE_UINT8(lpum, PMCW), + VMSTATE_UINT8(pim, PMCW), + VMSTATE_UINT16(mbi, PMCW), + VMSTATE_UINT8(pom, PMCW), + VMSTATE_UINT8(pam, PMCW), + VMSTATE_UINT8_ARRAY(chpid, PMCW, 8), + VMSTATE_UINT32(chars, PMCW), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_schib = { + .name = "s390_schib", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_STRUCT(pmcw, SCHIB, 0, vmstate_pmcw, PMCW), + VMSTATE_STRUCT(scsw, SCHIB, 0, vmstate_scsw, SCSW), + VMSTATE_UINT64(mba, SCHIB), + VMSTATE_UINT8_ARRAY(mda, SCHIB, 4), + VMSTATE_END_OF_LIST() + } +}; + + +static const VMStateDescription vmstate_ccw1 = { + .name = "s390_ccw1", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT8(cmd_code, CCW1), + VMSTATE_UINT8(flags, CCW1), + VMSTATE_UINT16(count, CCW1), + VMSTATE_UINT32(cda, CCW1), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_ciw = { + .name = "s390_ciw", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT8(type, CIW), + VMSTATE_UINT8(command, CIW), + VMSTATE_UINT16(count, CIW), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_sense_id = { + .name = "s390_sense_id", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT8(reserved, SenseId), + VMSTATE_UINT16(cu_type, SenseId), + VMSTATE_UINT8(cu_model, SenseId), + VMSTATE_UINT16(dev_type, SenseId), + VMSTATE_UINT8(dev_model, SenseId), + VMSTATE_UINT8(unused, SenseId), + VMSTATE_STRUCT_ARRAY(ciw, SenseId, MAX_CIWS, 0, vmstate_ciw, CIW), + VMSTATE_END_OF_LIST() + } +}; + +static int subch_dev_post_load(void *opaque, int version_id); +static void subch_dev_pre_save(void *opaque); + +const VMStateDescription vmstate_subch_dev = { + .name = "s390_subch_dev", + .version_id = 1, + .minimum_version_id = 1, + .post_load = subch_dev_post_load, + .pre_save = subch_dev_pre_save, + .fields = (VMStateField[]) { + VMSTATE_UINT8_EQUAL(cssid, SubchDev), + VMSTATE_UINT8_EQUAL(ssid, SubchDev), + VMSTATE_UINT16(migrated_schid, SubchDev), + VMSTATE_UINT16(devno, SubchDev), + VMSTATE_BOOL(thinint_active, SubchDev), + VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB), + VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32), + VMSTATE_UINT64(channel_prog, SubchDev), + VMSTATE_STRUCT(last_cmd, SubchDev, 0, vmstate_ccw1, CCW1), + VMSTATE_BOOL(last_cmd_valid, SubchDev), + VMSTATE_STRUCT(id, SubchDev, 0, vmstate_sense_id, SenseId), + VMSTATE_BOOL(ccw_fmt_1, SubchDev), + VMSTATE_UINT8(ccw_no_data_cnt, SubchDev), + VMSTATE_END_OF_LIST() + } +}; + +typedef struct IndAddrPtrTmp { + IndAddr **parent; + uint64_t addr; + int32_t len; +} IndAddrPtrTmp; + +static int post_load_ind_addr(void *opaque, int version_id) +{ + IndAddrPtrTmp *ptmp = opaque; + IndAddr **ind_addr = ptmp->parent; + + if (ptmp->len != 0) { + *ind_addr = get_indicator(ptmp->addr, ptmp->len); + } else { + *ind_addr = NULL; + } + return 0; +} + +static void pre_save_ind_addr(void *opaque) +{ + IndAddrPtrTmp *ptmp = opaque; + IndAddr *ind_addr = *(ptmp->parent); + + if (ind_addr != NULL) { + ptmp->len = ind_addr->len; + ptmp->addr = ind_addr->addr; + } else { + ptmp->len = 0; + ptmp->addr = 0L; + } +} + +const VMStateDescription vmstate_ind_addr_tmp = { + .name = "s390_ind_addr_tmp", + .pre_save = pre_save_ind_addr, + .post_load = post_load_ind_addr, + + .fields = (VMStateField[]) { + VMSTATE_INT32(len, IndAddrPtrTmp), + VMSTATE_UINT64(addr, IndAddrPtrTmp), + VMSTATE_END_OF_LIST() + } +}; + +const VMStateDescription vmstate_ind_addr = { + .name = "s390_ind_addr_tmp", + .fields = (VMStateField[]) { + VMSTATE_WITH_TMP(IndAddr*, IndAddrPtrTmp, vmstate_ind_addr_tmp), + VMSTATE_END_OF_LIST() + } +}; + typedef struct CssImage { SubchSet *sch_set[MAX_SSID + 1]; ChpInfo chpids[MAX_CHPID + 1]; @@ -75,6 +247,52 @@ static ChannelSubSys channel_subsys = { QTAILQ_HEAD_INITIALIZER(channel_subsys.indicator_addresses), }; +static void subch_dev_pre_save(void *opaque) +{ + SubchDev *s = opaque; + + /* Prepare remote_schid for save */ + s->migrated_schid = s->schid; +} + +static int subch_dev_post_load(void *opaque, int version_id) +{ + + SubchDev *s = opaque; + + /* Re-assign the subchannel to remote_schid if necessary */ + if (s->migrated_schid != s->schid) { + if (css_find_subch(true, s->cssid, s->ssid, s->schid) == s) { + /* + * Cleanup the slot before moving to s->migrated_schid provided + * it still belongs to us, i.e. it was not changed by previous + * invocation of this function. + */ + css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, NULL); + } + /* It's OK to re-assign without a prior de-assign. */ + s->schid = s->migrated_schid; + css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s); + } + + /* + * Hack alert. If we don't migrate the channel subsystem status + * we still need to find out if the guest enabled mss/mcss-e. + * If the subchannel is enabled, it certainly was able to access it, + * so adjust the max_ssid/max_cssid values for relevant ssid/cssid + * values. This is not watertight, but better than nothing. + */ + if (s->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA) { + if (s->ssid) { + channel_subsys.max_ssid = MAX_SSID; + } + if (s->cssid != channel_subsys.default_cssid) { + channel_subsys.max_cssid = MAX_CSSID; + } + } + return 0; +} + IndAddr *get_indicator(hwaddr ind_addr, int len) { IndAddr *indicator; @@ -1662,149 +1880,6 @@ int css_enable_mss(void) return 0; } -void subch_device_save(SubchDev *s, QEMUFile *f) -{ - int i; - - qemu_put_byte(f, s->cssid); - qemu_put_byte(f, s->ssid); - qemu_put_be16(f, s->schid); - qemu_put_be16(f, s->devno); - qemu_put_byte(f, s->thinint_active); - /* SCHIB */ - /* PMCW */ - qemu_put_be32(f, s->curr_status.pmcw.intparm); - qemu_put_be16(f, s->curr_status.pmcw.flags); - qemu_put_be16(f, s->curr_status.pmcw.devno); - qemu_put_byte(f, s->curr_status.pmcw.lpm); - qemu_put_byte(f, s->curr_status.pmcw.pnom); - qemu_put_byte(f, s->curr_status.pmcw.lpum); - qemu_put_byte(f, s->curr_status.pmcw.pim); - qemu_put_be16(f, s->curr_status.pmcw.mbi); - qemu_put_byte(f, s->curr_status.pmcw.pom); - qemu_put_byte(f, s->curr_status.pmcw.pam); - qemu_put_buffer(f, s->curr_status.pmcw.chpid, 8); - qemu_put_be32(f, s->curr_status.pmcw.chars); - /* SCSW */ - qemu_put_be16(f, s->curr_status.scsw.flags); - qemu_put_be16(f, s->curr_status.scsw.ctrl); - qemu_put_be32(f, s->curr_status.scsw.cpa); - qemu_put_byte(f, s->curr_status.scsw.dstat); - qemu_put_byte(f, s->curr_status.scsw.cstat); - qemu_put_be16(f, s->curr_status.scsw.count); - qemu_put_be64(f, s->curr_status.mba); - qemu_put_buffer(f, s->curr_status.mda, 4); - /* end SCHIB */ - qemu_put_buffer(f, s->sense_data, 32); - qemu_put_be64(f, s->channel_prog); - /* last cmd */ - qemu_put_byte(f, s->last_cmd.cmd_code); - qemu_put_byte(f, s->last_cmd.flags); - qemu_put_be16(f, s->last_cmd.count); - qemu_put_be32(f, s->last_cmd.cda); - qemu_put_byte(f, s->last_cmd_valid); - qemu_put_byte(f, s->id.reserved); - qemu_put_be16(f, s->id.cu_type); - qemu_put_byte(f, s->id.cu_model); - qemu_put_be16(f, s->id.dev_type); - qemu_put_byte(f, s->id.dev_model); - qemu_put_byte(f, s->id.unused); - for (i = 0; i < ARRAY_SIZE(s->id.ciw); i++) { - qemu_put_byte(f, s->id.ciw[i].type); - qemu_put_byte(f, s->id.ciw[i].command); - qemu_put_be16(f, s->id.ciw[i].count); - } - qemu_put_byte(f, s->ccw_fmt_1); - qemu_put_byte(f, s->ccw_no_data_cnt); -} - -int subch_device_load(SubchDev *s, QEMUFile *f) -{ - SubchDev *old_s; - uint16_t old_schid = s->schid; - int i; - - s->cssid = qemu_get_byte(f); - s->ssid = qemu_get_byte(f); - s->schid = qemu_get_be16(f); - s->devno = qemu_get_be16(f); - /* Re-assign subch. */ - if (old_schid != s->schid) { - old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid]; - /* - * (old_s != s) means that some other device has its correct - * subchannel already assigned (in load). - */ - if (old_s == s) { - css_subch_assign(s->cssid, s->ssid, old_schid, s->devno, NULL); - } - /* It's OK to re-assign without a prior de-assign. */ - css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s); - } - s->thinint_active = qemu_get_byte(f); - /* SCHIB */ - /* PMCW */ - s->curr_status.pmcw.intparm = qemu_get_be32(f); - s->curr_status.pmcw.flags = qemu_get_be16(f); - s->curr_status.pmcw.devno = qemu_get_be16(f); - s->curr_status.pmcw.lpm = qemu_get_byte(f); - s->curr_status.pmcw.pnom = qemu_get_byte(f); - s->curr_status.pmcw.lpum = qemu_get_byte(f); - s->curr_status.pmcw.pim = qemu_get_byte(f); - s->curr_status.pmcw.mbi = qemu_get_be16(f); - s->curr_status.pmcw.pom = qemu_get_byte(f); - s->curr_status.pmcw.pam = qemu_get_byte(f); - qemu_get_buffer(f, s->curr_status.pmcw.chpid, 8); - s->curr_status.pmcw.chars = qemu_get_be32(f); - /* SCSW */ - s->curr_status.scsw.flags = qemu_get_be16(f); - s->curr_status.scsw.ctrl = qemu_get_be16(f); - s->curr_status.scsw.cpa = qemu_get_be32(f); - s->curr_status.scsw.dstat = qemu_get_byte(f); - s->curr_status.scsw.cstat = qemu_get_byte(f); - s->curr_status.scsw.count = qemu_get_be16(f); - s->curr_status.mba = qemu_get_be64(f); - qemu_get_buffer(f, s->curr_status.mda, 4); - /* end SCHIB */ - qemu_get_buffer(f, s->sense_data, 32); - s->channel_prog = qemu_get_be64(f); - /* last cmd */ - s->last_cmd.cmd_code = qemu_get_byte(f); - s->last_cmd.flags = qemu_get_byte(f); - s->last_cmd.count = qemu_get_be16(f); - s->last_cmd.cda = qemu_get_be32(f); - s->last_cmd_valid = qemu_get_byte(f); - s->id.reserved = qemu_get_byte(f); - s->id.cu_type = qemu_get_be16(f); - s->id.cu_model = qemu_get_byte(f); - s->id.dev_type = qemu_get_be16(f); - s->id.dev_model = qemu_get_byte(f); - s->id.unused = qemu_get_byte(f); - for (i = 0; i < ARRAY_SIZE(s->id.ciw); i++) { - s->id.ciw[i].type = qemu_get_byte(f); - s->id.ciw[i].command = qemu_get_byte(f); - s->id.ciw[i].count = qemu_get_be16(f); - } - s->ccw_fmt_1 = qemu_get_byte(f); - s->ccw_no_data_cnt = qemu_get_byte(f); - /* - * Hack alert. We don't migrate the channel subsystem status (no - * device!), but we need to find out if the guest enabled mss/mcss-e. - * If the subchannel is enabled, it certainly was able to access it, - * so adjust the max_ssid/max_cssid values for relevant ssid/cssid - * values. This is not watertight, but better than nothing. - */ - if (s->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA) { - if (s->ssid) { - channel_subsys.max_ssid = MAX_SSID; - } - if (s->cssid != channel_subsys.default_cssid) { - channel_subsys.max_cssid = MAX_CSSID; - } - } - return 0; -} - void css_reset_sch(SubchDev *sch) { PMCW *p = &sch->curr_status.pmcw; diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index e7167e3d05..a75eedeffb 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -34,9 +34,87 @@ #include "virtio-ccw.h" #include "trace.h" #include "hw/s390x/css-bridge.h" +#include "hw/s390x/s390-virtio-ccw.h" #define NR_CLASSIC_INDICATOR_BITS 64 +static int virtio_ccw_dev_post_load(void *opaque, int version_id) +{ + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(opaque); + CcwDevice *ccw_dev = CCW_DEVICE(dev); + CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); + + ccw_dev->sch->driver_data = dev; + if (CCW_DEVICE(dev)->sch->thinint_active) { + dev->routes.adapter.adapter_id = css_get_adapter_id( + CSS_IO_ADAPTER_VIRTIO, + dev->thinint_isc); + } + /* Re-fill subch_id after loading the subchannel states.*/ + if (ck->refill_ids) { + ck->refill_ids(ccw_dev); + } + return 0; +} + +typedef struct VirtioCcwDeviceTmp { + VirtioCcwDevice *parent; + uint16_t config_vector; +} VirtioCcwDeviceTmp; + +static void virtio_ccw_dev_tmp_pre_save(void *opaque) +{ + VirtioCcwDeviceTmp *tmp = opaque; + VirtioCcwDevice *dev = tmp->parent; + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); + + tmp->config_vector = vdev->config_vector; +} + +static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id) +{ + VirtioCcwDeviceTmp *tmp = opaque; + VirtioCcwDevice *dev = tmp->parent; + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); + + vdev->config_vector = tmp->config_vector; + return 0; +} + +const VMStateDescription vmstate_virtio_ccw_dev_tmp = { + .name = "s390_virtio_ccw_dev_tmp", + .pre_save = virtio_ccw_dev_tmp_pre_save, + .post_load = virtio_ccw_dev_tmp_post_load, + .fields = (VMStateField[]) { + VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp), + VMSTATE_END_OF_LIST() + } +}; + +const VMStateDescription vmstate_virtio_ccw_dev = { + .name = "s390_virtio_ccw_dev", + .version_id = 1, + .minimum_version_id = 1, + .post_load = virtio_ccw_dev_post_load, + .fields = (VMStateField[]) { + VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice), + VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), + VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), + VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), + /* + * Ugly hack because VirtIODevice does not migrate itself. + * This also makes legacy via vmstate_save_state possible. + */ + VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp, + vmstate_virtio_ccw_dev_tmp), + VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \ + AdapterRoutes), + VMSTATE_UINT8(thinint_isc, VirtioCcwDevice), + VMSTATE_INT32(revision, VirtioCcwDevice), + VMSTATE_END_OF_LIST() + } +}; + static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size, VirtioCcwDevice *dev); @@ -1234,85 +1312,13 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f) static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) { VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); - CcwDevice *ccw_dev = CCW_DEVICE(d); - SubchDev *s = ccw_dev->sch; - VirtIODevice *vdev = virtio_ccw_get_vdev(s); - - subch_device_save(s, f); - if (dev->indicators != NULL) { - qemu_put_be32(f, dev->indicators->len); - qemu_put_be64(f, dev->indicators->addr); - } else { - qemu_put_be32(f, 0); - qemu_put_be64(f, 0UL); - } - if (dev->indicators2 != NULL) { - qemu_put_be32(f, dev->indicators2->len); - qemu_put_be64(f, dev->indicators2->addr); - } else { - qemu_put_be32(f, 0); - qemu_put_be64(f, 0UL); - } - if (dev->summary_indicator != NULL) { - qemu_put_be32(f, dev->summary_indicator->len); - qemu_put_be64(f, dev->summary_indicator->addr); - } else { - qemu_put_be32(f, 0); - qemu_put_be64(f, 0UL); - } - qemu_put_be16(f, vdev->config_vector); - qemu_put_be64(f, dev->routes.adapter.ind_offset); - qemu_put_byte(f, dev->thinint_isc); - qemu_put_be32(f, dev->revision); + vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL); } static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) { VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); - CcwDevice *ccw_dev = CCW_DEVICE(d); - CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); - SubchDev *s = ccw_dev->sch; - VirtIODevice *vdev = virtio_ccw_get_vdev(s); - int len; - - s->driver_data = dev; - subch_device_load(s, f); - /* Re-fill subch_id after loading the subchannel states.*/ - if (ck->refill_ids) { - ck->refill_ids(ccw_dev); - } - len = qemu_get_be32(f); - if (len != 0) { - dev->indicators = get_indicator(qemu_get_be64(f), len); - } else { - qemu_get_be64(f); - dev->indicators = NULL; - } - len = qemu_get_be32(f); - if (len != 0) { - dev->indicators2 = get_indicator(qemu_get_be64(f), len); - } else { - qemu_get_be64(f); - dev->indicators2 = NULL; - } - len = qemu_get_be32(f); - if (len != 0) { - dev->summary_indicator = get_indicator(qemu_get_be64(f), len); - } else { - qemu_get_be64(f); - dev->summary_indicator = NULL; - } - qemu_get_be16s(f, &vdev->config_vector); - dev->routes.adapter.ind_offset = qemu_get_be64(f); - dev->thinint_isc = qemu_get_byte(f); - dev->revision = qemu_get_be32(f); - if (s->thinint_active) { - dev->routes.adapter.adapter_id = css_get_adapter_id( - CSS_IO_ADAPTER_VIRTIO, - dev->thinint_isc); - } - - return 0; + return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1); } static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h index e61fa74d9b..d74e44d312 100644 --- a/include/hw/s390x/css.h +++ b/include/hw/s390x/css.h @@ -88,6 +88,7 @@ struct SubchDev { bool ccw_fmt_1; bool thinint_active; uint8_t ccw_no_data_cnt; + uint16_t migrated_schid; /* used for missmatch detection */ /* transport-provided data: */ int (*ccw_cb) (SubchDev *, CCW1); void (*disable_cb)(SubchDev *); @@ -95,22 +96,27 @@ struct SubchDev { void *driver_data; }; +extern const VMStateDescription vmstate_subch_dev; + typedef struct IndAddr { hwaddr addr; uint64_t map; unsigned long refcnt; - int len; + int32_t len; QTAILQ_ENTRY(IndAddr) sibling; } IndAddr; +extern const VMStateDescription vmstate_ind_addr; + +#define VMSTATE_PTR_TO_IND_ADDR(_f, _s) \ + VMSTATE_STRUCT(_f, _s, 1, vmstate_ind_addr, IndAddr*) + IndAddr *get_indicator(hwaddr ind_addr, int len); void release_indicator(AdapterInfo *adapter, IndAddr *indicator); int map_indicator(AdapterInfo *adapter, IndAddr *indicator); typedef SubchDev *(*css_subch_cb_func)(uint8_t m, uint8_t cssid, uint8_t ssid, uint16_t schid); -void subch_device_save(SubchDev *s, QEMUFile *f); -int subch_device_load(SubchDev *s, QEMUFile *f); int css_create_css_image(uint8_t cssid, bool default_image); bool css_devno_used(uint8_t cssid, uint8_t ssid, uint16_t devno); void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h index f9e6890c90..caa6fc608d 100644 --- a/include/hw/s390x/s390_flic.h +++ b/include/hw/s390x/s390_flic.h @@ -31,6 +31,11 @@ typedef struct AdapterRoutes { int gsi[ADAPTER_ROUTES_MAX_GSI]; } AdapterRoutes; +extern const VMStateDescription vmstate_adapter_routes; + +#define VMSTATE_ADAPTER_ROUTES(_f, _s) \ + VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes) + #define TYPE_S390_FLIC_COMMON "s390-flic" #define S390_FLIC_COMMON(obj) \ OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON)
Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for flexibility (extending using subsections) and for fun. To achieve this we need to hack the config_vector, which is VirtIODevice (that is common virtio) state, in the middle of the VirtioCcwDevice state representation. This somewhat ugly, but we have no choice because the stream format needs to be preserved. Almost no changes in behavior. Exception is everything that comes with vmstate like extra bookkeeping about what's in the stream, and maybe some extra checks and better error reporting. Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> --- This patch is related to the series 'migration: s390x css migration', in a sense that it's basically the compatibility preserving changes split out as requested by Dave. Note: This patch can be split in two parts, where the first part is limited to getting rid of the qemu_put's and qemu_gets from subch_device_save and subch_device_load. With that the scope of the second part would be concerned with the virtio_ccw_save_config and virtio_ccw_load_config. Would it significantly benefit readability -- probably not. --- hw/intc/s390_flic.c | 28 ++++ hw/s390x/ccw-device.c | 10 ++ hw/s390x/ccw-device.h | 4 + hw/s390x/css.c | 361 ++++++++++++++++++++++++++----------------- hw/s390x/virtio-ccw.c | 154 +++++++++--------- include/hw/s390x/css.h | 12 +- include/hw/s390x/s390_flic.h | 5 + 7 files changed, 354 insertions(+), 220 deletions(-)