On Fri, 23 Nov 2018 15:13:12 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
> On 22/11/2018 17:54, Cornelia Huck wrote:
> > A vfio-ccw device may provide an async command subregion for
> > issuing halt/clear subchannel requests. If it is present, use
> > it for sending halt/clear request to the device; if not, fall
> > back to emulation (as done today).
> >
> > Signed-off-by: Cornelia Huck <cohuck@xxxxxxxxxx>
> > ---
> > hw/s390x/css.c | 27 +++++++--
> > hw/vfio/ccw.c | 109 +++++++++++++++++++++++++++++++++++-
> > include/hw/s390x/s390-ccw.h | 3 +
> > 3 files changed, 133 insertions(+), 6 deletions(-)
> >
> > @@ -114,6 +120,87 @@ again:
> > }
> > }
> >
> > +int vfio_ccw_handle_clear(SubchDev *sch)
> > +{
> > + S390CCWDevice *cdev = sch->driver_data;
> > + VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
> > + struct ccw_cmd_region *region = vcdev->async_cmd_region;
> > + int ret;
> > +
> > + if (!vcdev->async_cmd_region) {
> > + /* Async command region not available, fall back to emulation */
> > + return -ENOSYS;
> > + }
> > +
> > + memset(region, 0, sizeof(*region));
> > + region->command = VFIO_CCW_ASYNC_CMD_CSCH;
> > +
> > +again:
> > + ret = pwrite(vcdev->vdev.fd, region,
> > + vcdev->async_cmd_region_size, vcdev->async_cmd_region_offset);
> > + if (ret != vcdev->async_cmd_region_size) {
> > + if (errno == EAGAIN) {
>
>
> Where do the EAGAIN come from?
It might be set by pwrite.
>
>
> > + goto again;
> > + }
> > + error_report("vfio-ccw: wirte I/O region failed with errno=%d", errno);
I should also fix up this message here and for hsch as well :)
> > + ret = -errno;
> > + } else {
> > + ret = region->ret_code;
> > + }
> > + switch (ret) {
> > + case 0:
> > + case -ENODEV:
> > + case -EACCES:
>
> should never happen?
It should not happen; but it can nevertheless be returned, so...
>
> > + return 0;
> > + case -EFAULT:
> > + default:
> > + sch_gen_unit_exception(sch);
> > + css_inject_io_interrupt(sch);
> > + return 0;
> > + }
> > +}
> > +
>
> otherwise LGTM
Thanks!
