On Thu, 6 Dec 2018 10:26:12 -0500 Farhan Ali <alifm@xxxxxxxxxxxxx> wrote: > On 12/06/2018 09:39 AM, Cornelia Huck wrote: > > On Wed, 5 Dec 2018 13:34:11 -0500 > > Farhan Ali <alifm@xxxxxxxxxxxxx> wrote: > > > >> On 12/05/2018 07:54 AM, Cornelia Huck wrote: > >>>> Yeah, that is perfectly clear, but it ain't the complete story. E.g. > >>>> are subsequent commands blocking until the preceding command finishes > >>>> is part of the interface. And what is good implementation depends on the > >>>> answer. What I mean, I first need to understand how things are supposed > >>>> to work (together) so I can double check that against the > >>>> implementation. Otherwise all I can do is nitpicking. > >>>> > >>>> To get more tangible: we are in the middle of processing an SSCH request > >>>> (e.g. doing the translation) when a HSCH comes in. What should happen? > >>>> Should we start processing HSCH after he instruction part of SSCH is > >>>> done -- which currently includes translation? Or should we -EBUSY? Or do > >>>> we abort START related activities and do the HALT stuff? > >>> I think most of the sorting-out-the-operations stuff should be done by > >>> the hardware itself, and we should not really try to enforce anything > >>> special in our vfio code. > >>> > >>> For your example, it might be best if a hsch is always accepted and > >>> send on towards the hardware. Probably best to reflect back -EAGAIN if > >>> we're currently processing another instruction from another vcpu, so > >>> that the user space caller can retry. Same for ssch, if another ssch is > >>> already being processed. We*could* reflect cc 2 if the fctl bit is > >>> already set, but that won't do for csch, so it is probably best to have > >>> the hardware figure that out in any case. > >>> > >>> If I read the code correctly, we currently reflect -EBUSY and not > >>> -EAGAIN if we get a ssch request while already processing another one. > >>> QEMU hands that back to the guest as a cc 2, which is not 100% correct. > >>> In practice, we don't see this with Linux guests due to locking. > >>> > >> > >> If we have a ssch and a csch immediately afterwards from userspace, will > >> we end up issuing csch first and then ssch to the hardware? > >> > >> If I understand correctly, the ccw translation as part of the ssch can > >> be a slow operation so it might be possible we issue the csch first? > >> In that case we won't actually clear the original start function as > >> intended. > > > > When we start processing the ssch request (translation and so on), we > > set the state to BUSY. This means that any csch request will get a > > -EBUSY, no overtaking possible. (I think maybe I'll need to check what > > this series looks like if I rebase it on top of Pierre's rework, as he > > did some changes in the state machine.) > > I think you meant the state is set to BOXED? otherwise the patch 3 says > if state is BUSY and CLEAR event request comes in, we issue the clear > instruction, no? That's what I meant with "need to rebase" :) The BOXED state is gone; I just had not rebased on top of it. There's more changes in the state machine; if we are on the same page as to what should happen, I can start massaging the patches.