From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 3/3] kvm-s390: streamline memslot handling Date: Sun, 24 May 2009 17:39:10 +0300 Message-ID: <4A195C0E.3000900@redhat.com> References: <1242826497-6797-1-git-send-email-ehrhardt@linux.vnet.ibm.com> <1242826497-6797-4-git-send-email-ehrhardt@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, borntraeger@de.ibm.com, cotte@de.ibm.com, heiko.carstens@de.ibm.com, schwidefsky@de.ibm.com, Marcelo Tosatti To: ehrhardt@linux.vnet.ibm.com Return-path: Received: from mx2.redhat.com ([66.187.237.31]:44423 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751907AbZEXOlQ (ORCPT ); Sun, 24 May 2009 10:41:16 -0400 In-Reply-To: <1242826497-6797-4-git-send-email-ehrhardt@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: ehrhardt@linux.vnet.ibm.com wrote: > From: Christian Ehrhardt > > This patch relocates the variables kvm-s390 uses to track guest mem addr/size. > As discussed dropping the variables at struct kvm_arch level allows to use the > common vcpu->request based mechanism to reload guest memory if e.g. changes > via set_memory_region. > The kick mechanism introduced in this series is used to ensure running vcpus > leave guest state to catch the update. > > > > > rerun_vcpu: > + if (vcpu->requests) > + if (test_and_clear_bit(KVM_REQ_MMU_RELOAD, &vcpu->requests)) > + kvm_s390_vcpu_set_mem(vcpu); > + > /* verify, that memory has been registered */ > - if (!vcpu->kvm->arch.guest_memsize) { > + if (!vcpu->arch.sie_block->gmslm) { > vcpu_put(vcpu); > + VCPU_EVENT(vcpu, 3, "%s", "no memory registered to run vcpu"); > return -EINVAL; > } x86 uses a double check: first we check vcpu->requests outside atomic context, then we enter the critical section and check again for signals and vcpu->requests. This allows us (a) to do naughty things in vcpu->requests handlers, (b) keep the critical section short. Does this apply here? > - /* update sie control blocks, and unlock all vcpus */ > + /* request update of sie control block for all available vcpus */ > for (i = 0; i < KVM_MAX_VCPUS; ++i) { > if (kvm->vcpus[i]) { > - kvm->vcpus[i]->arch.sie_block->gmsor = > - kvm->arch.guest_origin; > - kvm->vcpus[i]->arch.sie_block->gmslm = > - kvm->arch.guest_memsize + > - kvm->arch.guest_origin + > - VIRTIODESCSPACE - 1ul; > - mutex_unlock(&kvm->vcpus[i]->mutex); > + set_bit(KVM_REQ_MMU_RELOAD, &kvm->vcpus[i]->requests); > + kvm_s390_inject_sigp_stop(kvm->vcpus[i], > + ACTION_RELOADVCPU_ON_STOP); > } > } > There already exists a loop which does this, see make_all_cpus_request(). It uses an IPI (Marcelo, can't it use the reschedule interrupt?). It has a couple of optimizations -- if the request is already set, it skips the IPI, and it avoids the IPI for vcpus out of guest mode. Maybe it could fit s390 too. -- error compiling committee.c: too many arguments to function