On Mon, 28 Jan 2019 20:15:48 +0100 Halil Pasic <pasic@xxxxxxxxxxxxx> wrote: > On Mon, 28 Jan 2019 18:09:48 +0100 > Cornelia Huck <cohuck@xxxxxxxxxx> wrote: > > > On Fri, 25 Jan 2019 15:01:01 +0100 > > Halil Pasic <pasic@xxxxxxxxxxxxx> wrote: > > > > > On Fri, 25 Jan 2019 13:58:35 +0100 > > > Cornelia Huck <cohuck@xxxxxxxxxx> wrote: > > > > > > - The code should not be interrupted while we process the channel > > > > program, do the ssch etc. We want the caller to try again later (i.e. > > > > return -EAGAIN) > > > > (...) > > > > > > - With the async interface, we want user space to be able to submit a > > > > halt/clear while a start request is still in flight, but not while > > > > we're processing a start request with translation etc. We probably > > > > want to do -EAGAIN in that case. > > > > > > This reads very similar to your first point. > > > > Not quite. ssch() means that we have a cp around; for hsch()/csch() we > > don't have such a thing. So we want to protect the process of > > translating the cp etc., but we don't need such protection for the > > halt/clear processing. > > > > What does this don't 'need such protection' mean in terms of code, > moving the unlock of the io_mutex upward (in > vfio_ccw_async_region_write())? We don't have a cp that we need to process, so we don't need protection for that. > > > > IDLE --- IO_REQ --> BUSY ---> CP_PENDING --- IRQ ---> IDLE (if final > > There ain't no trigger/action list between BUSY and CP_PENDING. > I'm also in the dark about where the issuing of the ssch() happen > here (is it an internal transition within CP_PENDING?). 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. Please refer to the original ideas. This is obviously not supposed to be a complete description of every case we might encounter. > > 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.) Which "second requested ssch"? In the first case, user space is supposed to retry; in the second case, it should map it to a cc (and the guest does whatever it does on busy conditions). We can't issue a ssch if we're not able to handle multiple cps. > > > > > 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). Nope, we're doing hsch/csch. We're just not moving out of IDLE, as we (a) don't have any cp processing we need to protect and (b) no need to fence of multiple attempts of hsch/csch. > > > > > 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). Yes. hsch/csch do not trigger a state change (other than possibly dropping into NOT_OPER for cc 3).