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 -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html