Re: [qemu-s390x] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling

[Eric Farman]

On 01/29/2019 05:20 AM, Cornelia Huck wrote:
> On Mon, 28 Jan 2019 16:48:10 -0500
> Eric Farman <address@hidden> wrote:
>
> > On 01/28/2019 02:15 PM, Halil Pasic wrote:
> > > On Mon, 28 Jan 2019 18:09:48 +0100
> > > Cornelia Huck <address@hidden> 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.  ;-)