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: Fri, 19 Oct 2007 12:16:40 -0500 Message-ID: <1192814200.10451.18.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> <1192743798.21205.30.camel@basalt> <4717D87E.5010000@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 , Carsten Otte , "Zhang, Xiantao" To: Anthony Liguori Return-path: In-Reply-To: <4717D87E.5010000-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 17:04 -0500, Anthony Liguori wrote: > Hollis Blanchard wrote: > > > > 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). Ah, so you're saying that once we have a "to_x86()" macro, the nesting becomes irrelevant. Furthermore, "to_x86()" won't even be defined when compiling shared code, which will ensure that nobody tries to slip something in. Finally, it means we don't have to keep the "arch" member at the end of kvm_vcpu (though I don't think that's a big deal really). I think I can live with that. Carsten? [BTW, a quick glance at svm.c turns up weirdness like this: static int rdmsr_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run) { u32 ecx = svm->vcpu.regs[VCPU_REGS_RCX]; u64 data; if (svm_get_msr(&svm->vcpu, ecx, &data)) svm_inject_gp(&svm->vcpu, 0); ... } static void svm_inject_gp(struct kvm_vcpu *vcpu, unsigned error_code) { struct vcpu_svm *svm = to_svm(vcpu); ... } There's some unnecessary kvm_vcpu/svm_vcpu conversion going on there.] -- 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/