Re: [RFC v2 1/5] vfio-ccw: Fix misleading comment when setting orb.cmd.c64

Cornelia Huck
2019-07-09T09:57:38Z

On Mon, 8 Jul 2019 16:10:34 -0400
Farhan Ali <alifm@linux.ibm.com> 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@linux.ibm.com>
> ---
> 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@redhat.com>