From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH v3] KVM: x86 emulator: access GPRs on demand Date: Fri, 17 Aug 2012 14:29:38 -0300 Message-ID: <20120817172938.GA21708@amt.cnet> References: <1345128889-29672-1-git-send-email-avi@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:45972 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753195Ab2HQRqH (ORCPT ); Fri, 17 Aug 2012 13:46:07 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7HHk7Ds013443 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 17 Aug 2012 13:46:07 -0400 Content-Disposition: inline In-Reply-To: <1345128889-29672-1-git-send-email-avi@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Aug 16, 2012 at 05:54:49PM +0300, Avi Kivity wrote: > Instead of populating the the entire register file, read in registers > as they are accessed, and write back only the modified ones. This > saves a VMREAD and VMWRITE on Intel (for rsp, since it is not usually > used during emulation), and a two 128-byte copies for the registers. > > Signed-off-by: Avi Kivity > --- > > v3: > fix misplaced parentheses in em_loop() and em_jcxz(), unbreaking those instructions. > > v2: > add APIs for managing the register cache. This reduces the potential for confusion > between ctxt->regs_dirty and vcpu->arch.regs_dirty. > move cache management to the entry points > add missing writebacks to int and task switch emulation > > > arch/x86/include/asm/kvm_emulate.h | 20 ++- > arch/x86/kvm/emulate.c | 305 ++++++++++++++++++++++--------------- > arch/x86/kvm/x86.c | 45 +++--- > 3 files changed, 223 insertions(+), 147 deletions(-) > > @@ -2715,14 +2764,17 @@ int emulator_task_switch(struct x86_emulate_ctxt *ctxt, > { > int rc; > > + invalidate_registers(ctxt); > ctxt->_eip = ctxt->eip; > ctxt->dst.type = OP_NONE; > > rc = emulator_do_task_switch(ctxt, tss_selector, idt_index, reason, > has_error_code, error_code); > > - if (rc == X86EMUL_CONTINUE) > + if (rc == X86EMUL_CONTINUE) { > ctxt->eip = ctxt->_eip; > + writeback_registers(ctxt); > + } > > return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK; > } No clear point when emulator register cache is active, when it is not (AFAICS this patch does not invalidate registers on emulation start (the above being one of the exceptions) does not clear valid bit on writeback-to-vcpu-cache on emulation exit). Concern is that emulator can start with cached registers marked as valid but in fact are invalid from previous emulation round. Maybe move invalidate() to init_emulate_ctxt?