From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: PATCH: nVMX: Better MSR_IA32_FEATURE_CONTROL handling Date: Thu, 15 Mar 2012 20:08:56 +0200 Message-ID: <4F623038.2080201@redhat.com> References: <20120307155846.GA17631@fermat.math.technion.ac.il> <4F578A53.20809@redhat.com> <20120315174014.GA21487@fermat.math.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Julian Stecklina , Alexander Graf , "Roedel, Joerg" To: "Nadav Har'El" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47943 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754822Ab2COSJX (ORCPT ); Thu, 15 Mar 2012 14:09:23 -0400 In-Reply-To: <20120315174014.GA21487@fermat.math.technion.ac.il> Sender: kvm-owner@vger.kernel.org List-ID: On 03/15/2012 07:40 PM, Nadav Har'El wrote: > On Wed, Mar 07, 2012, Avi Kivity wrote about "Re: PATCH: nVMX: Better MSR_IA32_FEATURE_CONTROL handling": > > > struct page *apic_access_page; > > > + u64 msr_ia32_feature_control; > > > }; > >... > > (msrs_to_save). The variable itself should live in vcpu->arch, even if > > some bits are vendor specific. > > Does this MSR exist in AMD? I was under the impression that it is an > Intel-only MSR, and that AMD has something different, the VM_CR MSR, > so it didn't make sense to put this in vcpu->arch. Is my impression > wrong? vmx.c is not about guest-side Intel features, it's about vmx specific implementation of x86 features. Whenever possible we put things in x86.c so if the other vendor picks up the feature, it will just work. Nested vmx is of course impossible to put in x86.c. This MSR is somewhat on the borderline, there's nothing vendor specific about the _implementation_, but it all the features it controls are very vendor specific. So I guess it can stay in vmx.c, if you can make it available for save/restore. > I seems, by the way, that svm.c has vm_cr_msr in svm->nested, basically the > same what I did, not in vcpu->arch. Why is this bad? The same reasoning applies. > Also, it seems that VM_CR is also not on the list on msrs_to_save. > A bug? Yes. -- error compiling committee.c: too many arguments to function