From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 15 May 2019 14:23:39 +0200 From: Cornelia Huck Subject: Re: [PATCH v2 5/7] s390/cio: Allow zero-length CCWs in vfio-ccw Message-ID: <20190515142339.12065a1d.cohuck@redhat.com> In-Reply-To: <20190514234248.36203-6-farman@linux.ibm.com> References: <20190514234248.36203-1-farman@linux.ibm.com> <20190514234248.36203-6-farman@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: kvm-owner@vger.kernel.org List-Archive: List-Post: To: Eric Farman Cc: Farhan Ali , Halil Pasic , Pierre Morel , linux-s390@vger.kernel.org, kvm@vger.kernel.org List-ID: On Wed, 15 May 2019 01:42:46 +0200 Eric Farman 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 > --- > drivers/s390/cio/vfio_ccw_cp.c | 26 ++++++++------------------ > 1 file changed, 8 insertions(+), 18 deletions(-)