From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [kvm-ppc-devel] [PATCH] Split kvm_vcpu to support new archs. Date: Thu, 18 Oct 2007 17:04:46 -0500 Message-ID: <4717D87E.5010000@codemonkey.ws> References: <42DFA526FC41B1429CE7279EF83C6BDC809A6A@pdsmsx415.ccr.corp.intel.com> <1192737702.21205.17.camel@basalt> <4717CA4B.7040307@codemonkey.ws> <1192742084.21205.22.camel@basalt> <4717D095.40708@codemonkey.ws> <1192743798.21205.30.camel@basalt> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, kvm-ppc-devel , "Zhang, Xiantao" To: Hollis Blanchard Return-path: In-Reply-To: <1192743798.21205.30.camel@basalt> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Hollis Blanchard wrote: > On Thu, 2007-10-18 at 16:31 -0500, Anthony Liguori wrote: > >> Hollis Blanchard wrote: >> >>> On Thu, 2007-10-18 at 16:04 -0500, Anthony Liguori wrote: >>> >>> >>>> Hollis Blanchard wrote: >>>> >>>> >>>>> What is the problem with >>>>> embedding an architecture-specific sub-structure, i.e. >>>>> struct kvm_vcpu { >>>>> ... >>>>> struct arch_kvm_vcpu arch_vcpu; >>>>> }; >>>>> >>>>> >>>>> >>>> I think you want the opposite direction of nesting. >>>> >>>> >>> ... >>> >>> >>>> We should move to: >>>> >>>> struct kvm_vcpu { >>>> /* stuff common to x86/ppc/ia64 */ >>>> }; >>>> >>>> struct vcpu_x86 { >>>> struct kvm_vcpu vcpu; >>>> /* stuff common to vt/svm */ >>>> } >>>> >>>> struct vcpu_svm { >>>> struct vcpu_x86 vcpu; >>>> /* svm specific stuff */ >>>> }; >>>> >>>> >>> Why? >>> >>> >> It provides better encapsulation. If you have a kvm_vcpu, unless you do >> container_of(), you can't access the arch_vcpu. It helps make sure that >> architecture common code remains common. >> > > I must be misunderstanding, because this seems completely backwards to > me. With your nesting, any time architecture code wants to access > architecture state (which is almost all the time), you'd *need* > container_of: > > void arch_func(struct kvm_vcpu *vcpu) { > struct arch_vcpu *arch = container_of(vcpu, arch_vcpu, > arch); > arch->gpr[3] = 0; > } > > In contrast, my nesting proposal would look like this: > > void arch_func(struct kvm_vcpu *vcpu) { > vcpu->arch.gpr[3] = 0; > } > Well, you'd probably define a to_ppc() and then do something like: void arch_func(struct kvm_vcpu *vcpu) { to_arch(vcpu)->gpr[3] = 0; } Which is exactly what's done in the vt/svm backend (see usage of to_svm/to_vmx). Regards, Anthony Liguori >> It also leaves open the possibility of supporting multiple architectures >> at the same time. I don't know why you would want to do that :-) >> > > That's true, though this could also be accomplished by keeping arch_vcpu > as the last member of kvm_vcpu. > > ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/