From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hollis Blanchard Subject: Re: [kvm-ppc-devel] [PATCH] Split kvm_vcpu to support new archs. Date: Thu, 18 Oct 2007 16:43:18 -0500 Message-ID: <1192743798.21205.30.camel@basalt> 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> Reply-To: Hollis Blanchard 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: Anthony Liguori Return-path: In-Reply-To: <4717D095.40708-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org> 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 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; } > 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. -- Hollis Blanchard IBM Linux Technology Center ------------------------------------------------------------------------- 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/