From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Subject: Re: [PATCH 1/6] kvm-s390: Fix memory slot versus run Date: Mon, 11 May 2009 15:00:46 +0200 Message-ID: <4A08217E.6000102@linux.vnet.ibm.com> References: <1241534358-32172-1-git-send-email-ehrhardt@linux.vnet.ibm.com> <1241534358-32172-2-git-send-email-ehrhardt@linux.vnet.ibm.com> <4A017C2E.6060306@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, Christian Borntraeger , Carsten Otte To: Avi Kivity Return-path: Received: from mtagate7.uk.ibm.com ([195.212.29.140]:60401 "EHLO mtagate7.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752288AbZEKNAw (ORCPT ); Mon, 11 May 2009 09:00:52 -0400 Received: from d06nrmr1806.portsmouth.uk.ibm.com (d06nrmr1806.portsmouth.uk.ibm.com [9.149.39.193]) by mtagate7.uk.ibm.com (8.14.3/8.13.8) with ESMTP id n4BD0oQJ402672 for ; Mon, 11 May 2009 13:00:50 GMT Received: from d06av04.portsmouth.uk.ibm.com (d06av04.portsmouth.uk.ibm.com [9.149.37.216]) by d06nrmr1806.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n4BD0oA03063834 for ; Mon, 11 May 2009 14:00:50 +0100 Received: from d06av04.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av04.portsmouth.uk.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n4BD0nnV013052 for ; Mon, 11 May 2009 14:00:50 +0100 In-Reply-To: <4A017C2E.6060306@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > ehrhardt@linux.vnet.ibm.com wrote: >> From: Carsten Otte >> >> This patch fixes an incorrectness in the kvm backend for s390. >> In case virtual cpus are being created before the corresponding >> memory slot is being registered, we need to update the sie >> control blocks for the virtual cpus. In order to do that, we >> use the vcpu->mutex to lock out kvm_run and friends. This way >> we can ensure a consistent update of the memory for the entire >> smp configuration. >> @@ -657,6 +657,8 @@ int kvm_arch_set_memory_region(struct kv >> struct kvm_memory_slot old, >> int user_alloc) >> { >> + int i; >> + >> /* A few sanity checks. We can have exactly one memory slot=20 >> which has >> to start at guest virtual zero and which has to be located a= t a >> page boundary in userland and which has to end at a page=20 >> boundary. >> @@ -676,13 +678,27 @@ int kvm_arch_set_memory_region(struct kv >> if (mem->memory_size & (PAGE_SIZE - 1)) >> return -EINVAL; >> =20 >> + /* lock all vcpus */ >> + for (i =3D 0; i < KVM_MAX_VCPUS; ++i) { >> + if (kvm->vcpus[i]) >> + mutex_lock(&kvm->vcpus[i]->mutex); >> + } >> + >> =20 > > Can't that livelock? Nothing requires a vcpu to ever exit, and if th= e=20 > cpu on which it's running on has no other load and no interrupts, it=20 > could remain in guest mode indefinitely, and then the ioctl will hang= ,=20 > waiting for something to happen. > Yes it could wait indefinitely - good spot. > On x86, we use slots_lock to protect memory slots. When we change th= e=20 > global memory configuration, we set a bit in vcpu->requests, and send= =20 > an IPI to all cpus that are currently in guest mode for our guest. =20 > This forces the cpu back to host mode. On the next entry, vcpu_run=20 > notices vcpu->requests has the bit set and reloads the mmu=20 > configuration. Of course, all this may be overkill for s390. > I thought about implementing it with slots_lock, vcpu->request, etc but= =20 it really looks like overkill for s390. At least today we can assume that we only have one memslot. Therefore a= =20 set_memslot with already created vcpu's will still not interfere with=20 running vcpus (they can't run without memslot and since we have only on= e=20 they won't run). Anyway I the code is prepared to "meet" running vcpus, because it might= =20 be different in future. To prevent the livelock issue I changed the cod= e=20 using mutex_trylock and in case I can't get the lock I explicitly let=20 the vcpu exit from guest. --=20 Gr=C3=BCsse / regards,=20 Christian Ehrhardt IBM Linux Technology Center, Open Virtualization