Re: [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS

[Avi Kivity <avi@redhat.com>]

  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<avi@redhat.com>
>> ---
>> 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.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.