* [PATCH 0/2] Fix MSR_IA32_PERF_STATUS
@ 2010-09-01 7:34 Avi Kivity
2010-09-01 7:34 ` [PATCH 1/2] KVM: Don't save/restore MSR_IA32_PERF_STATUS Avi Kivity
2010-09-01 7:34 ` [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS Avi Kivity
0 siblings, 2 replies; 6+ messages in thread
From: Avi Kivity @ 2010-09-01 7:34 UTC (permalink / raw)
To: Marcelo Tosatti, kvm, Alexander Graf
MSR_IA32_PERF_STATUS is read-only, yet exposed for save/restore. This
generates annoying printk()s when userspace attempts to initialize it.
The value returned is also bogus, the comments appear to refer to another msr.
Fix both issues. Alex, please test with a guest Mac.
Avi Kivity (2):
KVM: Don't save/restore MSR_IA32_PERF_STATUS
KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS
arch/x86/kvm/x86.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] KVM: Don't save/restore MSR_IA32_PERF_STATUS
2010-09-01 7:34 [PATCH 0/2] Fix MSR_IA32_PERF_STATUS Avi Kivity
@ 2010-09-01 7:34 ` Avi Kivity
2010-09-01 7:34 ` [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS Avi Kivity
1 sibling, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2010-09-01 7:34 UTC (permalink / raw)
To: Marcelo Tosatti, kvm, Alexander Graf
It is read/only; restoring it only results in annoying messages.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
arch/x86/kvm/x86.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cc6d6cd..1cbf168 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -739,7 +739,7 @@ static u32 msrs_to_save[] = {
#ifdef CONFIG_X86_64
MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
#endif
- MSR_IA32_TSC, MSR_IA32_PERF_STATUS, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA
+ MSR_IA32_TSC, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA
};
static unsigned num_msrs_to_save;
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS
2010-09-01 7:34 [PATCH 0/2] Fix MSR_IA32_PERF_STATUS Avi Kivity
2010-09-01 7:34 ` [PATCH 1/2] KVM: Don't save/restore MSR_IA32_PERF_STATUS Avi Kivity
@ 2010-09-01 7:34 ` Avi Kivity
2010-09-01 8:49 ` Alexander Graf
1 sibling, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2010-09-01 7:34 UTC (permalink / raw)
To: Marcelo Tosatti, kvm, Alexander Graf
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;
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS
2010-09-01 7:34 ` [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS Avi Kivity
@ 2010-09-01 8:49 ` Alexander Graf
2010-09-01 8:55 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2010-09-01 8:49 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, Alexander Graf
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.
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS
2010-09-01 8:49 ` Alexander Graf
@ 2010-09-01 8:55 ` Avi Kivity
2010-09-01 8:56 ` Alexander Graf
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2010-09-01 8:55 UTC (permalink / raw)
To: Alexander Graf; +Cc: Marcelo Tosatti, kvm, Alexander Graf
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS
2010-09-01 8:55 ` Avi Kivity
@ 2010-09-01 8:56 ` Alexander Graf
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Graf @ 2010-09-01 8:56 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, KVM list
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<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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-01 8:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-01 7:34 [PATCH 0/2] Fix MSR_IA32_PERF_STATUS Avi Kivity
2010-09-01 7:34 ` [PATCH 1/2] KVM: Don't save/restore MSR_IA32_PERF_STATUS Avi Kivity
2010-09-01 7:34 ` [PATCH 2/2] KVM: Fix bogus value returned for MSR_IA32_PERF_STATUS Avi Kivity
2010-09-01 8:49 ` Alexander Graf
2010-09-01 8:55 ` Avi Kivity
2010-09-01 8:56 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox