Re: [Qemu-devel] [PATCH 0/3] vfio-ccw: support hsch/csch (kernel part)

[Farhan Ali <alifm@linux.ibm.com>]

On 12/06/2018 09:39 AM, Cornelia Huck wrote:
> On Wed, 5 Dec 2018 13:34:11 -0500
> Farhan Ali <alifm@linux.ibm.com> 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?


> 
> My idea above was to return -EAGAIN instead of -EBUSY, so that user
> space can retry the operation.
> 
>