On 31.07.2018 10:56, Pierre Morel wrote:
> On 31/07/2018 10:36, David Hildenbrand wrote:
>> On 31.07.2018 10:34, Pierre Morel wrote:
>>> On 30/07/2018 16:40, Halil Pasic wrote:
>>>>
>>>> On 07/30/2018 01:15 PM, Pierre Morel wrote:
>>>>> On 30/07/2018 11:24, David Hildenbrand wrote:
>>>>>> On 26.07.2018 21:54, Christian Borntraeger wrote:
>>>>>>> From: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
>>>>>>>
>>>>>>> Introduces two new KVM interface to lear the APM, AQM and ADM masks in
>>>>>>> the guest's CRYCB. The VCPUs are taken out of SIE to ensure the
>>>>>>> VCPUs do
>>>>>>> not get out of synch.
>>>>>> s/synch/sync/
>>>>>>
>>>>>> When will this be called and why?
>>>>>>
>>>>>> If I read correctly, this can happen while other VCPUs are running
>>>>>> (currently in the SIE). Please note that
>>>>>> kvm_s390_vcpu_block_all/kvm_s390_vcpu_unblock_all will not care about
>>>>>> vSIE. So a CPU inside the vSIE loop will not be hindered of executing
>>>>>> the SIE. (because so far, all VCPU requests we handle don't rely on
>>>>>> that)
>>>>>>
>>>>>> So it could happen here, that after this call a vSIE CPU still can
>>>>>> access some adapters if we allowed to forward some of them to the
>>>>>> nested
>>>>>> guest.
>>>>> You are right for the principle.
>>>>>
>>>>> However this function is only called when the mediated device is release
>>>>> which is, as we do not support hotplug, when the vfio-device is closed
>>>>> and the guest already disappeared.
>>>>>
>>>>> So I do not think it is useful the way it is currently used.
>>>>>
>>>>> What about just letting this call and function fall and take more
>>>>> time on this problem when we introduce hotplug?
>>>>>
>>>> The point is, I don't think we can prohibit userspace to close the
>>>> vfio device before the VM is teared down (hot-unplug). The idea
>>>> was, do the best we can to give up the resources. I was not aware
>>>> of the problems with the VSIE.
>>>>
>>>> QEMU however does make sure there is no hot-unplug. So no user
>>>> can observe the problem.
>>>>
>>>> Halil
>>>
>>> David,
>>> what about if I extend this function to handle
>>> the SIE when I introduce the SIE in patch 20
>>> by calling kvm_s390_vsie_kick() ?
>>>
>> SIE kick is not enough. The semantics of that are simply "please exit
>> the SIE to check for any events". It can immediately enter it again
>> reusing the old created crycb block. There would have to be some
>> additional check. But such stuff is usually prone to races.
>
> I intended to use it in extension of the vcpu_block() so that
> when the vsie() is exited the vcpu falls in the SIE which will be blocked.
>
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 0eecc9d..4937299 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1925,9 +1925,13 @@ static void kvm_s390_set_crycb_format(struct kvm
> *kvm)
>
> void kvm_arch_crypto_clear_masks(struct kvm *kvm)
> {
> - mutex_lock(&kvm->lock);
> - kvm_s390_vcpu_block_all(kvm);
> + int i;
>
> + mutex_lock(&kvm->lock);
> + kvm_for_each_vcpu(i, vcpu, kvm) {
> + kvm_s390_vcpu_block(vcpu);
> + kvm_s390_vsie_kick(vcpu);
> + }
No that does not work. The VSIE can be immediately reentered afther the
kick with the old crycb block.
> memset(&kvm->arch.crypto.crycb->apcb0, 0,
> sizeof(kvm->arch.crypto.crycb->apcb0));
> memset(&kvm->arch.crypto.crycb->apcb1, 0,
>
--
Thanks,
David / dhildenb
