| Message ID | 20170428130951.44771-11-bjsdjshi@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Dong, On 28/04/2017 15:09, Dong Jia Shi wrote: > From: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> > > Introduce a new callback on subchannel to handle ccw-request. > Realize the callback in vfio-ccw device. Besides, resort to > the event notifier handler to handling the ccw-request results. > 1. Pread the I/O results via MMIO region. > 2. Update the scsw info to guest. > 3. Inject an I/O interrupt to notify guest the I/O result. > > Signed-off-by: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> > Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> > --- > hw/s390x/css.c | 4 +-- > hw/s390x/s390-ccw.h | 1 + > hw/vfio/ccw.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/s390x/css.h | 2 ++ > 4 files changed, 78 insertions(+), 2 deletions(-) > > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 1052eea..507c60f 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -259,7 +259,7 @@ uint16_t css_build_subchannel_id(SubchDev *sch) > return css_do_build_subchannel_id(sch->cssid, sch->ssid); > } > > -static void css_inject_io_interrupt(SubchDev *sch) > +void css_inject_io_interrupt(SubchDev *sch) > { > uint8_t isc = (sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ISC) >> 11; > > @@ -668,7 +668,7 @@ static void copy_pmcw_to_guest(PMCW *dest, const PMCW *src) > dest->chars = cpu_to_be32(src->chars); > } > > -static void copy_scsw_to_guest(SCSW *dest, const SCSW *src) > +void copy_scsw_to_guest(SCSW *dest, const SCSW *src) > { > dest->flags = cpu_to_be16(src->flags); > dest->ctrl = cpu_to_be16(src->ctrl); > diff --git a/hw/s390x/s390-ccw.h b/hw/s390x/s390-ccw.h > index b58d8e9..4e2fa65 100644 > --- a/hw/s390x/s390-ccw.h > +++ b/hw/s390x/s390-ccw.h > @@ -27,6 +27,7 @@ typedef struct S390CCWDevice { > CcwDevice parent_obj; > CssDevId hostid; > char *mdevid; > + int (*handle_request) (ORB *, SCSW *, void *); shouldn't this callback be part of the class instead? > } S390CCWDevice; > > typedef struct S390CCWDeviceClass { > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > index cda3057..3ff78c5 100644 > --- a/hw/vfio/ccw.c > +++ b/hw/vfio/ccw.c > @@ -47,6 +47,36 @@ struct VFIODeviceOps vfio_ccw_ops = { > .vfio_compute_needs_reset = vfio_ccw_compute_needs_reset, > }; > > +static int vfio_ccw_handle_request(ORB *orb, SCSW *scsw, void *data) > +{ > + S390CCWDevice *cdev = data; > + VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev); > + struct ccw_io_region *region = vcdev->io_region; > + int ret; > + > + QEMU_BUILD_BUG_ON(sizeof(region->orb_area) != sizeof(ORB)); > + QEMU_BUILD_BUG_ON(sizeof(region->scsw_area) != sizeof(SCSW)); > + QEMU_BUILD_BUG_ON(sizeof(region->irb_area) != sizeof(IRB)); > + > + memset(region, 0, sizeof(*region)); > + > + memcpy(region->orb_area, orb, sizeof(ORB)); > + memcpy(region->scsw_area, scsw, sizeof(SCSW)); > + > +again: > + ret = pwrite(vcdev->vdev.fd, region, > + vcdev->io_region_size, vcdev->io_region_offset); > + if (ret != vcdev->io_region_size) { > + if (errno == EAGAIN) { > + goto again; > + } > + error_report("vfio-ccw: wirte I/O region failed with errno=%d", errno); > + return -errno; > + } > + > + return region->ret_code; > +} > + > static void vfio_ccw_reset(DeviceState *dev) > { > CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev); > @@ -59,10 +89,52 @@ static void vfio_ccw_reset(DeviceState *dev) > static void vfio_ccw_io_notifier_handler(void *opaque) > { > VFIOCCWDevice *vcdev = opaque; > + struct ccw_io_region *region = vcdev->io_region; > + S390CCWDevice *cdev = S390_CCW_DEVICE(vcdev); > + CcwDevice *ccw_dev = CCW_DEVICE(cdev); > + SubchDev *sch = ccw_dev->sch; > + SCSW *s = &sch->curr_status.scsw; > + IRB irb; > > if (!event_notifier_test_and_clear(&vcdev->io_notifier)) { > return; > } > + > + if (pread(vcdev->vdev.fd, region, > + vcdev->io_region_size, vcdev->io_region_offset) == -1) { is it possible to read less bytes than requested? Thanks Eric > + switch (errno) { > + case ENODEV: > + /* Generate a deferred cc 3 condition. */ > + s->flags |= SCSW_FLAGS_MASK_CC; > + s->ctrl &= ~SCSW_CTRL_MASK_STCTL; > + s->ctrl |= (SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND); > + goto read_err; > + case EFAULT: > + /* memory problem, generate channel data check */ > + s->ctrl &= ~SCSW_ACTL_START_PEND; > + s->cstat = SCSW_CSTAT_DATA_CHECK; > + s->ctrl &= ~SCSW_CTRL_MASK_STCTL; > + s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY | > + SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND; > + goto read_err; > + default: > + /* error, generate channel program check */ > + s->ctrl &= ~SCSW_ACTL_START_PEND; > + s->cstat = SCSW_CSTAT_PROG_CHECK; > + s->ctrl &= ~SCSW_CTRL_MASK_STCTL; > + s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY | > + SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND; > + goto read_err; > + } > + } > + > + memcpy(&irb, region->irb_area, sizeof(IRB)); > + > + /* Update control block via irb. */ > + copy_scsw_to_guest(s, &irb.scsw); > + > +read_err: > + css_inject_io_interrupt(sch); > } > > static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) > @@ -235,6 +307,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) > VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev); > S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev); > > + cdev->handle_request = vfio_ccw_handle_request; > /* Call the class init function for subchannel. */ > if (cdc->realize) { > cdc->realize(cdev, vcdev->vdev.sysfsdev, errp); > diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h > index a8bf1db..c8c283a 100644 > --- a/include/hw/s390x/css.h > +++ b/include/hw/s390x/css.h > @@ -135,6 +135,8 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, > void css_sch_build_virtual_schib(SubchDev *sch, uint8_t chpid, uint8_t type); > int css_sch_build_schib(SubchDev *sch, CssDevId *dev_id); > uint16_t css_build_subchannel_id(SubchDev *sch); > +void copy_scsw_to_guest(SCSW *dest, const SCSW *src); > +void css_inject_io_interrupt(SubchDev *sch); > void css_reset(void); > void css_reset_sch(SubchDev *sch); > void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid); >
On Tue, 2 May 2017 10:15:19 +0800 Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote: > * Auger Eric <eric.auger@redhat.com> [2017-05-01 19:28:23 +0200]: > > > static void vfio_ccw_io_notifier_handler(void *opaque) > > > { > > > VFIOCCWDevice *vcdev = opaque; > > > + struct ccw_io_region *region = vcdev->io_region; > > > + S390CCWDevice *cdev = S390_CCW_DEVICE(vcdev); > > > + CcwDevice *ccw_dev = CCW_DEVICE(cdev); > > > + SubchDev *sch = ccw_dev->sch; > > > + SCSW *s = &sch->curr_status.scsw; > > > + IRB irb; > > > > > > if (!event_notifier_test_and_clear(&vcdev->io_notifier)) { > > > return; > > > } > > > + > > > + if (pread(vcdev->vdev.fd, region, > > > + vcdev->io_region_size, vcdev->io_region_offset) == -1) { > > is it possible to read less bytes than requested? > Currently, impossible. The kernel side returns either the full region, > or an error. > But we surely could make it more friendly for a partial return. I will > add a check for that case, and generate a channel program check for it. I think a channel-control check would be more appropriate here.
On Wed, 3 May 2017 10:18:50 +0800 Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote: > * Cornelia Huck <cornelia.huck@de.ibm.com> [2017-05-02 18:04:07 +0200]: > > > On Tue, 2 May 2017 10:15:19 +0800 > > Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote: > > > > > * Auger Eric <eric.auger@redhat.com> [2017-05-01 19:28:23 +0200]: > > > > > > > static void vfio_ccw_io_notifier_handler(void *opaque) > > > > > { > > > > > VFIOCCWDevice *vcdev = opaque; > > > > > + struct ccw_io_region *region = vcdev->io_region; > > > > > + S390CCWDevice *cdev = S390_CCW_DEVICE(vcdev); > > > > > + CcwDevice *ccw_dev = CCW_DEVICE(cdev); > > > > > + SubchDev *sch = ccw_dev->sch; > > > > > + SCSW *s = &sch->curr_status.scsw; > > > > > + IRB irb; > > > > > > > > > > if (!event_notifier_test_and_clear(&vcdev->io_notifier)) { > > > > > return; > > > > > } > > > > > + > > > > > + if (pread(vcdev->vdev.fd, region, > > > > > + vcdev->io_region_size, vcdev->io_region_offset) == -1) { > > > > is it possible to read less bytes than requested? > > > Currently, impossible. The kernel side returns either the full region, > > > or an error. > > > But we surely could make it more friendly for a partial return. I will > > > add a check for that case, and generate a channel program check for it. > > > > I think a channel-control check would be more appropriate here. > > Hi Conny, > > After reading PoP 14-43, I think you are right. (...) > So I decide to add this piece of code: > if (size != vcdev->io_region_size) { > /* Information transfer error, generate channel-control check. */ > s->ctrl &= ~SCSW_ACTL_START_PEND; > s->cstat = SCSW_CSTAT_CHN_CTRL_CHK; > s->ctrl &= ~SCSW_CTRL_MASK_STCTL; > s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY | > SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND; > goto read_err; > } Looks good.
diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 1052eea..507c60f 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -259,7 +259,7 @@ uint16_t css_build_subchannel_id(SubchDev *sch) return css_do_build_subchannel_id(sch->cssid, sch->ssid); } -static void css_inject_io_interrupt(SubchDev *sch) +void css_inject_io_interrupt(SubchDev *sch) { uint8_t isc = (sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ISC) >> 11; @@ -668,7 +668,7 @@ static void copy_pmcw_to_guest(PMCW *dest, const PMCW *src) dest->chars = cpu_to_be32(src->chars); } -static void copy_scsw_to_guest(SCSW *dest, const SCSW *src) +void copy_scsw_to_guest(SCSW *dest, const SCSW *src) { dest->flags = cpu_to_be16(src->flags); dest->ctrl = cpu_to_be16(src->ctrl); diff --git a/hw/s390x/s390-ccw.h b/hw/s390x/s390-ccw.h index b58d8e9..4e2fa65 100644 --- a/hw/s390x/s390-ccw.h +++ b/hw/s390x/s390-ccw.h @@ -27,6 +27,7 @@ typedef struct S390CCWDevice { CcwDevice parent_obj; CssDevId hostid; char *mdevid; + int (*handle_request) (ORB *, SCSW *, void *); } S390CCWDevice; typedef struct S390CCWDeviceClass { diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index cda3057..3ff78c5 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -47,6 +47,36 @@ struct VFIODeviceOps vfio_ccw_ops = { .vfio_compute_needs_reset = vfio_ccw_compute_needs_reset, }; +static int vfio_ccw_handle_request(ORB *orb, SCSW *scsw, void *data) +{ + S390CCWDevice *cdev = data; + VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev); + struct ccw_io_region *region = vcdev->io_region; + int ret; + + QEMU_BUILD_BUG_ON(sizeof(region->orb_area) != sizeof(ORB)); + QEMU_BUILD_BUG_ON(sizeof(region->scsw_area) != sizeof(SCSW)); + QEMU_BUILD_BUG_ON(sizeof(region->irb_area) != sizeof(IRB)); + + memset(region, 0, sizeof(*region)); + + memcpy(region->orb_area, orb, sizeof(ORB)); + memcpy(region->scsw_area, scsw, sizeof(SCSW)); + +again: + ret = pwrite(vcdev->vdev.fd, region, + vcdev->io_region_size, vcdev->io_region_offset); + if (ret != vcdev->io_region_size) { + if (errno == EAGAIN) { + goto again; + } + error_report("vfio-ccw: wirte I/O region failed with errno=%d", errno); + return -errno; + } + + return region->ret_code; +} + static void vfio_ccw_reset(DeviceState *dev) { CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev); @@ -59,10 +89,52 @@ static void vfio_ccw_reset(DeviceState *dev) static void vfio_ccw_io_notifier_handler(void *opaque) { VFIOCCWDevice *vcdev = opaque; + struct ccw_io_region *region = vcdev->io_region; + S390CCWDevice *cdev = S390_CCW_DEVICE(vcdev); + CcwDevice *ccw_dev = CCW_DEVICE(cdev); + SubchDev *sch = ccw_dev->sch; + SCSW *s = &sch->curr_status.scsw; + IRB irb; if (!event_notifier_test_and_clear(&vcdev->io_notifier)) { return; } + + if (pread(vcdev->vdev.fd, region, + vcdev->io_region_size, vcdev->io_region_offset) == -1) { + switch (errno) { + case ENODEV: + /* Generate a deferred cc 3 condition. */ + s->flags |= SCSW_FLAGS_MASK_CC; + s->ctrl &= ~SCSW_CTRL_MASK_STCTL; + s->ctrl |= (SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND); + goto read_err; + case EFAULT: + /* memory problem, generate channel data check */ + s->ctrl &= ~SCSW_ACTL_START_PEND; + s->cstat = SCSW_CSTAT_DATA_CHECK; + s->ctrl &= ~SCSW_CTRL_MASK_STCTL; + s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY | + SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND; + goto read_err; + default: + /* error, generate channel program check */ + s->ctrl &= ~SCSW_ACTL_START_PEND; + s->cstat = SCSW_CSTAT_PROG_CHECK; + s->ctrl &= ~SCSW_CTRL_MASK_STCTL; + s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY | + SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND; + goto read_err; + } + } + + memcpy(&irb, region->irb_area, sizeof(IRB)); + + /* Update control block via irb. */ + copy_scsw_to_guest(s, &irb.scsw); + +read_err: + css_inject_io_interrupt(sch); } static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) @@ -235,6 +307,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev); S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev); + cdev->handle_request = vfio_ccw_handle_request; /* Call the class init function for subchannel. */ if (cdc->realize) { cdc->realize(cdev, vcdev->vdev.sysfsdev, errp); diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h index a8bf1db..c8c283a 100644 --- a/include/hw/s390x/css.h +++ b/include/hw/s390x/css.h @@ -135,6 +135,8 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, void css_sch_build_virtual_schib(SubchDev *sch, uint8_t chpid, uint8_t type); int css_sch_build_schib(SubchDev *sch, CssDevId *dev_id); uint16_t css_build_subchannel_id(SubchDev *sch); +void copy_scsw_to_guest(SCSW *dest, const SCSW *src); +void css_inject_io_interrupt(SubchDev *sch); void css_reset(void); void css_reset_sch(SubchDev *sch); void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid);