Re: [PATCH v1 04/16] vfio/ccw: move where IDA flag is set in ORB

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 11/21/22 4:40 PM, Eric Farman wrote:
> The output of vfio_ccw is always a Format-2 IDAL, but the code that
> explicitly sets this is buried in cp_init().
> 
> In fact the input is often already a Format-2 IDAL, and would be
> rejected (via the check in ccwchain_calc_length()) if it weren't,
> so explicitly setting it doesn't do much. Setting it way down here
> only makes it impossible to make decisions in support of other
> IDAL formats.
> 
> Let's move that to where the rest of the ORB is set up, so that the
> CCW processing in cp_prefetch() is performed according to the
> contents of the unmodified guest ORB.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>  drivers/s390/cio/vfio_ccw_cp.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index 268a90252521..3a11132b1685 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
> @@ -707,15 +707,9 @@ int cp_init(struct channel_program *cp, union orb *orb)
>  	/* Build a ccwchain for the first CCW segment */
>  	ret = ccwchain_handle_ccw(orb->cmd.cpa, cp);
>  
> -	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;
>  }
>  
> @@ -837,6 +831,11 @@ union orb *cp_get_orb(struct channel_program *cp, struct subchannel *sch)
>  	orb->cmd.intparm = (u32)virt_to_phys(sch);
>  	orb->cmd.fmt = 1;
>  
> +	/*
> +	 * Everything built by vfio-ccw is a Format-2 IDAL.
> +	 */
> +	orb->cmd.c64 = 1;
> +
>  	if (orb->cmd.lpm == 0)
>  		orb->cmd.lpm = sch->lpm;
>