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: Mon, 25 Jun 2012 16:12:53 +0300 Message-ID: <20120625131253.GA16583@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> <20120624142753.GX6533@redhat.com> <4FE86046.6090702@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]:34257 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756584Ab2FYNMz (ORCPT ); Mon, 25 Jun 2012 09:12:55 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5PDCtMS015551 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 25 Jun 2012 09:12:55 -0400 Content-Disposition: inline In-Reply-To: <4FE86046.6090702@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jun 25, 2012 at 03:57:42PM +0300, Avi Kivity wrote: > On 06/24/2012 05:27 PM, Gleb Natapov wrote: > > 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. > > Right now the all segment related operations are behind the line; the > line is linear | physical. Having a ->linearize op will change that. > > > > >> 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. > > Right. But I think we can have x86_linearize() that doesn't take a > context parameter, only ops. > All ops take context parameter though. -- Gleb.