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

[Cornelia Huck <cohuck@redhat.com>]

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>