From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH] V2 Handle guest access to BBL_CR_CTL3 MSR Date: Tue, 1 Feb 2011 05:50:58 -0200 Message-ID: <20110201075058.GA4648@amt.cnet> References: <4D3917BC.3040502@redhat.com> <20110131200603.GA28167@amt.cnet> <4D472024.1000807@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: KVM list , Avi Kivity To: john cooper Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33618 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754105Ab1BAKKA (ORCPT ); Tue, 1 Feb 2011 05:10:00 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id p11A9xmQ021226 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 1 Feb 2011 05:10:00 -0500 Content-Disposition: inline In-Reply-To: <4D472024.1000807@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jan 31, 2011 at 03:48:36PM -0500, john cooper wrote: > Marcelo Tosatti wrote: > > On Fri, Jan 21, 2011 at 12:21:00AM -0500, john cooper wrote: > >> [Resubmit of prior version which contained a wayward > >> patch hunk. Thanks Marcelo] > >> > >> A correction to Intel cpu model CPUID data (patch queued) > >> caused winxp to BSOD when booted with a Penryn model. > >> This was traced to the CPUID "model" field correction from > >> 6 -> 23 (as is proper for a Penryn class of cpu). Only in > >> this case does the problem surface. > >> > >> The cause for this failure is winxp accessing the BBL_CR_CTL3 > >> MSR which is unsupported by current kvm, appears to be a > >> legacy MSR not fully characterized yet existing in current > >> silicon, and is apparently carried forward in MSR space to > >> accommodate vintage code as here. It is not yet conclusive > >> whether this MSR implements any of its legacy functionality > >> or is just an ornamental dud for compatibility. While I > >> found no silicon version specific documentation link to > >> this MSR, a general description exists in Intel's developer's > >> reference which agrees with the functional behavior of > >> other bootloader/kernel code I've examined accessing > >> BBL_CR_CTL3. Regrettably winxp appears to be setting bit #19 > >> called out as "reserved" in the above document. > >> > >> So to minimally accommodate this MSR, kvm msr get will provide > >> the equivalent mock data and kvm msr write will simply toss the > >> guest passed data without interpretation. While this treatment > >> of BBL_CR_CTL3 addresses the immediate problem, the approach may > >> be modified pending clarification from Intel. > >> > >> Signed-off-by: john cooper > >> --- > >> > >> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h > >> index 4d0dfa0..5bfafb6 100644 > >> --- a/arch/x86/include/asm/msr-index.h > >> +++ b/arch/x86/include/asm/msr-index.h > >> @@ -38,6 +38,7 @@ > >> > >> #define MSR_MTRRcap 0x000000fe > >> #define MSR_IA32_BBL_CR_CTL 0x00000119 > >> +#define MSR_IA32_BBL_CR_CTL3 0x0000011e > >> > >> #define MSR_IA32_SYSENTER_CS 0x00000174 > >> #define MSR_IA32_SYSENTER_ESP 0x00000175 > >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > >> index bcc0efc..04d6c55 100644 > >> --- a/arch/x86/kvm/x86.c > >> +++ b/arch/x86/kvm/x86.c > >> @@ -1592,6 +1592,12 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data) > >> } else > >> return set_msr_hyperv(vcpu, msr, data); > >> break; > >> + case MSR_IA32_BBL_CR_CTL3: > >> + /* Drop writes to this legacy MSR -- see rdmsr > >> + * counterpart for further detail. > >> + */ > >> + pr_unimpl(vcpu, "ignored wrmsr: 0x%x data %llx\n", msr, data); > >> + break; > >> default: > >> if (msr && (msr == vcpu->kvm->arch.xen_hvm_config.msr)) > >> return xen_hvm_config(vcpu, data); > >> @@ -1846,6 +1852,19 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata) > >> } else > >> return get_msr_hyperv(vcpu, msr, pdata); > >> break; > >> + case MSR_IA32_BBL_CR_CTL3: > >> + /* This legacy MSR exists but isn't fully documented in current > >> + * silicon. It is however accessed by winxp in very narrow > >> + * scenarios where it sets bit #19, itself documented as > >> + * a "reserved" bit. Best effort attempt to source coherent > >> + * read data here should the balance of the register be > >> + * interpreted by the guest: > >> + * > >> + * L2 cache control register 3: 64GB range, 256KB size, > >> + * enabled, latency 0x1, configured > >> + */ > >> + data = 0xbe702111; > >> + break; > > > > Why bits 26-29 and 31 enabled? > > That was a judgment call. Intel's documentation for msr 0x11e > defines them as reserved bits, however they are returned as 0xbe > for the Penryn class processors I'd checked. Not exhaustive, yet > bit #19 at the root of the problem is also documented as reserved. > I was attempting to err in favor of actual deployed silicon. > > Thanks, > > -john OK. Applied, thanks.