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:46:54 +0200 Message-ID: <4A082C4E.60501@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> <4A08217E.6000102@linux.vnet.ibm.com> <4A0824EF.5010201@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 mtagate3.uk.ibm.com ([195.212.29.136]:57065 "EHLO mtagate3.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751284AbZEKNq6 (ORCPT ); Mon, 11 May 2009 09:46:58 -0400 Received: from d06nrmr1707.portsmouth.uk.ibm.com (d06nrmr1707.portsmouth.uk.ibm.com [9.149.39.225]) by mtagate3.uk.ibm.com (8.14.3/8.13.8) with ESMTP id n4BDkwm3517944 for ; Mon, 11 May 2009 13:46:58 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1707.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n4BDkwP41843320 for ; Mon, 11 May 2009 14:46:58 +0100 Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n4BDkwb6023833 for ; Mon, 11 May 2009 14:46:58 +0100 In-Reply-To: <4A0824EF.5010201@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > Christian Ehrhardt wrote: >> >>> On x86, we use slots_lock to protect memory slots. When we change=20 >>> the global memory configuration, we set a bit in vcpu->requests, an= d=20 >>> send an IPI to all cpus that are currently in guest mode for our=20 >>> guest. This forces the cpu back to host mode. On the next entry,=20 >>> vcpu_run 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=20 >> but it really looks like overkill for s390. > > We could make (some of) it common code, so it won't look so bad. =20 > There's value in having all kvm ports do things similarly; though of=20 > course we shouldn't force the solution when it isn't really needed. > > vcpu->requests is useful whenever we modify global VM state that need= s=20 > to be seen by all vcpus in host mode; see kvm_reload_remote_mmus(). yeah I read that code after your first hint in that thread, and I agree= =20 that merging some of this into common code might be good. But in my opinion not now for this bugfix patch (the intention is just=20 to prevent a user being able to crash the host via vcpu create,set mem&= =20 and vcpu run in that order). It might be a good point to further streamline this once we use the sam= e=20 userspace code, but I think it doesn't make sense yet. > >> At least today we can assume that we only have one memslot. Therefor= e=20 >> a set_memslot with already created vcpu's will still not interfere=20 >> with running vcpus (they can't run without memslot and since we have= =20 >> only one they won't run). >> Anyway I the code is prepared to "meet" running vcpus, because it=20 >> might be different in future. To prevent the livelock issue I change= d=20 >> the code using mutex_trylock and in case I can't get the lock I=20 >> explicitly let the vcpu exit from guest. > > Why not do it unconditionally? > hmm I might have written that misleading - eventually it's a loop until= =20 it got the lock while !trylock kick vcpu out of guest schedule There is no reason to kick out guests where I got the lock cleanly as=20 far as I see. Especially as I expect the vcpus not running in the common case as i=20 explained above (can't run without memslot + we only have one =3D> no v= cpu=20 will run). --=20 Gr=C3=BCsse / regards,=20 Christian Ehrhardt IBM Linux Technology Center, Open Virtualization