Re: [RFC PATCH v1 03/10] vfio-ccw: Use subchannel lpm in the orb

[Cornelia Huck <cohuck@redhat.com>]

On Fri, 15 Nov 2019 03:56:13 +0100
Eric Farman <farman@linux.ibm.com> wrote:

> From: Farhan Ali <alifm@linux.ibm.com>
> 
> The subchannel logical path mask (lpm) would have the most
> up to date information of channel paths that are logically
> available for the subchannel.
> 
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
> 
> Notes:
>     v0->v1: [EF]
>      - None; however I am greatly confused by this one.  Thoughts?

I think it's actually wrong.

> 
>  drivers/s390/cio/vfio_ccw_cp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index 3645d1720c4b..d4a86fb9d162 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
> @@ -779,9 +779,7 @@ union orb *cp_get_orb(struct channel_program *cp, u32 intparm, u8 lpm)
>  	orb->cmd.intparm = intparm;
>  	orb->cmd.fmt = 1;
>  	orb->cmd.key = PAGE_DEFAULT_KEY >> 4;
> -
> -	if (orb->cmd.lpm == 0)
> -		orb->cmd.lpm = lpm;

In the end, the old code will use the lpm from the subchannel
structure, if userspace did not supply anything to be used.

> +	orb->cmd.lpm = lpm;

The new code will always discard any lpm userspace has supplied and
replace it with the lpm from the subchannel structure. This feels
wrong; what if the I/O is supposed to be restricted to a subset of the
paths?

If we want to include the current value of the subchannel lpm in the
requests, we probably want to AND the masks instead.

>  
>  	chain = list_first_entry(&cp->ccwchain_list, struct ccwchain, next);
>  	cpa = chain->ch_ccw;