From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS Date: Wed, 1 Sep 2010 10:56:26 +0200 Message-ID: References: <1283326497-20206-1-git-send-email-avi@redhat.com> <1283326497-20206-3-git-send-email-avi@redhat.com> <0DFA763B-EBFF-4913-8BD0-675C4247994B@suse.de> <4C7E14F2.7040908@redhat.com> Mime-Version: 1.0 (Apple Message framework v1081) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8BIT Cc: Marcelo Tosatti , KVM list To: Avi Kivity Return-path: Received: from cantor2.suse.de ([195.135.220.15]:37942 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752077Ab0IAI42 convert rfc822-to-8bit (ORCPT ); Wed, 1 Sep 2010 04:56:28 -0400 In-Reply-To: <4C7E14F2.7040908@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01.09.2010, at 10:55, Avi Kivity wrote: > On 09/01/2010 11:49 AM, Alexander Graf wrote: >> On 01.09.2010, at 09:34, Avi Kivity wrote: >> >>> The returned value is completely bogus, and sets reserved bits. >>> Return zero instead. >>> >>> Signed-off-by: Avi Kivity >>> --- >>> arch/x86/kvm/x86.c | 5 +---- >>> 1 files changed, 1 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 1cbf168..a2c03f1 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -1641,10 +1641,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata) >>> data = vcpu->arch.ia32_misc_enable_msr; >>> break; >>> case MSR_IA32_PERF_STATUS: >>> - /* TSC increment by tick */ >>> - data = 1000ULL; >>> - /* CPU multiplier */ >>> - data |= (((uint64_t)4ULL)<< 40); >>> + data = 0; >>> break; >>> case MSR_EFER: >>> data = vcpu->arch.efer; >> >> This is the respective code snippet from xnu: >> >> /* >> * Get the TSC increment. The TSC is incremented by this >> * on every bus tick. Calculate the TSC conversion factors >> * to and from nano-seconds. >> */ >> if (cpuid_info()->cpuid_family == CPU_FAMILY_PENTIUM_M) { >> uint64_t prfsts; >> >> prfsts = rdmsr64(IA32_PERF_STS); >> tscGranularity = (uint32_t)bitfield(prfsts, 44, 40); >> N_by_2_bus_ratio = prfsts& bit(46); >> >> } else { >> panic("rtclock_init: unknown CPU family: 0x%X\n", >> cpuid_info()->cpuid_family); >> } >> >> if (N_by_2_bus_ratio) >> tscFCvtt2n = busFCvtt2n * 2 / (uint64_t)tscGranularity; >> else >> tscFCvtt2n = busFCvtt2n / (uint64_t)tscGranularity; >> >> tscFreq = ((1 * Giga)<< 32) / tscFCvtt2n; >> tscFCvtn2t = 0xFFFFFFFFFFFFFFFFULL / tscFCvtt2n; >> >> >> So by passing in 0 here, you effectively make that code divide something by 0 which results in a panic. >> >> > > Right. Searching again I find that this is actually documented, (under MSR_PERF_STATU > s instead of IA32_PERF_STATUS). I'll just drop this patch. Sounds good. The other one seems sane to me. I don't have a working setup for Mac guests atm. That was on the TODO list once the AHCI stuff gets upstream :). Alex