Hi Dong, On 01/05/2017 13:20, Auger Eric wrote: > Hi Dong, > > On 28/04/2017 15:09, Dong Jia Shi wrote: >> In order to support subchannels pass-through, we introduce a s390 >> subchannel device called "s390-ccw" to hold the real subchannel info. >> The s390-ccw devices inherit from the abstract CcwDevice which connect >> to the existing virtual-css-bus. Forget that one. Thought my first reply has not been sent. Thanks Eric >> >> Signed-off-by: Dong Jia Shi <bjsdjshi@xxxxxxxxxxxxxxxxxx> >> --- >> hw/s390x/Makefile.objs | 1 + >> hw/s390x/s390-ccw.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++ >> hw/s390x/s390-ccw.h | 38 ++++++++++++++ >> 3 files changed, 174 insertions(+) >> create mode 100644 hw/s390x/s390-ccw.c >> create mode 100644 hw/s390x/s390-ccw.h >> >> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs >> index 41ac4ec..72a3d37 100644 >> --- a/hw/s390x/Makefile.objs >> +++ b/hw/s390x/Makefile.objs >> @@ -13,3 +13,4 @@ obj-y += ccw-device.o >> obj-y += s390-pci-bus.o s390-pci-inst.o >> obj-y += s390-skeys.o >> obj-$(CONFIG_KVM) += s390-skeys-kvm.o >> +obj-y += s390-ccw.o >> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c >> new file mode 100644 >> index 0000000..8b4b9cf >> --- /dev/null >> +++ b/hw/s390x/s390-ccw.c >> @@ -0,0 +1,135 @@ >> +/* >> + * s390 CCW Assignment Support >> + * >> + * Copyright 2017 IBM Corp >> + * Author(s): Dong Jia Shi <bjsdjshi@xxxxxxxxxxxxxxxxxx> >> + * Xiao Feng Ren <renxiaof@xxxxxxxxxxxxxxxxxx> >> + * Pierre Morel <pmorel@xxxxxxxxxxxxxxxxxx> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 >> + * or (at your option) any later version. See the COPYING file in the >> + * top-level directory. >> + */ >> +#include "qemu/osdep.h" >> +#include "qapi/error.h" >> +#include "hw/sysbus.h" >> +#include "libgen.h" >> +#include "hw/s390x/css.h" >> +#include "hw/s390x/css-bridge.h" >> +#include "s390-ccw.h" >> + >> +static void s390_ccw_get_dev_info(S390CCWDevice *cdev, >> + char *sysfsdev, >> + Error **errp) >> +{ >> + unsigned int cssid, ssid, devid; >> + char dev_path[PATH_MAX] = {0}, *tmp; >> + >> + if (!sysfsdev) { >> + error_setg(errp, "No host device provided"); >> + error_append_hint(errp, >> + "Use -device vfio-ccw,sysfsdev=PATH_TO_DEVICE\n"); >> + return; >> + } >> + >> + if (!realpath(sysfsdev, dev_path)) { >> + error_setg(errp, "Host device '%s' not found", sysfsdev); >> + return; >> + } >> + >> + cdev->mdevid = g_strdup(basename(dev_path)); >> + >> + tmp = basename(dirname(dev_path)); >> + sscanf(tmp, "%2x.%1x.%4x", &cssid, &ssid, &devid); > I guess you don't need to check sscanf succeeded? >> + >> + cdev->hostid.cssid = cssid; >> + cdev->hostid.ssid = ssid; >> + cdev->hostid.devid = devid; >> + cdev->hostid.valid = true; >> +} >> + >> +static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp) >> +{ >> + CcwDevice *ccw_dev = CCW_DEVICE(cdev); >> + CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); >> + DeviceState *parent = DEVICE(ccw_dev); >> + BusState *qbus = qdev_get_parent_bus(parent); >> + VirtualCssBus *cbus = VIRTUAL_CSS_BUS(qbus); >> + SubchDev *sch; >> + int ret; >> + Error *err = NULL; >> + >> + s390_ccw_get_dev_info(cdev, sysfsdev, errp); >> + if (*errp) { > In the past, I was told by Markus that derefencing errp was not clean in > general since caller could pass NULL to ignore errors (although qdev > core doesn't). You may use a local error instead. >> + return; >> + } >> + >> + sch = css_create_sch(ccw_dev->devno, false, cbus->squash_mcss, errp); >> + if (!sch) { >> + return; >> + } >> + sch->driver_data = cdev; >> + >> + ccw_dev->sch = sch; >> + ret = css_sch_build_schib(sch, &cdev->hostid); >> + if (ret) { >> + error_setg(&err, "%s: Failed to build initial schib: %d", >> + __func__, ret); >> + goto out_err; >> + } >> + >> + ck->realize(ccw_dev, &err); >> + if (err) { >> + goto out_err; >> + } >> + >> + css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, >> + parent->hotplugged, 1); >> + return; >> + >> +out_err: >> + error_propagate(errp, err); >> + css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL); >> + ccw_dev->sch = NULL; >> + g_free(sch); > don't you want to free cdev->mdevid too? > > Thanks > > Eric >> +} >> + >> +static void s390_ccw_unrealize(S390CCWDevice *cdev, Error **errp) >> +{ >> + CcwDevice *ccw_dev = CCW_DEVICE(cdev); >> + SubchDev *sch = ccw_dev->sch; >> + >> + if (sch) { >> + css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL); >> + g_free(sch); >> + ccw_dev->sch = NULL; >> + } >> + >> + g_free(cdev->mdevid); >> +} >> + >> +static void s390_ccw_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + S390CCWDeviceClass *cdc = S390_CCW_DEVICE_CLASS(klass); >> + >> + dc->bus_type = TYPE_VIRTUAL_CSS_BUS; >> + cdc->realize = s390_ccw_realize; >> + cdc->unrealize = s390_ccw_unrealize; >> +} >> + >> +static const TypeInfo s390_ccw_info = { >> + .name = TYPE_S390_CCW, >> + .parent = TYPE_CCW_DEVICE, >> + .instance_size = sizeof(S390CCWDevice), >> + .class_size = sizeof(S390CCWDeviceClass), >> + .class_init = s390_ccw_class_init, >> + .abstract = true, >> +}; >> + >> +static void register_s390_ccw_type(void) >> +{ >> + type_register_static(&s390_ccw_info); >> +} >> + >> +type_init(register_s390_ccw_type) >> diff --git a/hw/s390x/s390-ccw.h b/hw/s390x/s390-ccw.h >> new file mode 100644 >> index 0000000..b58d8e9 >> --- /dev/null >> +++ b/hw/s390x/s390-ccw.h >> @@ -0,0 +1,38 @@ >> +/* >> + * s390 CCW Assignment Support >> + * >> + * Copyright 2017 IBM Corp. >> + * Author(s): Dong Jia Shi <bjsdjshi@xxxxxxxxxxxxxxxxxx> >> + * Xiao Feng Ren <renxiaof@xxxxxxxxxxxxxxxxxx> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or (at >> + * your option) any later version. See the COPYING file in the top-level >> + * directory. >> + */ >> + >> +#ifndef HW_S390_CCW_H >> +#define HW_S390_CCW_H >> + >> +#include "hw/s390x/ccw-device.h" >> + >> +#define TYPE_S390_CCW "s390-ccw" >> +#define S390_CCW_DEVICE(obj) \ >> + OBJECT_CHECK(S390CCWDevice, (obj), TYPE_S390_CCW) >> +#define S390_CCW_DEVICE_CLASS(klass) \ >> + OBJECT_CLASS_CHECK(S390CCWDeviceClass, (klass), TYPE_S390_CCW) >> +#define S390_CCW_DEVICE_GET_CLASS(obj) \ >> + OBJECT_GET_CLASS(S390CCWDeviceClass, (obj), TYPE_S390_CCW) >> + >> +typedef struct S390CCWDevice { >> + CcwDevice parent_obj; >> + CssDevId hostid; >> + char *mdevid; >> +} S390CCWDevice; >> + >> +typedef struct S390CCWDeviceClass { >> + CCWDeviceClass parent_class; >> + void (*realize)(S390CCWDevice *dev, char *sysfsdev, Error **errp); >> + void (*unrealize)(S390CCWDevice *dev, Error **errp); >> +} S390CCWDeviceClass; >> + >> +#endif >>