* [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration
@ 2017-05-29 13:55 Halil Pasic
2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 1/7] s390x: vmstatify config migration for virtio-ccw Halil Pasic
` (8 more replies)
0 siblings, 9 replies; 24+ messages in thread
From: Halil Pasic @ 2017-05-29 13:55 UTC (permalink / raw)
To: Cornelia Huck, Dr. David Alan Gilbert
Cc: Dong Jia Shi, Juan Quintela, qemu-devel, Halil Pasic
The scope of this patch series is now limited to decoupling channel
subsystem migration from the migration of virtio-ccw proxies.
The vmstatifying of virtio-ccw proxies is now done by another series
currently consisting of a single patch running under the name 'vmstatify
config migration for virtio-ccw'. Since this series depends on that
patch, I have included it here too (for convenience).
The reorganization was requested by Dave.
v1 --> v2:
* Split out the vmystatify virtio-ccw part, reorganize
* Use WITH_TMP instead of one-shot VMStateInfo's
Halil Pasic (7):
s390x: vmstatify config migration for virtio-ccw
s390x: add helper get_machine_class
s390x: add css_migration_enabled to machine class
s390x/css: add missing css state conditionally
s390x/css: add ORB to SubchDev
s390x/css: activate ChannelSubSys migration
s390x/css: use SubchDev.orb
hw/intc/s390_flic.c | 48 ++++
hw/s390x/ccw-device.c | 10 +
hw/s390x/ccw-device.h | 4 +
hw/s390x/css.c | 494 +++++++++++++++++++++++++------------
hw/s390x/s390-virtio-ccw.c | 58 +++--
hw/s390x/virtio-ccw.c | 155 ++++++------
include/hw/s390x/css.h | 17 +-
include/hw/s390x/s390-virtio-ccw.h | 7 +
include/hw/s390x/s390_flic.h | 5 +
9 files changed, 544 insertions(+), 254 deletions(-)
--
2.11.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH v2 1/7] s390x: vmstatify config migration for virtio-ccw 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic @ 2017-05-29 13:55 ` Halil Pasic 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 2/7] s390x: add helper get_machine_class Halil Pasic ` (7 subsequent siblings) 8 siblings, 0 replies; 24+ messages in thread From: Halil Pasic @ 2017-05-29 13:55 UTC (permalink / raw) To: Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, Juan Quintela, qemu-devel, Halil Pasic 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> --- Note: Do not review this here! There is a stand alone patch on the list. Having this here is to is just for convenience. --- 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 2/7] s390x: add helper get_machine_class 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 1/7] s390x: vmstatify config migration for virtio-ccw Halil Pasic @ 2017-05-29 13:55 ` Halil Pasic 2017-06-01 11:13 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 3/7] s390x: add css_migration_enabled to machine class Halil Pasic ` (6 subsequent siblings) 8 siblings, 1 reply; 24+ messages in thread From: Halil Pasic @ 2017-05-29 13:55 UTC (permalink / raw) To: Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, Juan Quintela, qemu-devel, Halil Pasic We will need the machine class at machine initialization time, so the usual way via qdev won't do. Let's cache the machine class and also use the default values of the base machine for capability discovery. Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> --- hw/s390x/s390-virtio-ccw.c | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index fdd4384ff0..04148916ed 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -242,36 +242,35 @@ static inline void machine_set_dea_key_wrap(Object *obj, bool value, ms->dea_key_wrap = value; } -bool ri_allowed(void) -{ - if (kvm_enabled()) { - MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); - if (object_class_dynamic_cast(OBJECT_CLASS(mc), - TYPE_S390_CCW_MACHINE)) { - S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc); +static S390CcwMachineClass *current_mc; - return s390mc->ri_allowed; - } +static S390CcwMachineClass *get_machine_class(void) +{ + if (unlikely(!current_mc)) { /* - * Make sure the "none" machine can have ri, otherwise it won't * be - * unlocked in KVM and therefore the host CPU model might be wrong. - */ - return true; + * No s390 ccw machine was instantiated, we are likely to + * be called for the 'none' machine. The properties will + * have their after-initialization values. + */ + current_mc = S390_MACHINE_CLASS( + object_class_by_name(TYPE_S390_CCW_MACHINE)); } - return 0; + return current_mc; } -bool cpu_model_allowed(void) +bool ri_allowed(void) { - MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); - if (object_class_dynamic_cast(OBJECT_CLASS(mc), - TYPE_S390_CCW_MACHINE)) { - S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc); - - return s390mc->cpu_model_allowed; + if (!kvm_enabled()) { + return false; } - /* allow CPU model qmp queries with the "none" machine */ - return true; + /* for "none" machine this results in true */ + return get_machine_class()->ri_allowed; +} + +bool cpu_model_allowed(void) +{ + /* for "none" machine this results in true */ + return get_machine_class()->cpu_model_allowed; } static char *machine_get_loadparm(Object *obj, Error **errp) @@ -360,6 +359,7 @@ static const TypeInfo ccw_machine_info = { static void ccw_machine_##suffix##_instance_init(Object *obj) \ { \ MachineState *machine = MACHINE(obj); \ + current_mc = S390_MACHINE_CLASS(MACHINE_GET_CLASS(machine)); \ ccw_machine_##suffix##_instance_options(machine); \ } \ static const TypeInfo ccw_machine_##suffix##_info = { \ -- 2.11.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/7] s390x: add helper get_machine_class 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 2/7] s390x: add helper get_machine_class Halil Pasic @ 2017-06-01 11:13 ` Cornelia Huck 0 siblings, 0 replies; 24+ messages in thread From: Cornelia Huck @ 2017-06-01 11:13 UTC (permalink / raw) To: Halil Pasic Cc: Dr. David Alan Gilbert, Dong Jia Shi, Juan Quintela, qemu-devel On Mon, 29 May 2017 15:55:15 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > We will need the machine class at machine initialization time, so the > usual way via qdev won't do. Let's cache the machine class and also use > the default values of the base machine for capability discovery. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 46 +++++++++++++++++++++++----------------------- > 1 file changed, 23 insertions(+), 23 deletions(-) I'm still not sure if that is the best way to go; but if it isn't, I'm unable to find that better path. So, Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com> ^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 3/7] s390x: add css_migration_enabled to machine class 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 1/7] s390x: vmstatify config migration for virtio-ccw Halil Pasic 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 2/7] s390x: add helper get_machine_class Halil Pasic @ 2017-05-29 13:55 ` Halil Pasic 2017-06-01 11:16 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally Halil Pasic ` (5 subsequent siblings) 8 siblings, 1 reply; 24+ messages in thread From: Halil Pasic @ 2017-05-29 13:55 UTC (permalink / raw) To: Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, Juan Quintela, qemu-devel, Halil Pasic Currently the migration of the channel subsystem (css) is only partial and is done by the virtio ccw proxies -- the only migratable css devices existing at the moment. With the current work on emulated and passthrough devices we need to decouple the migration of the channel subsystem state from virtio ccw, and have a separate section for it. A new section however necessarily breaks the migration compatibility. So let us introduce a switch at the machine class, and put it in 'off' state for now. We will turn the switch 'on' for future machines once all preparations are met. For compatibility machines the switch will stay 'off'. Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> --- hw/s390x/s390-virtio-ccw.c | 13 +++++++++++++ include/hw/s390x/s390-virtio-ccw.h | 7 +++++++ 2 files changed, 20 insertions(+) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 04148916ed..95256d3982 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -196,6 +196,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) s390mc->ri_allowed = true; s390mc->cpu_model_allowed = true; + s390mc->css_migration_enabled = false; /* TODO: set to true */ mc->init = ccw_init; mc->reset = s390_machine_reset; mc->hot_add_cpu = s390_hot_add_cpu; @@ -344,6 +345,11 @@ static const TypeInfo ccw_machine_info = { }, }; +bool css_migration_enabled(void) +{ + return get_machine_class()->css_migration_enabled; +} + #define DEFINE_CCW_MACHINE(suffix, verstr, latest) \ static void ccw_machine_##suffix##_class_init(ObjectClass *oc, \ void *data) \ @@ -445,6 +451,10 @@ static const TypeInfo ccw_machine_info = { static void ccw_machine_2_10_instance_options(MachineState *machine) { + /* + * TODO Once preparations are done register vmstate for the css if + * css_migration_enabled(). + */ } static void ccw_machine_2_10_class_options(MachineClass *mc) @@ -459,8 +469,11 @@ static void ccw_machine_2_9_instance_options(MachineState *machine) static void ccw_machine_2_9_class_options(MachineClass *mc) { + S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc); + ccw_machine_2_10_class_options(mc); SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9); + s390mc->css_migration_enabled = false; } DEFINE_CCW_MACHINE(2_9, "2.9", false); diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h index 7b8a3e4d74..e9c4f4182b 100644 --- a/include/hw/s390x/s390-virtio-ccw.h +++ b/include/hw/s390x/s390-virtio-ccw.h @@ -38,6 +38,7 @@ typedef struct S390CcwMachineClass { /*< public >*/ bool ri_allowed; bool cpu_model_allowed; + bool css_migration_enabled; } S390CcwMachineClass; /* runtime-instrumentation allowed by the machine */ @@ -45,4 +46,10 @@ bool ri_allowed(void); /* cpu model allowed by the machine */ bool cpu_model_allowed(void); +/** + * Returns true if (vmstate based) migration of the channel subsystem + * is enabled, false if it is disabled. + */ +bool css_migration_enabled(void); + #endif -- 2.11.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/7] s390x: add css_migration_enabled to machine class 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 3/7] s390x: add css_migration_enabled to machine class Halil Pasic @ 2017-06-01 11:16 ` Cornelia Huck 0 siblings, 0 replies; 24+ messages in thread From: Cornelia Huck @ 2017-06-01 11:16 UTC (permalink / raw) To: Halil Pasic Cc: Dr. David Alan Gilbert, Dong Jia Shi, Juan Quintela, qemu-devel On Mon, 29 May 2017 15:55:16 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Currently the migration of the channel subsystem (css) is only partial > and is done by the virtio ccw proxies -- the only migratable css devices extra blank ---------------------------------------------------^ > existing at the moment. > > With the current work on emulated and passthrough devices we need to > decouple the migration of the channel subsystem state from virtio ccw, > and have a separate section for it. A new section however necessarily > breaks the migration compatibility. > > So let us introduce a switch at the machine class, and put it in 'off' > state for now. We will turn the switch 'on' for future machines once all > preparations are met. For compatibility machines the switch will stay > 'off'. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 13 +++++++++++++ > include/hw/s390x/s390-virtio-ccw.h | 7 +++++++ > 2 files changed, 20 insertions(+) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 04148916ed..95256d3982 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -196,6 +196,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) > > s390mc->ri_allowed = true; > s390mc->cpu_model_allowed = true; > + s390mc->css_migration_enabled = false; /* TODO: set to true */ > mc->init = ccw_init; > mc->reset = s390_machine_reset; > mc->hot_add_cpu = s390_hot_add_cpu; > @@ -344,6 +345,11 @@ static const TypeInfo ccw_machine_info = { > }, > }; > > +bool css_migration_enabled(void) > +{ > + return get_machine_class()->css_migration_enabled; > +} > + > #define DEFINE_CCW_MACHINE(suffix, verstr, latest) \ > static void ccw_machine_##suffix##_class_init(ObjectClass *oc, \ > void *data) \ > @@ -445,6 +451,10 @@ static const TypeInfo ccw_machine_info = { > > static void ccw_machine_2_10_instance_options(MachineState *machine) > { > + /* > + * TODO Once preparations are done register vmstate for the css if > + * css_migration_enabled(). > + */ > } > > static void ccw_machine_2_10_class_options(MachineClass *mc) > @@ -459,8 +469,11 @@ static void ccw_machine_2_9_instance_options(MachineState *machine) > > static void ccw_machine_2_9_class_options(MachineClass *mc) > { > + S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc); > + > ccw_machine_2_10_class_options(mc); > SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9); > + s390mc->css_migration_enabled = false; > } > DEFINE_CCW_MACHINE(2_9, "2.9", false); > > diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h > index 7b8a3e4d74..e9c4f4182b 100644 > --- a/include/hw/s390x/s390-virtio-ccw.h > +++ b/include/hw/s390x/s390-virtio-ccw.h > @@ -38,6 +38,7 @@ typedef struct S390CcwMachineClass { > /*< public >*/ > bool ri_allowed; > bool cpu_model_allowed; > + bool css_migration_enabled; > } S390CcwMachineClass; > > /* runtime-instrumentation allowed by the machine */ > @@ -45,4 +46,10 @@ bool ri_allowed(void); > /* cpu model allowed by the machine */ > bool cpu_model_allowed(void); > > +/** > + * Returns true if (vmstate based) migration of the channel subsystem > + * is enabled, false if it is disabled. > + */ > +bool css_migration_enabled(void); > + > #endif Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com> ^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic ` (2 preceding siblings ...) 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 3/7] s390x: add css_migration_enabled to machine class Halil Pasic @ 2017-05-29 13:55 ` Halil Pasic 2017-05-31 18:52 ` Juan Quintela 2017-06-01 11:42 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 5/7] s390x/css: add ORB to SubchDev Halil Pasic ` (4 subsequent siblings) 8 siblings, 2 replies; 24+ messages in thread From: Halil Pasic @ 2017-05-29 13:55 UTC (permalink / raw) To: Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, Juan Quintela, qemu-devel, Halil Pasic Although we have recently vmstatified the migration of some css infrastructure, for some css entities there is still state to be migrated left, because the focus was keeping migration stream compatibility (that is basically everything as-is). Let us add vmstate helpers and extend existing vmstate descriptions so that we have everything we need. Let us guard the added state with via css_migration_enabled, so we keep the compatible behavior if css migration is disabled. Let's also annotate the bits which do not need to be migrated for better readability. Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> --- hw/intc/s390_flic.c | 20 +++++++++++++++ hw/s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c index 7e7546a576..b889e74571 100644 --- a/hw/intc/s390_flic.c +++ b/hw/intc/s390_flic.c @@ -139,6 +139,22 @@ static void qemu_s390_flic_register_types(void) type_init(qemu_s390_flic_register_types) +static bool adapter_info_so_needed(void *opaque) +{ + return css_migration_enabled(); +} + +const VMStateDescription vmstate_adapter_info_so = { + .name = "s390_adapter_info/summary_offset", + .version_id = 1, + .minimum_version_id = 1, + .needed = adapter_info_so_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT32(summary_offset, AdapterInfo), + VMSTATE_END_OF_LIST() + } +}; + const VMStateDescription vmstate_adapter_info = { .name = "s390_adapter_info", .version_id = 1, @@ -152,6 +168,10 @@ const VMStateDescription vmstate_adapter_info = { */ VMSTATE_END_OF_LIST() }, + .subsections = (const VMStateDescription * []) { + &vmstate_adapter_info_so, + NULL + } }; const VMStateDescription vmstate_adapter_routes = { diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 15517a16e4..a15b58d303 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -27,12 +27,45 @@ typedef struct CrwContainer { QTAILQ_ENTRY(CrwContainer) sibling; } CrwContainer; +static const VMStateDescription vmstate_crw = { + .name = "s390_crw", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT16(flags, CRW), + VMSTATE_UINT16(rsid, CRW), + VMSTATE_END_OF_LIST() + }, +}; + +static const VMStateDescription vmstate_crw_container = { + .name = "s390_crw_container", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_STRUCT(crw, CrwContainer, 0, vmstate_crw, CRW), + VMSTATE_END_OF_LIST() + }, +}; + typedef struct ChpInfo { uint8_t in_use; uint8_t type; uint8_t is_virtual; } ChpInfo; +static const VMStateDescription vmstate_chp_info = { + .name = "s390_chp_info", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT8(in_use, ChpInfo), + VMSTATE_UINT8(type, ChpInfo), + VMSTATE_UINT8(is_virtual, ChpInfo), + VMSTATE_END_OF_LIST() + } +}; + typedef struct SubchSet { SubchDev *sch[MAX_SCHID + 1]; unsigned long schids_used[BITS_TO_LONGS(MAX_SCHID + 1)]; @@ -215,6 +248,19 @@ typedef struct CssImage { ChpInfo chpids[MAX_CHPID + 1]; } CssImage; +static const VMStateDescription vmstate_css_img = { + .name = "s390_css_img", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + /* Subchannel sets have no relevant state. */ + VMSTATE_STRUCT_ARRAY(chpids, CssImage, MAX_CHPID + 1, 0, + vmstate_chp_info, ChpInfo), + VMSTATE_END_OF_LIST() + } + +}; + typedef struct IoAdapter { uint32_t id; uint8_t type; @@ -232,10 +278,34 @@ typedef struct ChannelSubSys { uint64_t chnmon_area; CssImage *css[MAX_CSSID + 1]; uint8_t default_cssid; + /* don't migrate */ IoAdapter *io_adapters[CSS_IO_ADAPTER_TYPE_NUMS][MAX_ISC + 1]; + /* don't migrate */ QTAILQ_HEAD(, IndAddr) indicator_addresses; } ChannelSubSys; +static const VMStateDescription vmstate_css = { + .name = "s390_css", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_QTAILQ_V(pending_crws, ChannelSubSys, 1, vmstate_crw_container, + CrwContainer, sibling), + VMSTATE_BOOL(sei_pending, ChannelSubSys), + VMSTATE_BOOL(do_crw_mchk, ChannelSubSys), + VMSTATE_BOOL(crws_lost, ChannelSubSys), + /* These were kind of migrated by virtio */ + VMSTATE_UINT8(max_cssid, ChannelSubSys), + VMSTATE_UINT8(max_ssid, ChannelSubSys), + VMSTATE_BOOL(chnmon_active, ChannelSubSys), + VMSTATE_UINT64(chnmon_area, ChannelSubSys), + VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(css, ChannelSubSys, MAX_CSSID + 1, + 0, vmstate_css_img, CssImage), + VMSTATE_UINT8(default_cssid, ChannelSubSys), + VMSTATE_END_OF_LIST() + } +}; + static ChannelSubSys channel_subsys = { .pending_crws = QTAILQ_HEAD_INITIALIZER(channel_subsys.pending_crws), .do_crw_mchk = true, @@ -275,6 +345,10 @@ static int subch_dev_post_load(void *opaque, int version_id) css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s); } + if (css_migration_enabled()) { + /* No compat voodoo to do ;) */ + return 0; + } /* * Hack alert. If we don't migrate the channel subsystem status * we still need to find out if the guest enabled mss/mcss-e. -- 2.11.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally Halil Pasic @ 2017-05-31 18:52 ` Juan Quintela 2017-06-01 9:35 ` Halil Pasic 2017-06-01 11:42 ` Cornelia Huck 1 sibling, 1 reply; 24+ messages in thread From: Juan Quintela @ 2017-05-31 18:52 UTC (permalink / raw) To: Halil Pasic Cc: Cornelia Huck, Dr. David Alan Gilbert, Dong Jia Shi, qemu-devel Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Although we have recently vmstatified the migration of some css > infrastructure, for some css entities there is still state to be > migrated left, because the focus was keeping migration stream > compatibility (that is basically everything as-is). > > Let us add vmstate helpers and extend existing vmstate descriptions so > that we have everything we need. Let us guard the added state with via > css_migration_enabled, so we keep the compatible behavior if css > migration is disabled. > > Let's also annotate the bits which do not need to be migrated for better > readability. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > --- > hw/intc/s390_flic.c | 20 +++++++++++++++ > hw/s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+) Reviewed-by: Juan Quintela <quintela@redhat.com> For the vmstate bits. I have exactly zero clue about s390 > +static const VMStateDescription vmstate_chp_info = { > + .name = "s390_chp_info", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(in_use, ChpInfo), > + VMSTATE_UINT8(type, ChpInfo), > + VMSTATE_UINT8(is_virtual, ChpInfo), > + VMSTATE_END_OF_LIST() > + } > +}; > + > typedef struct SubchSet { > SubchDev *sch[MAX_SCHID + 1]; > unsigned long schids_used[BITS_TO_LONGS(MAX_SCHID + 1)]; > @@ -215,6 +248,19 @@ typedef struct CssImage { > ChpInfo chpids[MAX_CHPID + 1]; > } CssImage; > > +static const VMStateDescription vmstate_css_img = { > + .name = "s390_css_img", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + /* Subchannel sets have no relevant state. */ > + VMSTATE_STRUCT_ARRAY(chpids, CssImage, MAX_CHPID + 1, 0, > + vmstate_chp_info, ChpInfo), > + VMSTATE_END_OF_LIST() > + } > + > +}; > +static const VMStateDescription vmstate_css = { > + .name = "s390_css", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_QTAILQ_V(pending_crws, ChannelSubSys, 1, vmstate_crw_container, > + CrwContainer, sibling), > + VMSTATE_BOOL(sei_pending, ChannelSubSys), > + VMSTATE_BOOL(do_crw_mchk, ChannelSubSys), > + VMSTATE_BOOL(crws_lost, ChannelSubSys), > + /* These were kind of migrated by virtio */ > + VMSTATE_UINT8(max_cssid, ChannelSubSys), > + VMSTATE_UINT8(max_ssid, ChannelSubSys), > + VMSTATE_BOOL(chnmon_active, ChannelSubSys), > + VMSTATE_UINT64(chnmon_area, ChannelSubSys), > + VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(css, ChannelSubSys, MAX_CSSID + 1, > + 0, vmstate_css_img, CssImage), I was about to suggest to move css from pointer to an embedded struct, and then noticed that MAX_CSSID is .... 65535. I guess that this is going to be sparse, very sparse. Perhaps there is an easier way to transmit it? You are transmiting: 65000 structs each of size MAX_CHPID (255) and each element is byte + byte +byte = 65000 * 255 * 3 = 47 MB. If it is really sparse, I think that sending something like a list of <index in array> <contents> could make sense, no? Or I am missunderstanding something? Later, Juan. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally 2017-05-31 18:52 ` Juan Quintela @ 2017-06-01 9:35 ` Halil Pasic 2017-06-01 11:32 ` Cornelia Huck 0 siblings, 1 reply; 24+ messages in thread From: Halil Pasic @ 2017-06-01 9:35 UTC (permalink / raw) To: quintela, Cornelia Huck; +Cc: Dong Jia Shi, Dr. David Alan Gilbert, qemu-devel On 05/31/2017 08:52 PM, Juan Quintela wrote: > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >> Although we have recently vmstatified the migration of some css >> infrastructure, for some css entities there is still state to be >> migrated left, because the focus was keeping migration stream >> compatibility (that is basically everything as-is). >> >> Let us add vmstate helpers and extend existing vmstate descriptions so >> that we have everything we need. Let us guard the added state with via >> css_migration_enabled, so we keep the compatible behavior if css >> migration is disabled. >> >> Let's also annotate the bits which do not need to be migrated for better >> readability. >> >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> >> --- >> hw/intc/s390_flic.c | 20 +++++++++++++++ >> hw/s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 94 insertions(+) > > Reviewed-by: Juan Quintela <quintela@redhat.com> > Thanks! > > For the vmstate bits. I have exactly zero clue about s390 > >> +static const VMStateDescription vmstate_chp_info = { >> + .name = "s390_chp_info", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT8(in_use, ChpInfo), >> + VMSTATE_UINT8(type, ChpInfo), >> + VMSTATE_UINT8(is_virtual, ChpInfo), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> typedef struct SubchSet { >> SubchDev *sch[MAX_SCHID + 1]; >> unsigned long schids_used[BITS_TO_LONGS(MAX_SCHID + 1)]; >> @@ -215,6 +248,19 @@ typedef struct CssImage { >> ChpInfo chpids[MAX_CHPID + 1]; >> } CssImage; >> >> +static const VMStateDescription vmstate_css_img = { >> + .name = "s390_css_img", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + /* Subchannel sets have no relevant state. */ >> + VMSTATE_STRUCT_ARRAY(chpids, CssImage, MAX_CHPID + 1, 0, >> + vmstate_chp_info, ChpInfo), >> + VMSTATE_END_OF_LIST() >> + } >> + >> +}; > >> +static const VMStateDescription vmstate_css = { >> + .name = "s390_css", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_QTAILQ_V(pending_crws, ChannelSubSys, 1, vmstate_crw_container, >> + CrwContainer, sibling), >> + VMSTATE_BOOL(sei_pending, ChannelSubSys), >> + VMSTATE_BOOL(do_crw_mchk, ChannelSubSys), >> + VMSTATE_BOOL(crws_lost, ChannelSubSys), >> + /* These were kind of migrated by virtio */ >> + VMSTATE_UINT8(max_cssid, ChannelSubSys), >> + VMSTATE_UINT8(max_ssid, ChannelSubSys), >> + VMSTATE_BOOL(chnmon_active, ChannelSubSys), >> + VMSTATE_UINT64(chnmon_area, ChannelSubSys), >> + VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(css, ChannelSubSys, MAX_CSSID + 1, >> + 0, vmstate_css_img, CssImage), > > I was about to suggest to move css from pointer to an embedded struct, > and then noticed that MAX_CSSID is .... 65535. I guess that this is > going to be sparse, very sparse. Perhaps there is an easier way to > transmit it? > > You are transmiting: > > 65000 structs each of size MAX_CHPID (255) and each element is byte + > byte +byte = 65000 * 255 * 3 = 47 MB. > > If it is really sparse, I think that sending something like a list > of <index in array> <contents> could make sense, no? > > Or I am missunderstanding something? > I think you are misunderstanding. Sparse arrays of pointers were not supported in the first place. Support was added by me recentily (patch 07d4e69 "migration/vmstate: fix array of ptr with nullptrs"). For a null pointer just a single byte is transmitted. So we should be more like around 64KB, given that it is really sparse. Bottom line, the in stream representation is at least as efficient as the in memory representation. Of course we could do something special for sparse arrays of pointers in general, and indeed, one of the possibilities is a list/vector of non null indexes. I would like to ask the s390x maintainers (mainly Connie) about the particular case, and the migration maintainers (I spontaneously think of you Juan and Dave) about the general case (If we want to have something like VMSTATE_ARRAY_OF_POINTER_TO_STRUCT_SPARSE). Regards, Halil > Later, Juan. > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally 2017-06-01 9:35 ` Halil Pasic @ 2017-06-01 11:32 ` Cornelia Huck 2017-06-01 11:46 ` Halil Pasic 0 siblings, 1 reply; 24+ messages in thread From: Cornelia Huck @ 2017-06-01 11:32 UTC (permalink / raw) To: Halil Pasic; +Cc: quintela, Dong Jia Shi, Dr. David Alan Gilbert, qemu-devel On Thu, 1 Jun 2017 11:35:27 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > On 05/31/2017 08:52 PM, Juan Quintela wrote: > > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > >> Although we have recently vmstatified the migration of some css > >> infrastructure, for some css entities there is still state to be > >> migrated left, because the focus was keeping migration stream > >> compatibility (that is basically everything as-is). > >> > >> Let us add vmstate helpers and extend existing vmstate descriptions so > >> that we have everything we need. Let us guard the added state with via > >> css_migration_enabled, so we keep the compatible behavior if css > >> migration is disabled. > >> > >> Let's also annotate the bits which do not need to be migrated for better > >> readability. > >> > >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > >> --- > >> hw/intc/s390_flic.c | 20 +++++++++++++++ > >> hw/s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 94 insertions(+) > > > > Reviewed-by: Juan Quintela <quintela@redhat.com> > > > > Thanks! > > > > > For the vmstate bits. I have exactly zero clue about s390 > > > >> +static const VMStateDescription vmstate_chp_info = { > >> + .name = "s390_chp_info", > >> + .version_id = 1, > >> + .minimum_version_id = 1, > >> + .fields = (VMStateField[]) { > >> + VMSTATE_UINT8(in_use, ChpInfo), > >> + VMSTATE_UINT8(type, ChpInfo), > >> + VMSTATE_UINT8(is_virtual, ChpInfo), > >> + VMSTATE_END_OF_LIST() > >> + } > >> +}; > >> + > >> typedef struct SubchSet { > >> SubchDev *sch[MAX_SCHID + 1]; > >> unsigned long schids_used[BITS_TO_LONGS(MAX_SCHID + 1)]; > >> @@ -215,6 +248,19 @@ typedef struct CssImage { > >> ChpInfo chpids[MAX_CHPID + 1]; > >> } CssImage; > >> > >> +static const VMStateDescription vmstate_css_img = { > >> + .name = "s390_css_img", > >> + .version_id = 1, > >> + .minimum_version_id = 1, > >> + .fields = (VMStateField[]) { > >> + /* Subchannel sets have no relevant state. */ > >> + VMSTATE_STRUCT_ARRAY(chpids, CssImage, MAX_CHPID + 1, 0, > >> + vmstate_chp_info, ChpInfo), > >> + VMSTATE_END_OF_LIST() > >> + } > >> + > >> +}; > > > >> +static const VMStateDescription vmstate_css = { > >> + .name = "s390_css", > >> + .version_id = 1, > >> + .minimum_version_id = 1, > >> + .fields = (VMStateField[]) { > >> + VMSTATE_QTAILQ_V(pending_crws, ChannelSubSys, 1, vmstate_crw_container, > >> + CrwContainer, sibling), > >> + VMSTATE_BOOL(sei_pending, ChannelSubSys), > >> + VMSTATE_BOOL(do_crw_mchk, ChannelSubSys), > >> + VMSTATE_BOOL(crws_lost, ChannelSubSys), > >> + /* These were kind of migrated by virtio */ > >> + VMSTATE_UINT8(max_cssid, ChannelSubSys), > >> + VMSTATE_UINT8(max_ssid, ChannelSubSys), > >> + VMSTATE_BOOL(chnmon_active, ChannelSubSys), > >> + VMSTATE_UINT64(chnmon_area, ChannelSubSys), > >> + VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(css, ChannelSubSys, MAX_CSSID + 1, > >> + 0, vmstate_css_img, CssImage), > > > > I was about to suggest to move css from pointer to an embedded struct, > > and then noticed that MAX_CSSID is .... 65535. I guess that this is > > going to be sparse, very sparse. Perhaps there is an easier way to > > transmit it? > > > > You are transmiting: > > > > 65000 structs each of size MAX_CHPID (255) and each element is byte + > > byte +byte = 65000 * 255 * 3 = 47 MB. > > > > If it is really sparse, I think that sending something like a list > > of <index in array> <contents> could make sense, no? > > > > Or I am missunderstanding something? > > > > I think you are misunderstanding. Sparse arrays of pointers were not > supported in the first place. Support was added by me recentily (patch > 07d4e69 "migration/vmstate: fix array of ptr with nullptrs"). For a null > pointer just a single byte is transmitted. So we should be more like > around 64KB, given that it is really sparse. Bottom line, the in stream > representation is at least as efficient as the in memory representation. > > Of course we could do something special for sparse arrays of pointers in > general, and indeed, one of the possibilities is a list/vector of non > null indexes. Just as discussion fodder: For a run-of-the-mill qemu guest, I'd expect a low double-digit number of subchannels in css 0xfe, in subchannel set 0, which are clustered right at the beginning, and one chpid (0) in css 0xfe. As the subchannel id is autogenerated based upon the devno (if provided), it is easy to have subchannels in an arbitrary subchannel set (of which there are only four), but to get high subchannel ids with a lot of empty slots before, you'd have to create a lot (really a lot) of devices and then detach most of them again - not a very likely pattern. The 3270 support does not really change much: one additional subchannel (with the same characteristics) and one further chpid. But I don't expect many people to use 3270, and it is not (yet) migratable anyway. vfio-ccw devices will be in another css (possibly mapped), but they are not migrateable either. So if we want to optimize, some "nothing of interest beyond that point" marker might be useful - but not very. > > I would like to ask the s390x maintainers (mainly Connie) about the > particular case, and the migration maintainers (I spontaneously think of > you Juan and Dave) about the general case (If we want to have something > like VMSTATE_ARRAY_OF_POINTER_TO_STRUCT_SPARSE). > > Regards, > Halil > > > Later, Juan. > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally 2017-06-01 11:32 ` Cornelia Huck @ 2017-06-01 11:46 ` Halil Pasic 2017-06-07 18:03 ` Juan Quintela 0 siblings, 1 reply; 24+ messages in thread From: Halil Pasic @ 2017-06-01 11:46 UTC (permalink / raw) To: Cornelia Huck; +Cc: qemu-devel, Dong Jia Shi, Dr. David Alan Gilbert, quintela On 06/01/2017 01:32 PM, Cornelia Huck wrote: > On Thu, 1 Jun 2017 11:35:27 +0200 > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > >> On 05/31/2017 08:52 PM, Juan Quintela wrote: >>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >>>> Although we have recently vmstatified the migration of some css >>>> infrastructure, for some css entities there is still state to be >>>> migrated left, because the focus was keeping migration stream >>>> compatibility (that is basically everything as-is). >>>> >>>> Let us add vmstate helpers and extend existing vmstate descriptions so >>>> that we have everything we need. Let us guard the added state with via >>>> css_migration_enabled, so we keep the compatible behavior if css >>>> migration is disabled. >>>> >>>> Let's also annotate the bits which do not need to be migrated for better >>>> readability. >>>> >>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> >>>> --- >>>> hw/intc/s390_flic.c | 20 +++++++++++++++ >>>> hw/s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 94 insertions(+) >>> >>> Reviewed-by: Juan Quintela <quintela@redhat.com> >>> >> >> Thanks! >> >>> >>> For the vmstate bits. I have exactly zero clue about s390 >>> >>>> +static const VMStateDescription vmstate_chp_info = { >>>> + .name = "s390_chp_info", >>>> + .version_id = 1, >>>> + .minimum_version_id = 1, >>>> + .fields = (VMStateField[]) { >>>> + VMSTATE_UINT8(in_use, ChpInfo), >>>> + VMSTATE_UINT8(type, ChpInfo), >>>> + VMSTATE_UINT8(is_virtual, ChpInfo), >>>> + VMSTATE_END_OF_LIST() >>>> + } >>>> +}; >>>> + >>>> typedef struct SubchSet { >>>> SubchDev *sch[MAX_SCHID + 1]; >>>> unsigned long schids_used[BITS_TO_LONGS(MAX_SCHID + 1)]; >>>> @@ -215,6 +248,19 @@ typedef struct CssImage { >>>> ChpInfo chpids[MAX_CHPID + 1]; >>>> } CssImage; >>>> >>>> +static const VMStateDescription vmstate_css_img = { >>>> + .name = "s390_css_img", >>>> + .version_id = 1, >>>> + .minimum_version_id = 1, >>>> + .fields = (VMStateField[]) { >>>> + /* Subchannel sets have no relevant state. */ >>>> + VMSTATE_STRUCT_ARRAY(chpids, CssImage, MAX_CHPID + 1, 0, >>>> + vmstate_chp_info, ChpInfo), >>>> + VMSTATE_END_OF_LIST() >>>> + } >>>> + >>>> +}; >>> >>>> +static const VMStateDescription vmstate_css = { >>>> + .name = "s390_css", >>>> + .version_id = 1, >>>> + .minimum_version_id = 1, >>>> + .fields = (VMStateField[]) { >>>> + VMSTATE_QTAILQ_V(pending_crws, ChannelSubSys, 1, vmstate_crw_container, >>>> + CrwContainer, sibling), >>>> + VMSTATE_BOOL(sei_pending, ChannelSubSys), >>>> + VMSTATE_BOOL(do_crw_mchk, ChannelSubSys), >>>> + VMSTATE_BOOL(crws_lost, ChannelSubSys), >>>> + /* These were kind of migrated by virtio */ >>>> + VMSTATE_UINT8(max_cssid, ChannelSubSys), >>>> + VMSTATE_UINT8(max_ssid, ChannelSubSys), >>>> + VMSTATE_BOOL(chnmon_active, ChannelSubSys), >>>> + VMSTATE_UINT64(chnmon_area, ChannelSubSys), >>>> + VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(css, ChannelSubSys, MAX_CSSID + 1, >>>> + 0, vmstate_css_img, CssImage), >>> >>> I was about to suggest to move css from pointer to an embedded struct, >>> and then noticed that MAX_CSSID is .... 65535. I guess that this is #define MAX_CSSID 255 ("include/hw/s390x/css.h" line 23) >>> going to be sparse, very sparse. Perhaps there is an easier way to >>> transmit it? >>> >>> You are transmiting: >>> >>> 65000 structs each of size MAX_CHPID (255) and each element is byte + >>> byte +byte = 65000 * 255 * 3 = 47 MB. >>> >>> If it is really sparse, I think that sending something like a list >>> of <index in array> <contents> could make sense, no? >>> >>> Or I am missunderstanding something? >>> >> >> I think you are misunderstanding. Sparse arrays of pointers were not >> supported in the first place. Support was added by me recentily (patch >> 07d4e69 "migration/vmstate: fix array of ptr with nullptrs"). For a null >> pointer just a single byte is transmitted. So we should be more like >> around 64KB, given that it is really sparse. Bottom line, the in stream Given that my calculation is also wrong. The overhead is below 255 bytes (overhead compared to not transferring anything for null pointers, which is clearly an unrealistic lower bound case). >> representation is at least as efficient as the in memory representation. >> >> Of course we could do something special for sparse arrays of pointers in >> general, and indeed, one of the possibilities is a list/vector of non >> null indexes. > > Just as discussion fodder: For a run-of-the-mill qemu guest, I'd expect > a low double-digit number of subchannels in css 0xfe, in subchannel set > 0, which are clustered right at the beginning, and one chpid (0) in css > 0xfe. > > As the subchannel id is autogenerated based upon the devno (if > provided), it is easy to have subchannels in an arbitrary subchannel > set (of which there are only four), but to get high subchannel ids with > a lot of empty slots before, you'd have to create a lot (really a lot) > of devices and then detach most of them again - not a very likely > pattern. > > The 3270 support does not really change much: one additional subchannel > (with the same characteristics) and one further chpid. But I don't > expect many people to use 3270, and it is not (yet) migratable anyway. > > vfio-ccw devices will be in another css (possibly mapped), but they are > not migrateable either. > > So if we want to optimize, some "nothing of interest beyond that point" > marker might be useful - but not very. Thus I do not think we need extra sparse array handling for this. (Nevertheless an extra sparse array handling might make sense for vmstate core, provided there is an use case). Halil > >> >> I would like to ask the s390x maintainers (mainly Connie) about the >> particular case, and the migration maintainers (I spontaneously think of >> you Juan and Dave) about the general case (If we want to have something >> like VMSTATE_ARRAY_OF_POINTER_TO_STRUCT_SPARSE). >> >> Regards, >> Halil >> >>> Later, Juan. >>> > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally 2017-06-01 11:46 ` Halil Pasic @ 2017-06-07 18:03 ` Juan Quintela 2017-06-08 8:52 ` Halil Pasic 0 siblings, 1 reply; 24+ messages in thread From: Juan Quintela @ 2017-06-07 18:03 UTC (permalink / raw) To: Halil Pasic Cc: Cornelia Huck, Dong Jia Shi, qemu-devel, Dr. David Alan Gilbert Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > On 06/01/2017 01:32 PM, Cornelia Huck wrote: >> On Thu, 1 Jun 2017 11:35:27 +0200 >> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >> >>>> >>>> I was about to suggest to move css from pointer to an embedded struct, >>>> and then noticed that MAX_CSSID is .... 65535. I guess that this is > > #define MAX_CSSID 255 > ("include/hw/s390x/css.h" line 23) I have to grep better O:-) >>>> going to be sparse, very sparse. Perhaps there is an easier way to >>>> transmit it? >>>> >>>> You are transmiting: >>>> >>>> 65000 structs each of size MAX_CHPID (255) and each element is byte + >>>> byte +byte = 65000 * 255 * 3 = 47 MB. >>>> >>>> If it is really sparse, I think that sending something like a list >>>> of <index in array> <contents> could make sense, no? >>>> >>>> Or I am missunderstanding something? >>>> >>> >>> I think you are misunderstanding. Sparse arrays of pointers were not >>> supported in the first place. Support was added by me recentily (patch >>> 07d4e69 "migration/vmstate: fix array of ptr with nullptrs"). For a null >>> pointer just a single byte is transmitted. So we should be more like >>> around 64KB, given that it is really sparse. Bottom line, the in stream > > Given that my calculation is also wrong. The overhead is below 255 bytes > (overhead compared to not transferring anything for null pointers, which > is clearly an unrealistic lower bound case). Yeap, I greeped wrong. 255 x 255 makes no sense to optimize at all. Sorry for the noise. >>> representation is at least as efficient as the in memory representation. >>> >>> Of course we could do something special for sparse arrays of pointers in >>> general, and indeed, one of the possibilities is a list/vector of non >>> null indexes. >> >> Just as discussion fodder: For a run-of-the-mill qemu guest, I'd expect >> a low double-digit number of subchannels in css 0xfe, in subchannel set >> 0, which are clustered right at the beginning, and one chpid (0) in css >> 0xfe. >> >> As the subchannel id is autogenerated based upon the devno (if >> provided), it is easy to have subchannels in an arbitrary subchannel >> set (of which there are only four), but to get high subchannel ids with >> a lot of empty slots before, you'd have to create a lot (really a lot) >> of devices and then detach most of them again - not a very likely >> pattern. >> >> The 3270 support does not really change much: one additional subchannel >> (with the same characteristics) and one further chpid. But I don't >> expect many people to use 3270, and it is not (yet) migratable anyway. >> >> vfio-ccw devices will be in another css (possibly mapped), but they are >> not migrateable either. >> >> So if we want to optimize, some "nothing of interest beyond that point" >> marker might be useful - but not very. > > Thus I do not think we need extra sparse array handling for this. > > (Nevertheless an extra sparse array handling might make sense for > vmstate core, provided there is an use case). So, I retire all my objections. Again, sorry for the noise. Thanks, Juan. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally 2017-06-07 18:03 ` Juan Quintela @ 2017-06-08 8:52 ` Halil Pasic 0 siblings, 0 replies; 24+ messages in thread From: Halil Pasic @ 2017-06-08 8:52 UTC (permalink / raw) To: quintela; +Cc: Cornelia Huck, Dong Jia Shi, qemu-devel, Dr. David Alan Gilbert On 06/07/2017 08:03 PM, Juan Quintela wrote: > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >> On 06/01/2017 01:32 PM, Cornelia Huck wrote: >>> On Thu, 1 Jun 2017 11:35:27 +0200 >>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >>> > >>>>> >>>>> I was about to suggest to move css from pointer to an embedded struct, >>>>> and then noticed that MAX_CSSID is .... 65535. I guess that this is >> >> #define MAX_CSSID 255 >> ("include/hw/s390x/css.h" line 23) > > I have to grep better O:-) > >>>>> going to be sparse, very sparse. Perhaps there is an easier way to >>>>> transmit it? >>>>> >>>>> You are transmiting: >>>>> >>>>> 65000 structs each of size MAX_CHPID (255) and each element is byte + >>>>> byte +byte = 65000 * 255 * 3 = 47 MB. >>>>> >>>>> If it is really sparse, I think that sending something like a list >>>>> of <index in array> <contents> could make sense, no? >>>>> >>>>> Or I am missunderstanding something? >>>>> >>>> >>>> I think you are misunderstanding. Sparse arrays of pointers were not >>>> supported in the first place. Support was added by me recentily (patch >>>> 07d4e69 "migration/vmstate: fix array of ptr with nullptrs"). For a null >>>> pointer just a single byte is transmitted. So we should be more like >>>> around 64KB, given that it is really sparse. Bottom line, the in stream >> >> Given that my calculation is also wrong. The overhead is below 255 bytes >> (overhead compared to not transferring anything for null pointers, which >> is clearly an unrealistic lower bound case). > > Yeap, I greeped wrong. 255 x 255 makes no sense to optimize at all. > Sorry for the noise. > >>>> representation is at least as efficient as the in memory representation. >>>> >>>> Of course we could do something special for sparse arrays of pointers in >>>> general, and indeed, one of the possibilities is a list/vector of non >>>> null indexes. >>> >>> Just as discussion fodder: For a run-of-the-mill qemu guest, I'd expect >>> a low double-digit number of subchannels in css 0xfe, in subchannel set >>> 0, which are clustered right at the beginning, and one chpid (0) in css >>> 0xfe. >>> >>> As the subchannel id is autogenerated based upon the devno (if >>> provided), it is easy to have subchannels in an arbitrary subchannel >>> set (of which there are only four), but to get high subchannel ids with >>> a lot of empty slots before, you'd have to create a lot (really a lot) >>> of devices and then detach most of them again - not a very likely >>> pattern. >>> >>> The 3270 support does not really change much: one additional subchannel >>> (with the same characteristics) and one further chpid. But I don't >>> expect many people to use 3270, and it is not (yet) migratable anyway. >>> >>> vfio-ccw devices will be in another css (possibly mapped), but they are >>> not migrateable either. >>> >>> So if we want to optimize, some "nothing of interest beyond that point" >>> marker might be useful - but not very. >> >> Thus I do not think we need extra sparse array handling for this. >> >> (Nevertheless an extra sparse array handling might make sense for >> vmstate core, provided there is an use case). > > So, I retire all my objections. > > Again, sorry for the noise. No problem. Actually, it's appreciated! Better have some false positives than having something stupid slipping in and having to live with it. Thanks for your review! Regards, Halil ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally Halil Pasic 2017-05-31 18:52 ` Juan Quintela @ 2017-06-01 11:42 ` Cornelia Huck 1 sibling, 0 replies; 24+ messages in thread From: Cornelia Huck @ 2017-06-01 11:42 UTC (permalink / raw) To: Halil Pasic Cc: Dr. David Alan Gilbert, Dong Jia Shi, Juan Quintela, qemu-devel On Mon, 29 May 2017 15:55:17 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Although we have recently vmstatified the migration of some css > infrastructure, for some css entities there is still state to be > migrated left, because the focus was keeping migration stream > compatibility (that is basically everything as-is). > > Let us add vmstate helpers and extend existing vmstate descriptions so > that we have everything we need. Let us guard the added state with via Either 'with' or 'via' ;) > css_migration_enabled, so we keep the compatible behavior if css > migration is disabled. > > Let's also annotate the bits which do not need to be migrated for better > readability. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > --- > hw/intc/s390_flic.c | 20 +++++++++++++++ > hw/s390x/css.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+) > > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c > index 7e7546a576..b889e74571 100644 > --- a/hw/intc/s390_flic.c > +++ b/hw/intc/s390_flic.c > @@ -139,6 +139,22 @@ static void qemu_s390_flic_register_types(void) > > type_init(qemu_s390_flic_register_types) > > +static bool adapter_info_so_needed(void *opaque) > +{ > + return css_migration_enabled(); > +} > + > +const VMStateDescription vmstate_adapter_info_so = { > + .name = "s390_adapter_info/summary_offset", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = adapter_info_so_needed, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(summary_offset, AdapterInfo), > + VMSTATE_END_OF_LIST() > + } > +}; > + > const VMStateDescription vmstate_adapter_info = { > .name = "s390_adapter_info", > .version_id = 1, > @@ -152,6 +168,10 @@ const VMStateDescription vmstate_adapter_info = { > */ > VMSTATE_END_OF_LIST() > }, > + .subsections = (const VMStateDescription * []) { > + &vmstate_adapter_info_so, > + NULL > + } > }; > > const VMStateDescription vmstate_adapter_routes = { > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 15517a16e4..a15b58d303 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -27,12 +27,45 @@ typedef struct CrwContainer { > QTAILQ_ENTRY(CrwContainer) sibling; > } CrwContainer; > > +static const VMStateDescription vmstate_crw = { > + .name = "s390_crw", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT16(flags, CRW), > + VMSTATE_UINT16(rsid, CRW), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > +static const VMStateDescription vmstate_crw_container = { > + .name = "s390_crw_container", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT(crw, CrwContainer, 0, vmstate_crw, CRW), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > typedef struct ChpInfo { > uint8_t in_use; > uint8_t type; > uint8_t is_virtual; > } ChpInfo; > > +static const VMStateDescription vmstate_chp_info = { > + .name = "s390_chp_info", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(in_use, ChpInfo), > + VMSTATE_UINT8(type, ChpInfo), > + VMSTATE_UINT8(is_virtual, ChpInfo), > + VMSTATE_END_OF_LIST() > + } > +}; > + > typedef struct SubchSet { > SubchDev *sch[MAX_SCHID + 1]; > unsigned long schids_used[BITS_TO_LONGS(MAX_SCHID + 1)]; > @@ -215,6 +248,19 @@ typedef struct CssImage { > ChpInfo chpids[MAX_CHPID + 1]; > } CssImage; > > +static const VMStateDescription vmstate_css_img = { > + .name = "s390_css_img", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + /* Subchannel sets have no relevant state. */ > + VMSTATE_STRUCT_ARRAY(chpids, CssImage, MAX_CHPID + 1, 0, > + vmstate_chp_info, ChpInfo), > + VMSTATE_END_OF_LIST() > + } > + > +}; > + > typedef struct IoAdapter { > uint32_t id; > uint8_t type; > @@ -232,10 +278,34 @@ typedef struct ChannelSubSys { > uint64_t chnmon_area; > CssImage *css[MAX_CSSID + 1]; > uint8_t default_cssid; > + /* don't migrate */ > IoAdapter *io_adapters[CSS_IO_ADAPTER_TYPE_NUMS][MAX_ISC + 1]; > + /* don't migrate */ > QTAILQ_HEAD(, IndAddr) indicator_addresses; > } ChannelSubSys; I think a couple of words as to _why_ they are not migrated would be nice. Imagine revisiting this in a few years :) > > +static const VMStateDescription vmstate_css = { > + .name = "s390_css", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_QTAILQ_V(pending_crws, ChannelSubSys, 1, vmstate_crw_container, > + CrwContainer, sibling), > + VMSTATE_BOOL(sei_pending, ChannelSubSys), > + VMSTATE_BOOL(do_crw_mchk, ChannelSubSys), > + VMSTATE_BOOL(crws_lost, ChannelSubSys), > + /* These were kind of migrated by virtio */ > + VMSTATE_UINT8(max_cssid, ChannelSubSys), > + VMSTATE_UINT8(max_ssid, ChannelSubSys), > + VMSTATE_BOOL(chnmon_active, ChannelSubSys), > + VMSTATE_UINT64(chnmon_area, ChannelSubSys), > + VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(css, ChannelSubSys, MAX_CSSID + 1, > + 0, vmstate_css_img, CssImage), > + VMSTATE_UINT8(default_cssid, ChannelSubSys), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static ChannelSubSys channel_subsys = { > .pending_crws = QTAILQ_HEAD_INITIALIZER(channel_subsys.pending_crws), > .do_crw_mchk = true, > @@ -275,6 +345,10 @@ static int subch_dev_post_load(void *opaque, int version_id) > css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s); > } > > + if (css_migration_enabled()) { > + /* No compat voodoo to do ;) */ > + return 0; > + } > /* > * Hack alert. If we don't migrate the channel subsystem status > * we still need to find out if the guest enabled mss/mcss-e. Other than the nits, Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> ^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 5/7] s390x/css: add ORB to SubchDev 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic ` (3 preceding siblings ...) 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally Halil Pasic @ 2017-05-29 13:55 ` Halil Pasic 2017-06-01 11:45 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration Halil Pasic ` (3 subsequent siblings) 8 siblings, 1 reply; 24+ messages in thread From: Halil Pasic @ 2017-05-29 13:55 UTC (permalink / raw) To: Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, Juan Quintela, qemu-devel, Halil Pasic Since we are going to need a migration compatibility breaking change to activate ChannelSubSys migration let us use the opportunity to introduce ORB to the SubchDev before that (otherwise we would need separate handling e.g. a compat property). The ORB will be useful for implementing IDA, or async handling of subchannel work. Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> Reviewed-by: Guenther Hutzl <hutzl@linux.vnet.ibm.com> --- hw/s390x/css.c | 35 +++++++++++++++++++++++++++++++++++ include/hw/s390x/css.h | 1 + 2 files changed, 36 insertions(+) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index a15b58d303..e840dfba2a 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -163,6 +163,36 @@ static const VMStateDescription vmstate_sense_id = { } }; +static const VMStateDescription vmstate_orb = { + .name = "s390_orb", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(intparm, ORB), + VMSTATE_UINT16(ctrl0, ORB), + VMSTATE_UINT8(lpm, ORB), + VMSTATE_UINT8(ctrl1, ORB), + VMSTATE_UINT32(cpa, ORB), + VMSTATE_END_OF_LIST() + } +}; + +static bool vmstate_schdev_orb_needed(void *opaque) +{ + return css_migration_enabled(); +} + +static const VMStateDescription vmstate_schdev_orb = { + .name = "s390_subch_dev/orb", + .version_id = 1, + .minimum_version_id = 1, + .needed = vmstate_schdev_orb_needed, + .fields = (VMStateField[]) { + VMSTATE_STRUCT(orb, SubchDev, 1, vmstate_orb, ORB), + VMSTATE_END_OF_LIST() + } +}; + static int subch_dev_post_load(void *opaque, int version_id); static void subch_dev_pre_save(void *opaque); @@ -187,6 +217,10 @@ const VMStateDescription vmstate_subch_dev = { VMSTATE_BOOL(ccw_fmt_1, SubchDev), VMSTATE_UINT8(ccw_no_data_cnt, SubchDev), VMSTATE_END_OF_LIST() + }, + .subsections = (const VMStateDescription * []) { + &vmstate_schdev_orb, + NULL } }; @@ -1253,6 +1287,7 @@ int css_do_ssch(SubchDev *sch, ORB *orb) if (channel_subsys.chnmon_active) { css_update_chnmon(sch); } + sch->orb = *orb; sch->channel_prog = orb->cpa; /* Trigger the start function. */ s->ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND); diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h index d74e44d312..e9c02af550 100644 --- a/include/hw/s390x/css.h +++ b/include/hw/s390x/css.h @@ -89,6 +89,7 @@ struct SubchDev { bool thinint_active; uint8_t ccw_no_data_cnt; uint16_t migrated_schid; /* used for missmatch detection */ + ORB orb; /* transport-provided data: */ int (*ccw_cb) (SubchDev *, CCW1); void (*disable_cb)(SubchDev *); -- 2.11.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/7] s390x/css: add ORB to SubchDev 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 5/7] s390x/css: add ORB to SubchDev Halil Pasic @ 2017-06-01 11:45 ` Cornelia Huck 0 siblings, 0 replies; 24+ messages in thread From: Cornelia Huck @ 2017-06-01 11:45 UTC (permalink / raw) To: Halil Pasic Cc: Dr. David Alan Gilbert, Dong Jia Shi, Juan Quintela, qemu-devel On Mon, 29 May 2017 15:55:18 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Since we are going to need a migration compatibility breaking change to > activate ChannelSubSys migration let us use the opportunity to introduce > ORB to the SubchDev before that (otherwise we would need separate > handling e.g. a compat property). > > The ORB will be useful for implementing IDA, or async handling of > subchannel work. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > Reviewed-by: Guenther Hutzl <hutzl@linux.vnet.ibm.com> > --- > hw/s390x/css.c | 35 +++++++++++++++++++++++++++++++++++ > include/hw/s390x/css.h | 1 + > 2 files changed, 36 insertions(+) Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> ^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic ` (4 preceding siblings ...) 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 5/7] s390x/css: add ORB to SubchDev Halil Pasic @ 2017-05-29 13:55 ` Halil Pasic 2017-06-01 8:40 ` Thomas Huth 2017-06-01 11:47 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 7/7] s390x/css: use SubchDev.orb Halil Pasic ` (2 subsequent siblings) 8 siblings, 2 replies; 24+ messages in thread From: Halil Pasic @ 2017-05-29 13:55 UTC (permalink / raw) To: Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, Juan Quintela, qemu-devel, Halil Pasic Turn on migration for the channel subsystem for the next machine. For legacy machines we still have to do things the old way. Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> --- hw/s390x/css.c | 5 +++++ hw/s390x/s390-virtio-ccw.c | 9 ++++----- hw/s390x/virtio-ccw.c | 1 - include/hw/s390x/css.h | 4 ++++ 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index e840dfba2a..9857eb3d6f 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -401,6 +401,11 @@ static int subch_dev_post_load(void *opaque, int version_id) return 0; } +void css_register_vmstate(void) +{ + vmstate_register(NULL, 0, &vmstate_css, &channel_subsys); +} + IndAddr *get_indicator(hwaddr ind_addr, int len) { IndAddr *indicator; diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 95256d3982..3ad6323add 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -196,7 +196,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) s390mc->ri_allowed = true; s390mc->cpu_model_allowed = true; - s390mc->css_migration_enabled = false; /* TODO: set to true */ + s390mc->css_migration_enabled = true; mc->init = ccw_init; mc->reset = s390_machine_reset; mc->hot_add_cpu = s390_hot_add_cpu; @@ -451,10 +451,9 @@ bool css_migration_enabled(void) static void ccw_machine_2_10_instance_options(MachineState *machine) { - /* - * TODO Once preparations are done register vmstate for the css if - * css_migration_enabled(). - */ + if (css_migration_enabled()) { + css_register_vmstate(); + } } static void ccw_machine_2_10_class_options(MachineClass *mc) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index a75eedeffb..4a0271b0be 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -1360,7 +1360,6 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) sch->id.cu_model = virtio_bus_get_vdev_id(&dev->bus); - css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, d->hotplugged, 1); } diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h index e9c02af550..812fc05804 100644 --- a/include/hw/s390x/css.h +++ b/include/hw/s390x/css.h @@ -209,4 +209,8 @@ extern PropertyInfo css_devid_ro_propinfo; * is responsible for unregistering and freeing it. */ SubchDev *css_create_virtual_sch(CssDevId bus_id, Error **errp); + +/** Turn on css migration */ +void css_register_vmstate(void); + #endif -- 2.11.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration Halil Pasic @ 2017-06-01 8:40 ` Thomas Huth 2017-06-01 10:12 ` Halil Pasic 2017-06-01 11:47 ` Cornelia Huck 1 sibling, 1 reply; 24+ messages in thread From: Thomas Huth @ 2017-06-01 8:40 UTC (permalink / raw) To: Halil Pasic, Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, qemu-devel, Juan Quintela On 29.05.2017 15:55, Halil Pasic wrote: > Turn on migration for the channel subsystem for the next machine. For > legacy machines we still have to do things the old way. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > --- > hw/s390x/css.c | 5 +++++ > hw/s390x/s390-virtio-ccw.c | 9 ++++----- > hw/s390x/virtio-ccw.c | 1 - > include/hw/s390x/css.h | 4 ++++ > 4 files changed, 13 insertions(+), 6 deletions(-) [...] > static void ccw_machine_2_10_class_options(MachineClass *mc) > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index a75eedeffb..4a0271b0be 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -1360,7 +1360,6 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) > > sch->id.cu_model = virtio_bus_get_vdev_id(&dev->bus); > > - > css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, > d->hotplugged, 1); > } Unrelated white-space change? Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration 2017-06-01 8:40 ` Thomas Huth @ 2017-06-01 10:12 ` Halil Pasic 0 siblings, 0 replies; 24+ messages in thread From: Halil Pasic @ 2017-06-01 10:12 UTC (permalink / raw) To: Thomas Huth, Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, qemu-devel, Juan Quintela On 06/01/2017 10:40 AM, Thomas Huth wrote: > On 29.05.2017 15:55, Halil Pasic wrote: >> Turn on migration for the channel subsystem for the next machine. For >> legacy machines we still have to do things the old way. >> >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> >> --- >> hw/s390x/css.c | 5 +++++ >> hw/s390x/s390-virtio-ccw.c | 9 ++++----- >> hw/s390x/virtio-ccw.c | 1 - >> include/hw/s390x/css.h | 4 ++++ >> 4 files changed, 13 insertions(+), 6 deletions(-) > [...] >> static void ccw_machine_2_10_class_options(MachineClass *mc) >> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c >> index a75eedeffb..4a0271b0be 100644 >> --- a/hw/s390x/virtio-ccw.c >> +++ b/hw/s390x/virtio-ccw.c >> @@ -1360,7 +1360,6 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) >> >> sch->id.cu_model = virtio_bus_get_vdev_id(&dev->bus); >> >> - >> css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, >> d->hotplugged, 1); >> } > > Unrelated white-space change? > > Thomas > Indeed :-( Will remove for the next re-spin. Thanks a lot! Halil ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration Halil Pasic 2017-06-01 8:40 ` Thomas Huth @ 2017-06-01 11:47 ` Cornelia Huck 1 sibling, 0 replies; 24+ messages in thread From: Cornelia Huck @ 2017-06-01 11:47 UTC (permalink / raw) To: Halil Pasic Cc: Dr. David Alan Gilbert, Dong Jia Shi, Juan Quintela, qemu-devel On Mon, 29 May 2017 15:55:19 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Turn on migration for the channel subsystem for the next machine. For > legacy machines we still have to do things the old way. > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > --- > hw/s390x/css.c | 5 +++++ > hw/s390x/s390-virtio-ccw.c | 9 ++++----- > hw/s390x/virtio-ccw.c | 1 - > include/hw/s390x/css.h | 4 ++++ > 4 files changed, 13 insertions(+), 6 deletions(-) From your reply to Christian's mail, I presume you also tested the result with some compat machines? With the whitespace change removed, Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> ^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 7/7] s390x/css: use SubchDev.orb 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic ` (5 preceding siblings ...) 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration Halil Pasic @ 2017-05-29 13:55 ` Halil Pasic 2017-06-01 11:50 ` Cornelia Huck 2017-05-31 16:46 ` [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Dr. David Alan Gilbert 2017-06-01 11:51 ` Cornelia Huck 8 siblings, 1 reply; 24+ messages in thread From: Halil Pasic @ 2017-05-29 13:55 UTC (permalink / raw) To: Cornelia Huck, Dr. David Alan Gilbert Cc: Dong Jia Shi, Juan Quintela, qemu-devel, Halil Pasic Instead of passing around a pointer to ORB let us simplify some functions signatures by using the previously introduced ORB saved at the subchannel (SubchDev). Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> Reviewed-by: Guenther Hutzl <hutzl@linux.vnet.ibm.com> --- hw/s390x/css.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 9857eb3d6f..0229cb9412 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -854,7 +854,7 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr, return ret; } -static void sch_handle_start_func(SubchDev *sch, ORB *orb) +static void sch_handle_start_func(SubchDev *sch) { PMCW *p = &sch->curr_status.pmcw; @@ -868,10 +868,10 @@ static void sch_handle_start_func(SubchDev *sch, ORB *orb) if (!(s->ctrl & SCSW_ACTL_SUSP)) { /* Start Function triggered via ssch, i.e. we have an ORB */ + ORB *orb = &sch->orb; s->cstat = 0; s->dstat = 0; /* Look at the orb and try to execute the channel program. */ - assert(orb != NULL); /* resume does not pass an orb */ p->intparm = orb->intparm; if (!(orb->lpm & path)) { /* Generate a deferred cc 3 condition. */ @@ -885,8 +885,7 @@ static void sch_handle_start_func(SubchDev *sch, ORB *orb) sch->ccw_no_data_cnt = 0; suspend_allowed = !!(orb->ctrl0 & ORB_CTRL0_MASK_SPND); } else { - /* Start Function resumed via rsch, i.e. we don't have an - * ORB */ + /* Start Function resumed via rsch */ s->ctrl &= ~(SCSW_ACTL_SUSP | SCSW_ACTL_RESUME_PEND); /* The channel program had been suspended before. */ suspend_allowed = true; @@ -962,7 +961,7 @@ static void sch_handle_start_func(SubchDev *sch, ORB *orb) * read/writes) asynchronous later on if we start supporting more than * our current very simple devices. */ -static void do_subchannel_work(SubchDev *sch, ORB *orb) +static void do_subchannel_work(SubchDev *sch) { SCSW *s = &sch->curr_status.scsw; @@ -973,7 +972,7 @@ static void do_subchannel_work(SubchDev *sch, ORB *orb) sch_handle_halt_func(sch); } else if (s->ctrl & SCSW_FCTL_START_FUNC) { /* Triggered by both ssch and rsch. */ - sch_handle_start_func(sch, orb); + sch_handle_start_func(sch); } else { /* Cannot happen. */ return; @@ -1182,7 +1181,7 @@ int css_do_csch(SubchDev *sch) s->ctrl &= ~(SCSW_CTRL_MASK_FCTL | SCSW_CTRL_MASK_ACTL); s->ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_ACTL_CLEAR_PEND; - do_subchannel_work(sch, NULL); + do_subchannel_work(sch); ret = 0; out: @@ -1223,7 +1222,7 @@ int css_do_hsch(SubchDev *sch) } s->ctrl |= SCSW_ACTL_HALT_PEND; - do_subchannel_work(sch, NULL); + do_subchannel_work(sch); ret = 0; out: @@ -1298,7 +1297,7 @@ int css_do_ssch(SubchDev *sch, ORB *orb) s->ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND); s->flags &= ~SCSW_FLAGS_MASK_PNO; - do_subchannel_work(sch, orb); + do_subchannel_work(sch); ret = 0; out: @@ -1578,7 +1577,7 @@ int css_do_rsch(SubchDev *sch) } s->ctrl |= SCSW_ACTL_RESUME_PEND; - do_subchannel_work(sch, NULL); + do_subchannel_work(sch); ret = 0; out: -- 2.11.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/7] s390x/css: use SubchDev.orb 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 7/7] s390x/css: use SubchDev.orb Halil Pasic @ 2017-06-01 11:50 ` Cornelia Huck 0 siblings, 0 replies; 24+ messages in thread From: Cornelia Huck @ 2017-06-01 11:50 UTC (permalink / raw) To: Halil Pasic Cc: Dr. David Alan Gilbert, Dong Jia Shi, Juan Quintela, qemu-devel On Mon, 29 May 2017 15:55:20 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > Instead of passing around a pointer to ORB let us simplify some > functions signatures by using the previously introduced ORB saved at the Either "function signatures" or "functions' signatures" :) > subchannel (SubchDev). > > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> > Reviewed-by: Guenther Hutzl <hutzl@linux.vnet.ibm.com> > --- > hw/s390x/css.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) This is a nice rework. Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic ` (6 preceding siblings ...) 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 7/7] s390x/css: use SubchDev.orb Halil Pasic @ 2017-05-31 16:46 ` Dr. David Alan Gilbert 2017-06-01 11:51 ` Cornelia Huck 8 siblings, 0 replies; 24+ messages in thread From: Dr. David Alan Gilbert @ 2017-05-31 16:46 UTC (permalink / raw) To: Halil Pasic; +Cc: Cornelia Huck, Dong Jia Shi, Juan Quintela, qemu-devel * Halil Pasic (pasic@linux.vnet.ibm.com) wrote: > The scope of this patch series is now limited to decoupling channel > subsystem migration from the migration of virtio-ccw proxies. > > The vmstatifying of virtio-ccw proxies is now done by another series > currently consisting of a single patch running under the name 'vmstatify > config migration for virtio-ccw'. Since this series depends on that > patch, I have included it here too (for convenience). > > The reorganization was requested by Dave. Yes, I think that's OK now. We've now got a clearer split and no new qemu_put_*'s Dave > > v1 --> v2: > * Split out the vmystatify virtio-ccw part, reorganize > * Use WITH_TMP instead of one-shot VMStateInfo's > > Halil Pasic (7): > s390x: vmstatify config migration for virtio-ccw > s390x: add helper get_machine_class > s390x: add css_migration_enabled to machine class > s390x/css: add missing css state conditionally > s390x/css: add ORB to SubchDev > s390x/css: activate ChannelSubSys migration > s390x/css: use SubchDev.orb > > hw/intc/s390_flic.c | 48 ++++ > hw/s390x/ccw-device.c | 10 + > hw/s390x/ccw-device.h | 4 + > hw/s390x/css.c | 494 +++++++++++++++++++++++++------------ > hw/s390x/s390-virtio-ccw.c | 58 +++-- > hw/s390x/virtio-ccw.c | 155 ++++++------ > include/hw/s390x/css.h | 17 +- > include/hw/s390x/s390-virtio-ccw.h | 7 + > include/hw/s390x/s390_flic.h | 5 + > 9 files changed, 544 insertions(+), 254 deletions(-) > > -- > 2.11.2 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic ` (7 preceding siblings ...) 2017-05-31 16:46 ` [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Dr. David Alan Gilbert @ 2017-06-01 11:51 ` Cornelia Huck 8 siblings, 0 replies; 24+ messages in thread From: Cornelia Huck @ 2017-06-01 11:51 UTC (permalink / raw) To: Halil Pasic Cc: Dr. David Alan Gilbert, Dong Jia Shi, Juan Quintela, qemu-devel On Mon, 29 May 2017 15:55:13 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > The scope of this patch series is now limited to decoupling channel > subsystem migration from the migration of virtio-ccw proxies. > > The vmstatifying of virtio-ccw proxies is now done by another series > currently consisting of a single patch running under the name 'vmstatify > config migration for virtio-ccw'. Since this series depends on that > patch, I have included it here too (for convenience). > > The reorganization was requested by Dave. > > v1 --> v2: > * Split out the vmystatify virtio-ccw part, reorganize > * Use WITH_TMP instead of one-shot VMStateInfo's > > Halil Pasic (7): > s390x: vmstatify config migration for virtio-ccw > s390x: add helper get_machine_class > s390x: add css_migration_enabled to machine class > s390x/css: add missing css state conditionally > s390x/css: add ORB to SubchDev > s390x/css: activate ChannelSubSys migration > s390x/css: use SubchDev.orb > > hw/intc/s390_flic.c | 48 ++++ > hw/s390x/ccw-device.c | 10 + > hw/s390x/ccw-device.h | 4 + > hw/s390x/css.c | 494 +++++++++++++++++++++++++------------ > hw/s390x/s390-virtio-ccw.c | 58 +++-- > hw/s390x/virtio-ccw.c | 155 ++++++------ > include/hw/s390x/css.h | 17 +- > include/hw/s390x/s390-virtio-ccw.h | 7 + > include/hw/s390x/s390_flic.h | 5 + > 9 files changed, 544 insertions(+), 254 deletions(-) > This patch set looks good to me, just some comments on the individual patches. ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2017-06-08 8:52 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-29 13:55 [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Halil Pasic 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 1/7] s390x: vmstatify config migration for virtio-ccw Halil Pasic 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 2/7] s390x: add helper get_machine_class Halil Pasic 2017-06-01 11:13 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 3/7] s390x: add css_migration_enabled to machine class Halil Pasic 2017-06-01 11:16 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 4/7] s390x/css: add missing css state conditionally Halil Pasic 2017-05-31 18:52 ` Juan Quintela 2017-06-01 9:35 ` Halil Pasic 2017-06-01 11:32 ` Cornelia Huck 2017-06-01 11:46 ` Halil Pasic 2017-06-07 18:03 ` Juan Quintela 2017-06-08 8:52 ` Halil Pasic 2017-06-01 11:42 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 5/7] s390x/css: add ORB to SubchDev Halil Pasic 2017-06-01 11:45 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 6/7] s390x/css: activate ChannelSubSys migration Halil Pasic 2017-06-01 8:40 ` Thomas Huth 2017-06-01 10:12 ` Halil Pasic 2017-06-01 11:47 ` Cornelia Huck 2017-05-29 13:55 ` [Qemu-devel] [PATCH v2 7/7] s390x/css: use SubchDev.orb Halil Pasic 2017-06-01 11:50 ` Cornelia Huck 2017-05-31 16:46 ` [Qemu-devel] [PATCH v2 0/7] migration: s390x css migration Dr. David Alan Gilbert 2017-06-01 11:51 ` Cornelia Huck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).