On 07/31/2018 01:04 PM, David Hildenbrand wrote:
> 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.
Hmm, shall we change that so that kvm_s390_vcpu_block will also block
vsie from entering?
>
>
>> memset(&kvm->arch.crypto.crycb->apcb0, 0,
>> sizeof(kvm->arch.crypto.crycb->apcb0));
>> memset(&kvm->arch.crypto.crycb->apcb1, 0,
I am asking myself if if would be valid to first memset the crycb and then do a kick.
On reentry sie will then use the new value. I will double check.
