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

Eric Farman
2019-05-15T15:04:47Z

On 5/15/19 8:23 AM, Cornelia Huck wrote:
> 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()

s/is/was/

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

Probably. If the guest gave us a valid address, the pin might actually
work now whereas before it would fail because the length was zero. If
the address were also invalid,

>IOW, this
> has no effect on bisectability?

I think so, but I suppose that either (A) patch 5 and 6 could be
squashed together, or (B) I could move the "set pa_nr to zero" (or more
accurately, set it to ccw->count) pieces from patch 6 into this patch,
so that the vfio_pin_pages() call occurs like it does today.

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