On Tue, 29 Jan 2019 09:14:40 -0500 Eric Farman <farman@xxxxxxxxxxxxx> wrote: > On 01/29/2019 05:20 AM, Cornelia Huck wrote: > > On Mon, 28 Jan 2019 16:48:10 -0500 > > Eric Farman <farman@xxxxxxxxxxxxx> wrote: > > > >> On 01/28/2019 02:15 PM, Halil Pasic wrote: > >>> On Mon, 28 Jan 2019 18:09:48 +0100 > >>> Cornelia Huck <cohuck@xxxxxxxxxx> 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. Cool. In the meantime, I've coded the changes, and I think the result looks reasonable. I'll give it some testing and then send it out; it's probably easier to discuss it with some code in front of us. [The QEMU part should not need any changes.] > > > > >> > >> 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. ;-) >