On Mon, 26 Nov 2018 19:01:55 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
> On 26/11/2018 10:58, Cornelia Huck wrote:
> > 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.
>
> I saw that the man indicate this, and so we are legitimate to handle the
> fail case, but I did not find EAGAIN in the path of the write for
> accessing devices and I did not find it in the access to the CSS.
>
> If we do not set it explicitly from the driver, the concern I have is:
> isn't it dangerous to try again and shouldn't we better abort?
If it is set by the reason stated in the man page, retry sounds like
the sensible thing, doesn't it? I don't think I've yet seen it in
practice, though.
[I don't think we should need to comb through the whole code path to
find out what might happen or not, at some point we'll just have to
trust the documentation.]
