Re: [PATCH 3/3] kvm-s390: streamline memslot handling - rebased

[Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>]

Avi Kivity wrote:
> Marcelo Tosatti wrote:
>>  
>>> (continued below)
>>>    
>>>> Anyway, yeah, the set request / wait mechanism you implement here is
>>>> quite similar to the idea mentioned earlier that could be used for 
>>>> x86.
>>>>
>>>> Just get rid of this explicit KVM_REQ_MMU_RELOAD knowledge in
>>>> arch-independent code please (if you want to see this merged).
>>>>         
>>> I agree to lift the wait part to other archs later if needed, but 
>>> as  mentioned above I could move this to arch code to the cost of 
>>> one arch  hook more. But as also mentioned it doesn't really hurt. I 
>>> agree that it  does not need to be KVM_REQ_MMU_RELOAD specific, we 
>>> could just  walk/clear/wake all bits on that vcpu->requests variable.
>>> Would that be generic enough in your opinion ?
>>>     
>>
>> Don't know.
>>
>> Avi?
>>   
>
> I think I lost the thread here, but I'll try.  Isn't the wake part 
> make_all_vcpus_request() in kvm_main.c?  The wait part could be moved 
> to a similar generic function.
>
I'll try to summarize my current thoughts a bit:
The rebased patch series brings several fixes and the wait/wakeup 
mechanism which is in discussion here.
As explained before this keeps the new wait implementation in s390 arch 
code which allows us to experiment with it. Later if we are happy with 
it we might (or not) continue the merge and bring this mechanism to 
make_all_vcpus_request (as on x86 you don't have the issues I try to fix 
here we don't need to hurry bringing that into generic code).

Well now to the wait/wakeup which is here in discussion in detail:
The s390 arch code can kick a guest, but we don't know implicitly (as 
x86 does) that the kick succeeded, it might happen somewhen sooner or later.
Therefore the code uses wait_on_bit to wait until the vcpu->request bit 
is consumed.
To ensure cleanup of these waiting threads in some special cases the 
clear&wake up is also needed at other places than the real bit 
consumption. One of them is the vcpu release code where we should 
clear&wakeup all waiters (Marcelo correctly pointed out that we should 
not be bit specific there, so I just just wake up all in the updated code).

That was the discussion here: "if it would be ok to clear & wake up 
all". As wake_up_bit doesn't hurt if there is no waiter it looks like 
the best solution to to do that in the generic part of vcpu_release. If 
ever someone else waits for this or another bit in vcpu->requests, the 
code ensures all of them are awaken on vcpu release.

I send an updated version of the rebased series in a few minutes, 
containing updates related to what marcelo pointed out.

P.S. in case you think we need much more discussions we might try to 
catch up on irc to save this thread a few cycles :-)

-- 

Grüsse / regards, Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization