From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Farman Subject: Re: [PATCH v2 2/5] vfio-ccw: concurrent I/O handling Date: Tue, 29 Jan 2019 09:14:40 -0500 Message-ID: <691e91ea-df02-1455-93ed-7e8369fceec2@linux.ibm.com> References: <20190121110354.2247-1-cohuck@redhat.com> <20190121110354.2247-3-cohuck@redhat.com> <2dac6201-9e71-b188-0385-d09d05071a1c@linux.ibm.com> <5627cb78-22b3-0557-7972-256bc9560d86@linux.ibm.com> <20190125112437.2c06fac6.cohuck@redhat.com> <20190125135835.2d59b511.cohuck@redhat.com> <20190125150101.3b61f0a1@oc2783563651> <20190128180948.506a9695.cohuck@redhat.com> <20190128201548.1ecfb84f@oc2783563651> <20190129112024.401b8184.cohuck@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190129112024.401b8184.cohuck@redhat.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel2=m.gmane.org@nongnu.org Sender: "Qemu-devel" List-Archive: List-Post: To: Cornelia Huck Cc: linux-s390@vger.kernel.org, Alex Williamson , Pierre Morel , kvm@vger.kernel.org, Farhan Ali , qemu-devel@nongnu.org, Halil Pasic , qemu-s390x@nongnu.org List-ID: On 01/29/2019 05:20 AM, Cornelia Huck wrote: > On Mon, 28 Jan 2019 16:48:10 -0500 > Eric Farman wrote: > >> On 01/28/2019 02:15 PM, Halil Pasic wrote: >>> On Mon, 28 Jan 2019 18:09:48 +0100 >>> Cornelia Huck wrote: > >> I guess if >>> the ssch() returns with non cc == 0 the CP_PENDING ---IRQ---> IDLE >>> transition >>> won't take place. And I guess the IRQ is a final one. >> >> Yes this is the one point I hadn't seen explicitly stated. We shouldn't >> remain in state=BUSY if the ssch got cc!=0, and probably return to IDLE >> when processing the failure. In Connie's response (Mon, 28 Jan 2019 >> 18:24:24 +0100) to my note, she expressed some agreement to that. > > Yes, I think that's what should happen. > > >>>> state for I/O) >>>> (normal ssch) >>>> >>>> BUSY --- IO_REQ ---> return -EAGAIN, stay in BUSY >>>> (user space is supposed to retry, as we'll eventually progress from >>>> BUSY) >>>> >>>> CP_PENDING --- IO_REQ ---> return -EBUSY, stay in CP_PENDING >>>> (user space is supposed to map this to the appropriate cc for the guest) >>> >>> From this it seems you don't intend to issue the second requested ssch() >>> any more (and don't want to do any translation). Is that right? (If it >>> is, that what I was asking for for a while, but then it's a pity for the >>> retries.) >>> >>>> >>>> IDLE --- ASYNC_REQ ---> IDLE >>>> (user space is welcome to do anything else right away) >>> >>> Your idea is to not issue a requested hsch() if we think we are IDLE >>> it seems. Do I understand this right? We would end up with a different >>> semantic for hsch()/and csch() (compared to PoP) in the guest with this >>> (AFAICT). >>> >>>> >>>> BUSY --- ASYNC_REQ ---> return -EAGAIN, stay in BUSY >>>> (user space is supposed to retry, as above) >>>> >>>> CP_PENDING --- ASYNC_REQ ---> return success, stay in CP_PENDING >>>> (the interrupt will get us out of CP_PENDING eventually) >>> >>> Issue (c|h)sch() is an action that is done on this internal >>> transition (within CP_PENDING). >> >> These three do read like CSCH/HSCH are subject to the same rules as >> SSCH, when in fact they would be (among other reasons) issued to clean >> up a lost interrupt from a previous SSCH. So maybe return -EAGAIN on >> state=BUSY (don't race ourselves with the start), but issue to hardware >> if CP_PENDING. > > I think there are some devices which require a certain hsch/csch > sequence during device bringup, so it's not just cleaning up after a > ssch. Ah, yes. Therefore, we should always try to do the requested hsch/csch, > unless things like "we're in the process of translating a cp, and can't > deal with another request right now" prevent it. Agreed. I'm in support of all of this. > >> >> If we get an async request when state=IDLE, then maybe just issue it for >> fun, as if it were an SSCH? > > For fun, but mainly because the guest wants it :) > Well, that too. ;-) From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:43347) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1goUAa-00027y-ED for qemu-devel@nongnu.org; Tue, 29 Jan 2019 09:15:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1goUAL-0003oi-5G for qemu-devel@nongnu.org; Tue, 29 Jan 2019 09:15:15 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:57860) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1goUAK-0003kZ-CP for qemu-devel@nongnu.org; Tue, 29 Jan 2019 09:15:05 -0500 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x0TE4Voq038744 for ; Tue, 29 Jan 2019 09:14:48 -0500 Received: from e31.co.us.ibm.com (e31.co.us.ibm.com [32.97.110.149]) by mx0a-001b2d01.pphosted.com with ESMTP id 2qaqtm2eva-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 29 Jan 2019 09:14:48 -0500 Received: from localhost by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 29 Jan 2019 14:14:47 -0000 References: <20190121110354.2247-1-cohuck@redhat.com> <20190121110354.2247-3-cohuck@redhat.com> <2dac6201-9e71-b188-0385-d09d05071a1c@linux.ibm.com> <5627cb78-22b3-0557-7972-256bc9560d86@linux.ibm.com> <20190125112437.2c06fac6.cohuck@redhat.com> <20190125135835.2d59b511.cohuck@redhat.com> <20190125150101.3b61f0a1@oc2783563651> <20190128180948.506a9695.cohuck@redhat.com> <20190128201548.1ecfb84f@oc2783563651> <20190129112024.401b8184.cohuck@redhat.com> From: Eric Farman Date: Tue, 29 Jan 2019 09:14:40 -0500 MIME-Version: 1.0 In-Reply-To: <20190129112024.401b8184.cohuck@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Message-Id: <691e91ea-df02-1455-93ed-7e8369fceec2@linux.ibm.com> Subject: Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: Halil Pasic , Farhan Ali , Pierre Morel , linux-s390@vger.kernel.org, kvm@vger.kernel.org, qemu-devel@nongnu.org, qemu-s390x@nongnu.org, Alex Williamson On 01/29/2019 05:20 AM, Cornelia Huck wrote: > On Mon, 28 Jan 2019 16:48:10 -0500 > Eric Farman wrote: > >> On 01/28/2019 02:15 PM, Halil Pasic wrote: >>> On Mon, 28 Jan 2019 18:09:48 +0100 >>> Cornelia Huck wrote: > >> I guess if >>> the ssch() returns with non cc == 0 the CP_PENDING ---IRQ---> IDLE >>> transition >>> won't take place. And I guess the IRQ is a final one. >> >> Yes this is the one point I hadn't seen explicitly stated. We shouldn't >> remain in state=BUSY if the ssch got cc!=0, and probably return to IDLE >> when processing the failure. In Connie's response (Mon, 28 Jan 2019 >> 18:24:24 +0100) to my note, she expressed some agreement to that. > > Yes, I think that's what should happen. > > >>>> state for I/O) >>>> (normal ssch) >>>> >>>> BUSY --- IO_REQ ---> return -EAGAIN, stay in BUSY >>>> (user space is supposed to retry, as we'll eventually progress from >>>> BUSY) >>>> >>>> CP_PENDING --- IO_REQ ---> return -EBUSY, stay in CP_PENDING >>>> (user space is supposed to map this to the appropriate cc for the guest) >>> >>> From this it seems you don't intend to issue the second requested ssch() >>> any more (and don't want to do any translation). Is that right? (If it >>> is, that what I was asking for for a while, but then it's a pity for the >>> retries.) >>> >>>> >>>> IDLE --- ASYNC_REQ ---> IDLE >>>> (user space is welcome to do anything else right away) >>> >>> Your idea is to not issue a requested hsch() if we think we are IDLE >>> it seems. Do I understand this right? We would end up with a different >>> semantic for hsch()/and csch() (compared to PoP) in the guest with this >>> (AFAICT). >>> >>>> >>>> BUSY --- ASYNC_REQ ---> return -EAGAIN, stay in BUSY >>>> (user space is supposed to retry, as above) >>>> >>>> CP_PENDING --- ASYNC_REQ ---> return success, stay in CP_PENDING >>>> (the interrupt will get us out of CP_PENDING eventually) >>> >>> Issue (c|h)sch() is an action that is done on this internal >>> transition (within CP_PENDING). >> >> These three do read like CSCH/HSCH are subject to the same rules as >> SSCH, when in fact they would be (among other reasons) issued to clean >> up a lost interrupt from a previous SSCH. So maybe return -EAGAIN on >> state=BUSY (don't race ourselves with the start), but issue to hardware >> if CP_PENDING. > > I think there are some devices which require a certain hsch/csch > sequence during device bringup, so it's not just cleaning up after a > ssch. Ah, yes. Therefore, we should always try to do the requested hsch/csch, > unless things like "we're in the process of translating a cp, and can't > deal with another request right now" prevent it. Agreed. I'm in support of all of this. > >> >> If we get an async request when state=IDLE, then maybe just issue it for >> fun, as if it were an SSCH? > > For fun, but mainly because the guest wants it :) > Well, that too. ;-)