Re: [PATCH v2 5/7] s390/cio: Allow zero-length CCWs in vfio-ccw

[Cornelia Huck <cohuck@redhat.com>]

On Wed, 15 May 2019 01:42:46 +0200
Eric Farman <farman@linux.ibm.com> wrote:

> It is possible that a guest might issue a CCW with a length of zero,
> and will expect a particular response.  Consider this chain:
> 
>    Address   Format-1 CCW
>    --------  -----------------
>  0 33110EC0  346022CC 33177468
>  1 33110EC8  CF200000 3318300C
> 
> CCW[0] moves a little more than two pages, but also has the
> Suppress Length Indication (SLI) bit set to handle the expectation
> that considerably less data will be moved.  CCW[1] also has the SLI
> bit set, and has a length of zero.  Once vfio-ccw does its magic,
> the kernel issues a start subchannel on behalf of the guest with this:
> 
>    Address   Format-1 CCW
>    --------  -----------------
>  0 021EDED0  346422CC 021F0000
>  1 021EDED8  CF240000 3318300C
> 
> Both CCWs were converted to an IDAL and have the corresponding flags
> set (which is by design), but only the address of the first data
> address is converted to something the host is aware of.  The second
> CCW still has the address used by the guest, which happens to be (A)
> (probably) an invalid address for the host, and (B) an invalid IDAW
> address (doubleword boundary, etc.).
> 
> While the I/O fails, it doesn't fail correctly.  In this example, we
> would receive a program check for an invalid IDAW address, instead of
> a unit check for an invalid command.
> 
> To fix this, revert commit 4cebc5d6a6ff ("vfio: ccw: validate the
> count field of a ccw before pinning") and allow the individual fetch
> routines to process them like anything else.  We'll make a slight
> adjustment to our allocation of the pfn_array (for direct CCWs) or
> IDAL (for IDAL CCWs) memory, so that we have room for at least one
> address even though no data will be transferred.
> 
> Note that this doesn't provide us with a channel program that will
> fail in the expected way.  Since our length is zero, vfio_pin_pages()
> returns -EINVAL and cp_prefetch() will thus fail.  This will be fixed
> in the next patch.

So, this failed before, and still fails, just differently? IOW, this
has no effect on bisectability?

> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  drivers/s390/cio/vfio_ccw_cp.c | 26 ++++++++------------------
>  1 file changed, 8 insertions(+), 18 deletions(-)