From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvWiJ-0007a8-5o for qemu-devel@nongnu.org; Wed, 16 Jan 2013 12:23:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TvWiH-0006aR-Pd for qemu-devel@nongnu.org; Wed, 16 Jan 2013 12:23:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51266) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvWiH-0006aK-Hq for qemu-devel@nongnu.org; Wed, 16 Jan 2013 12:23:45 -0500 Date: Wed, 16 Jan 2013 15:23:36 -0200 From: Marcelo Tosatti Message-ID: <20130116172336.GA22096@amt.cnet> References: <1357831744-3950-1-git-send-email-jjherne@us.ibm.com> <20130116160533.GA8541@amt.cnet> <6A3DF150A5B70D4F9B66A25E3F7C888D06564C9A@039-SN2MPN1-023.039d.mgd.msft.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D06564C9A@039-SN2MPN1-023.039d.mgd.msft.net> Subject: Re: [Qemu-devel] [PATCH 4/7 v2] KVM regsync: Add register bitmap parameter to do_kvm_cpu_synchronize_state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bhushan Bharat-R65777 Cc: "agraf@suse.de" , "aliguori@us.ibm.com" , "jan.kiszka@siemens.com" , "qemu-devel@nongnu.org" , "Jason J. Herne" , "borntraeger@de.ibm.com" On Wed, Jan 16, 2013 at 05:00:52PM +0000, Bhushan Bharat-R65777 wrote: > I think above code should be: > kvm_arch_put_registers(cpu, cpu->kvm_vcpu_dirty); > cpu->kvm_vcpu_dirty = false; > > so vcpu will not enter guest state with dirty registers in qemu. Not so clear - currently PUT_FULL/PUT_RESET are performed on pre-defined points. > > Unrelated: > > > > 2) Also, what is the reason for specifying sets of registers in arch-specific > > code? Is that because it allows PPC to fix their sync-timer register problem? > > > > When you are writing generic code, what does it mean to use > > 'KVM_REGSYNC_{RUNTIME,RESET,FULL}_STATE' ? > > Answer: it depends on the architecture. > > > > 3) On x86, kvm_arch_get_registers(GET_FULL) must not imply > > kvm_arch_put_registers(PUT_FULL). > > > > The S/390 problem, from > > http://lists.nongnu.org/archive/html/qemu-devel/2012-11/msg02213.html: > > > > ">>> The kvm register sync needs to happen in the kvm register sync > > >>> function :) > > >> That would eliminate the whole purpose of sync regs and forces us to > > >> have an expensive ioctl on lots of exits (again). I would prefer to > > >> sync the registers that we never need in qemu just here. > > > > > > That's why the register sync has different stages. > > > > Not the get_register. Which is called on every synchronize_state. Which happen > > quite often on s390." > > > > But wait: on these S/390 codepaths, you do GET_REGS already, via > > cpu_synchronize_state. > > > > So on S/390 > > > > - cpu_synchronize_state(env) > > - read any register from env > > > > Is not valid? This is what generic code assumes. > > > > > > Bhushan Bharat, the PPC problem, can you describe it clearly: from what i > > understood, an in-kernel register cannot be read/written back because that > > register value can change in the meantime. When is it necessary to write it > > back? (there is a similar problem with TSC on x86, which is "fixed" by only > > writing TSC on FULL_STATE arch_put_registers). > > There are two things: > > First-) > For timer related changes on PowerPC, some registers needed to be changed from QEMU, so we have to get the registers via KVM_GET_SREGS and then set those registers back to KVM via KVM_SET_SREGS. cpu_synchronize_state() will get registers but kvm_arch_put_registers() works on level based mechanism and does not provide a good way of setting a register-set. So we wrote a separate function that will push these registers back to KVM and this also uses KVM_SET_SREGS ioctl. This solves what is needed for PPC. Can you describe the problem in detail? You must sync a particular timer register only on special conditions, not during normal cpu_synchronize_state() runs? What register is that and why it cannot be synced normally? When is it necessary to sync it? > Second-) > Currently kvm_arch_get_registers() is not optimized in two sense; one, it always get all registers from KVM; two, in kvm_arch_get_registers() it copies all registers to env->. This patch-set handles the second issue of optimization, copy only the requested registers to env-> in kvm_arch_get_registers(), plus when kvm_arch_put_registers() is called then it copies only the modified registers for KVM_SET_SREGS. > > This optimization is looking good to me and allows sync of registers via one common kvm_arch_get/set_registers() and no separate function definition for setting is needed for timer related changes. The problem with S390 is not clear to me (besides cpu_synchronize_state must sync all state as mentioned). > > Thanks > -Bharat >