From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCHv2 4/5] KVM: emulator: move linearize() out of emulator code. Date: Sun, 24 Jun 2012 17:27:53 +0300 Message-ID: <20120624142753.GX6533@redhat.com> References: <1339502487-30049-1-git-send-email-gleb@redhat.com> <1339502487-30049-5-git-send-email-gleb@redhat.com> <4FE71225.2010504@redhat.com> <20120624132710.GW6533@redhat.com> <4FE7188A.2080500@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, mtosatti@redhat.com To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:1025 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753383Ab2FXO1z (ORCPT ); Sun, 24 Jun 2012 10:27:55 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5OERsdt018736 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 24 Jun 2012 10:27:54 -0400 Content-Disposition: inline In-Reply-To: <4FE7188A.2080500@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Sun, Jun 24, 2012 at 04:39:22PM +0300, Avi Kivity wrote: > On 06/24/2012 04:27 PM, Gleb Natapov wrote: > > On Sun, Jun 24, 2012 at 04:12:05PM +0300, Avi Kivity wrote: > >> On 06/12/2012 03:01 PM, Gleb Natapov wrote: > >> > The function will be used outside of the emulator. > >> > > >> > /* > >> > * x86_emulate_ops: > >> > * > >> > @@ -194,6 +199,10 @@ struct x86_emulate_ops { > >> > > >> > bool (*get_cpuid)(struct x86_emulate_ctxt *ctxt, > >> > u32 *eax, u32 *ebx, u32 *ecx, u32 *edx); > >> > + > >> > + int (*linearize)(struct x86_emulate_ctxt *ctxt, > >> > + struct segmented_address addr, unsigned size, > >> > + bool write, bool fetch, ulong *linear); > >> > }; > >> > > >> > >> linearize is defined in terms of the other ops; this means that if we > >> get a second user they will have to replicate it. > >> > > What do you mean? This patch series adds another user, so now there are two: one > > inside the emulator another is outside. > > I meant like task switching or real-mode interrupt emulation. > You mean code outside of KVM if we ever will make emulator reusable? It will have to have its own, much more simple version of the callback. > > > >> Why not make the current linearize available to users? > >> > > Code outside of the emulator does not call the emulator except when > > emulation is actually needed. To call linearize() from the emulator.c > > almost fully functional emulation ctxt will have to be set up (including > > fake instruction decoding, hacky and slower). > > ctxt->d use should be removed for the exported version and replaced by a > parameter. The internal version can still use it (calling the exported > version after extracting the parameter). > IMO we should stick to the pattern we have now: calling generic code from the emulator and not vice versa. Lets not create more spaghetti. > To not duplicate the logic > > I moved linearize() to generic code and made it available to emulator > > via callback. It actually saves a couple of callback invocations when > > emulator calls linearize() IIRC. > > It's not available to other emulator users (which don't exist yet > anyway). But having linearize() in the emulator is consistent with > placing logic in emulate.c and accessors outside. > It is the question of where we draw the line. For instance MMU details are now hidden from the emulator behind a callback. One can argue that emulator should have access to MMU directly via callbacks and emulate memory access by itself. > Regarding initialization, we should eventually initialize nothing and > let the emulator bring in needed data via callbacks (including general > registers). > Some things will have to be initialized (or rather reset to initial value) between emulator invocations. Access to registers can be done on demand, but this is unrelated to this series optimization. -- Gleb.