On Mon, 8 Jul 2019 16:10:34 -0400 Farhan Ali <alifm@xxxxxxxxxxxxx> wrote: > The comment is misleading because it tells us that > we should set orb.cmd.c64 before calling ccwchain_calc_length, > otherwise the function ccwchain_calc_length would return an > error. This is not completely accurate. > > We want to allow an orb without cmd.c64, and this is fine > as long as the channel program does not use IDALs. But we do > want to reject any channel program that uses IDALs and does > not set the flag, which what we do in ccwchain_calc_length. > > After we have done the ccw processing, it should be safe > to set cmd.c64, since we will convert them into IDALs. "After we have done the ccw processing, we need to set cmd.c64, as we use IDALs for all translated channel programs." ? > Fixes: fb9e7880af35 ("vfio: ccw: push down unsupported IDA check") > Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxx> > --- > drivers/s390/cio/vfio_ccw_cp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c > index d6a8dff..7622b72 100644 > --- a/drivers/s390/cio/vfio_ccw_cp.c > +++ b/drivers/s390/cio/vfio_ccw_cp.c > @@ -645,8 +645,8 @@ int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb) > if (ret) > cp_free(cp); > > - /* It is safe to force: if not set but idals used > - * ccwchain_calc_length returns an error. > + /* It is safe to force: if it was not set but idals used > + * ccwchain_calc_length would have returned an error. Thanks, much clearer. > */ > cp->orb.cmd.c64 = 1; > Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>