On 7/11/19 10:28 AM, Farhan Ali 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 is what we do in ccwchain_calc_length.
>
> After we have done the ccw processing, we need to set cmd.c64,
> as we use IDALs for all translated channel programs.
>
> Also for better code readability let's move the setting of
> cmd.c64 within the non error path.
>
Per Conny in v2:
Fixes: fb9e7880af35 ("vfio: ccw: push down unsupported IDA check")
> Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxx>
> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
Reviewed-by: Eric Farman <farman@xxxxxxxxxxxxx>
> ---
> drivers/s390/cio/vfio_ccw_cp.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index d6a8dff..c969d48 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
> @@ -645,14 +645,15 @@ 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.
> - */
> - cp->orb.cmd.c64 = 1;
> -
> - if (!ret)
> + if (!ret) {
> cp->initialized = true;
>
> + /* It is safe to force: if it was not set but idals used
> + * ccwchain_calc_length would have returned an error.
> + */
> + cp->orb.cmd.c64 = 1;
> + }
> +
> return ret;
> }
>
>
