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

Matthew Rosato
2022-12-15T20:55:15Z

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;
>