On Mon, 21 Jan 2019 10:48:32 -0500 Farhan Ali <alifm@xxxxxxxxxxxxx> wrote: > On 01/21/2019 10:18 AM, Cornelia Huck wrote: > > On Mon, 21 Jan 2019 09:54:09 -0500 > > Farhan Ali <alifm@xxxxxxxxxxxxx> wrote: > > > >> This check is unecessary as we already have the vfio state machine > >> to handle I/O requests. > >> > >> On the other hand, this check returns incorrect information to > >> userspace if the state of the subchannel is not idle. For example > >> if the state is busy and new I/O request comes in, this will return > >> an EACCES, whereas we should return EBUSY. > >> > >> Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxx> > >> --- > >> drivers/s390/cio/vfio_ccw_ops.c | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c > >> index f673e10..3fdcc6d 100644 > >> --- a/drivers/s390/cio/vfio_ccw_ops.c > >> +++ b/drivers/s390/cio/vfio_ccw_ops.c > >> @@ -193,8 +193,6 @@ static ssize_t vfio_ccw_mdev_write(struct mdev_device *mdev, > >> return -EINVAL; > >> > >> private = dev_get_drvdata(mdev_parent_dev(mdev)); > >> - if (private->state != VFIO_CCW_STATE_IDLE) > >> - return -EACCES; > >> > >> region = private->io_region; > >> if (copy_from_user((void *)region + *ppos, buf, count)) > > > > Hm, the patchset for halt/clear handling I recently posted changes this > > to a check for NOT_OPER || STANDBY. What do you think of that option? > > > > > I am concerned with the return code that we send userspace. With the > state machines we return an EIO for NOT_OPER or STANDBY, but we return > EACCES in the early check. QEMU on an EACCES returns a 'not_oper' to the > guest and for EIO will inject an interrupt. I actually think that not_oper is the saner option here: the device is in a state on the vfio side where it is not usable... > > I believe we should try to keep it consistent to make debugging errors > easier :) I certainly agree with that :)
