[PATCH] KVM: Handle MSR_IA32_PERF_CTL

[PATCH] KVM: Handle MSR_IA32_PERF_CTL

[kmeaw]

From: Dmitry Bilunov <kmeaw@yandex-team.ru>

Intel CPUs having Turbo Boost feature implement an MSR to provide a
control interface via rdmsr/wrmsr instructions. One could detect the
presence of this feature by issuing one of these instructions and
handling the #GP exception which is generated in case the referenced MSR
is not implemented by the CPU.

KVM's vCPU model behaves exactly as a real CPU in this case by injecting
a fault when MSR_IA32_PERF_CTL is called (which KVM does not support).
However, some operating systems use this register during an early boot
stage in which their kernel is not capable of handling #GP correctly,
causing #DP and finally a triple fault effectively resetting the vCPU.

This patch implements a dummy handler for MSR_IA32_PERF_CTL to avoid the
crashes. Most notably it fixes an issue with MacOS X 10.10 kernel.

Signed-off-by: Dmitry Bilunov <kmeaw@yandex-team.ru>
---
 arch/x86/kvm/x86.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c805cf4..9f38c7f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -983,6 +983,7 @@ static u32 emulated_msrs[] = {
        MSR_IA32_MCG_STATUS,
        MSR_IA32_MCG_CTL,
        MSR_IA32_SMBASE,
+       MSR_IA32_PERF_CTL,
 };
 
 static unsigned num_emulated_msrs;
@@ -2050,6 +2051,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
        case MSR_VM_HSAVE_PA:
        case MSR_AMD64_PATCH_LOADER:
        case MSR_AMD64_BU_CFG2:
+       case MSR_IA32_PERF_CTL:
                break;
 
        case MSR_EFER:
@@ -2314,6 +2316,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
        case MSR_AMD64_NB_CFG:
        case MSR_FAM10H_MMIO_CONF_BASE:
        case MSR_AMD64_BU_CFG2:
+       case MSR_IA32_PERF_CTL:
                msr_info->data = 0;
                break;
        case MSR_K7_EVNTSEL0 ... MSR_K7_EVNTSEL3:
-- 
2.8.2

Re: [PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Radim Krčmář]

2016-05-26 10:32+0300, kmeaw@yandex-team.ru:
> From: Dmitry Bilunov <kmeaw@yandex-team.ru>
> 
> Intel CPUs having Turbo Boost feature implement an MSR to provide a
> control interface via rdmsr/wrmsr instructions. One could detect the
> presence of this feature by issuing one of these instructions and
> handling the #GP exception which is generated in case the referenced MSR
> is not implemented by the CPU.
> 
> KVM's vCPU model behaves exactly as a real CPU in this case by injecting
> a fault when MSR_IA32_PERF_CTL is called (which KVM does not support).
> However, some operating systems use this register during an early boot
> stage in which their kernel is not capable of handling #GP correctly,
> causing #DP and finally a triple fault effectively resetting the vCPU.
> 
> This patch implements a dummy handler for MSR_IA32_PERF_CTL to avoid the
> crashes. Most notably it fixes an issue with MacOS X 10.10 kernel.
> 
> Signed-off-by: Dmitry Bilunov <kmeaw@yandex-team.ru>
> ---
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> @@ -2050,6 +2051,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>         case MSR_VM_HSAVE_PA:
>         case MSR_AMD64_PATCH_LOADER:
>         case MSR_AMD64_BU_CFG2:
> +       case MSR_IA32_PERF_CTL:
>                 break;

Does MacOS X write it too?

Thanks.

Re: [PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Gabriel L. Somlo]

On Thu, May 26, 2016 at 10:39:31PM +0200, Radim Krčmář wrote:
> 2016-05-26 10:32+0300, kmeaw@yandex-team.ru:
> > From: Dmitry Bilunov <kmeaw@yandex-team.ru>
> > 
> > Intel CPUs having Turbo Boost feature implement an MSR to provide a
> > control interface via rdmsr/wrmsr instructions. One could detect the
> > presence of this feature by issuing one of these instructions and
> > handling the #GP exception which is generated in case the referenced MSR
> > is not implemented by the CPU.
> > 
> > KVM's vCPU model behaves exactly as a real CPU in this case by injecting
> > a fault when MSR_IA32_PERF_CTL is called (which KVM does not support).
> > However, some operating systems use this register during an early boot
> > stage in which their kernel is not capable of handling #GP correctly,
> > causing #DP and finally a triple fault effectively resetting the vCPU.
> > 
> > This patch implements a dummy handler for MSR_IA32_PERF_CTL to avoid the
> > crashes. Most notably it fixes an issue with MacOS X 10.10 kernel.
> > 
> > Signed-off-by: Dmitry Bilunov <kmeaw@yandex-team.ru>
> > ---
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > @@ -2050,6 +2051,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >         case MSR_VM_HSAVE_PA:
> >         case MSR_AMD64_PATCH_LOADER:
> >         case MSR_AMD64_BU_CFG2:
> > +       case MSR_IA32_PERF_CTL:
> >                 break;
> 
> Does MacOS X write it too?

After setting /sys/module/kvm/parameters/ignore_msrs, all I get in
dmesg after firing up OS X is:

	vcpu0 ignored rdmsr: 0x199

So no, I don't think it would try to write it.

HTH,
--Gabriel

Re: [PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Radim Krčmář]

2016-05-27 09:49+0300, kmeaw@yandex-team.ru:
> 26.05.2016, 23:44, "Gabriel L. Somlo" <gsomlo@gmail.com>:
>> On Thu, May 26, 2016 at 10:39:31PM +0200, Radim Krčmář wrote:
>>>  2016-05-26 10:32+0300, kmeaw@yandex-team.ru:
>>>  > This patch implements a dummy handler for MSR_IA32_PERF_CTL to avoid the
>>>  > crashes. Most notably it fixes an issue with MacOS X 10.10 kernel.
>>>  Does MacOS X write it too?
>>
>> After setting /sys/module/kvm/parameters/ignore_msrs, all I get in
>> dmesg after firing up OS X is:
>>
>>         vcpu0 ignored rdmsr: 0x199
>>
>> So no, I don't think it would try to write it.
> 
> That's right, OS X does not issue an wrmsr to 0x199. More specifically, I have not
> observed that on my KVM instances. Should I remove the "wrmsr" portion from the
> patch?

Yes, please.  Silently ignoring the write is worse than #GP and #GP is
not a problem, so I wouldn't bother with a phony implementation.
Returning 0 on read is ok as seems to mean P-state=0, which is within
expectations.

(I wonder why MacOS X doesn't read IA32_PERF_STATUS, though.)

Re: [PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Radim Krčmář]

2016-05-27 17:22+0200, Radim Krčmář:
> (I wonder why MacOS X doesn't read IA32_PERF_STATUS, though.)

Oh, it maybe does ... we already emulate status and return 0x1000 in its
bottom 16 bits.  I have no idea what is that supposed to mean, but I
think we should return 0x1000 in IA32_PERF_CTL then.

(Would be nice to understand how that 0x1000 happened ... we might want
 0 in both.)

Re: [PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Paolo Bonzini]

> 2016-05-27 17:22+0200, Radim Krčmář:
> > (I wonder why MacOS X doesn't read IA32_PERF_STATUS, though.)
> 
> Oh, it maybe does ... we already emulate status and return 0x1000 in its
> bottom 16 bits.  I have no idea what is that supposed to mean, but I
> think we should return 0x1000 in IA32_PERF_CTL then.

It's 1000, not 0x1000 (instead, on real hardware the value is typically a
multiple of 256).  It was added for Darwin too.

Returning different values is okay, because they are different on real
hardware too:

(sudo dd if=/dev/cpu/0/msr skip=$((0x198)) iflag=skip_bytes bs=8 count=1;
 sudo dd if=/dev/cpu/0/msr skip=$((0x199)) iflag=skip_bytes bs=8 count=1) | od -tx8
0000000 00001f3900001100 0000000000001300
        ^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^
        PERF_STATUS      PERF_CTL

And perhaps if we returned non-zero values for PERF_CTL Darwin would try to
write to it.  So returning zero is fine, I think.  There is no correct answer...

Paolo

> (Would be nice to understand how that 0x1000 happened ... we might want
>  0 in both.)
> 

Re: [PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Radim Krčmář]

2016-05-31 03:53-0400, Paolo Bonzini:
> > 2016-05-27 17:22+0200, Radim Krčmář:
> > > (I wonder why MacOS X doesn't read IA32_PERF_STATUS, though.)
> > 
> > Oh, it maybe does ... we already emulate status and return 0x1000 in its
> > bottom 16 bits.  I have no idea what is that supposed to mean, but I
> > think we should return 0x1000 in IA32_PERF_CTL then.
> 
> It's 1000, not 0x1000 (instead, on real hardware the value is typically a
> multiple of 256).  It was added for Darwin too.

Ah, thanks.  (Drivers say that bottom 8 bits are not used.)

> Returning different values is okay, because they are different on real
> hardware too:
> 
> (sudo dd if=/dev/cpu/0/msr skip=$((0x198)) iflag=skip_bytes bs=8 count=1;
>  sudo dd if=/dev/cpu/0/msr skip=$((0x199)) iflag=skip_bytes bs=8 count=1) | od -tx8
> 0000000 00001f3900001100 0000000000001300
>         ^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^
>         PERF_STATUS      PERF_CTL
> 
> And perhaps if we returned non-zero values for PERF_CTL Darwin would try to
> write to it.  So returning zero is fine, I think.  There is no correct answer...

Yeah, 0 seems fine.  PERF_CTL the target value for PERF_STATUS, but OS
shouldn't put much trust in those values ... especially under KVM, where
those MSRs make little sense.

Re: [PATCH v3] KVM: Handle MSR_IA32_PERF_CTL

[Radim Krčmář]

2016-05-31 13:06+0300, kmeaw@yandex-team.ru:
> 31.05.2016, 11:21, "Paolo Bonzini" <pbonzini@redhat.com>:
> >>  2016-05-27 17:22+0200, Radim Krčmář:
> >>  > (I wonder why MacOS X doesn't read IA32_PERF_STATUS, though.)
> >>
> >>  Oh, it maybe does ... we already emulate status and return 0x1000 in its
> >>  bottom 16 bits. I have no idea what is that supposed to mean, but I
> >>  think we should return 0x1000 in IA32_PERF_CTL then.
> >
> > It's 1000, not 0x1000 (instead, on real hardware the value is typically a
> > multiple of 256). It was added for Darwin too.
> >
> > Returning different values is okay, because they are different on real
> > hardware too:
> >
> > (sudo dd if=/dev/cpu/0/msr skip=$((0x198)) iflag=skip_bytes bs=8 count=1;
> >  sudo dd if=/dev/cpu/0/msr skip=$((0x199)) iflag=skip_bytes bs=8 count=1) | od -tx8
> > 0000000 00001f3900001100 0000000000001300
> >         ^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^
> >         PERF_STATUS PERF_CTL
> >
> > And perhaps if we returned non-zero values for PERF_CTL Darwin would try to
> > write to it. So returning zero is fine, I think. There is no correct answer...
> 
> Thank you. I have removed MSR_IA32_PERF_CTL from emulated_msrs[]. Returning
> 1000 (0x3e8) for PERF_STATUS and 0 for PERF_CTL works fine with MacOS X.
> 
> Just in case here are MSRs from i5-4460:
> PERF_STATUS: 0000202800002100
> PERF_CTL:    0000000000002200
> 
> Chaning KVM's PERL_CTL from 0 to 0x2200 does not seem to interfere with MacOS X
> boot process. It does not attempt to wrmsr into this register.
> 
> Here is a refined version of the patch:
> -- 
> 
> From: Dmitry Bilunov <kmeaw@yandex-team.ru>
> 
> Intel CPUs having Turbo Boost feature implement an MSR to provide a
> control interface via rdmsr/wrmsr instructions. One could detect the
> presence of this feature by issuing one of these instructions and
> handling the #GP exception which is generated in case the referenced MSR
> is not implemented by the CPU.
> 
> KVM's vCPU model behaves exactly as a real CPU in this case by injecting
> a fault when MSR_IA32_PERF_CTL is called (which KVM does not support).
> However, some operating systems use this register during an early boot
> stage in which their kernel is not capable of handling #GP correctly,
> causing #DP and finally a triple fault effectively resetting the vCPU.
> 
> This patch implements a dummy handler for MSR_IA32_PERF_CTL to avoid the
> crashes.
> ---

The code looks good.  Please resend with your signed-off-by and
preserved writespace (tabs were converted to spaces),

thanks.

>  arch/x86/kvm/x86.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c805cf4..d0a5b4b 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2314,6 +2314,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>         case MSR_AMD64_NB_CFG:
>         case MSR_FAM10H_MMIO_CONF_BASE:
>         case MSR_AMD64_BU_CFG2:
> +       case MSR_IA32_PERF_CTL:
>                 msr_info->data = 0;
>                 break;
>         case MSR_K7_EVNTSEL0 ... MSR_K7_EVNTSEL3:
> -- 
> 2.8.2

[PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Dmitry Bilunov]

Intel CPUs having Turbo Boost feature implement an MSR to provide a
control interface via rdmsr/wrmsr instructions. One could detect the
presence of this feature by issuing one of these instructions and
handling the #GP exception which is generated in case the referenced MSR
is not implemented by the CPU.

KVM's vCPU model behaves exactly as a real CPU in this case by injecting
a fault when MSR_IA32_PERF_CTL is called (which KVM does not support).
However, some operating systems use this register during an early boot
stage in which their kernel is not capable of handling #GP correctly,
causing #DP and finally a triple fault effectively resetting the vCPU.

This patch implements a dummy handler for MSR_IA32_PERF_CTL to avoid the
crashes.

Signed-off-by: Dmitry Bilunov <kmeaw@yandex-team.ru>
---
 arch/x86/kvm/x86.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c805cf4..d0a5b4b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2314,6 +2314,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_AMD64_NB_CFG:
 	case MSR_FAM10H_MMIO_CONF_BASE:
 	case MSR_AMD64_BU_CFG2:
+	case MSR_IA32_PERF_CTL:
 		msr_info->data = 0;
 		break;
 	case MSR_K7_EVNTSEL0 ... MSR_K7_EVNTSEL3:
-- 
2.8.2

Re: [PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Radim Krčmář]

2016-05-31 17:38+0300, Dmitry Bilunov:
> Intel CPUs having Turbo Boost feature implement an MSR to provide a
> control interface via rdmsr/wrmsr instructions. One could detect the
> presence of this feature by issuing one of these instructions and
> handling the #GP exception which is generated in case the referenced MSR
> is not implemented by the CPU.
> 
> KVM's vCPU model behaves exactly as a real CPU in this case by injecting
> a fault when MSR_IA32_PERF_CTL is called (which KVM does not support).
> However, some operating systems use this register during an early boot
> stage in which their kernel is not capable of handling #GP correctly,
> causing #DP and finally a triple fault effectively resetting the vCPU.
> 
> This patch implements a dummy handler for MSR_IA32_PERF_CTL to avoid the
> crashes.
> 
> Signed-off-by: Dmitry Bilunov <kmeaw@yandex-team.ru>
> ---

Applied, thank you.

>  arch/x86/kvm/x86.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c805cf4..d0a5b4b 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2314,6 +2314,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	case MSR_AMD64_NB_CFG:
>  	case MSR_FAM10H_MMIO_CONF_BASE:
>  	case MSR_AMD64_BU_CFG2:
> +	case MSR_IA32_PERF_CTL:
>  		msr_info->data = 0;
>  		break;
>  	case MSR_K7_EVNTSEL0 ... MSR_K7_EVNTSEL3:
> -- 
> 2.8.2
> 

Re: [PATCH] KVM: Handle MSR_IA32_PERF_CTL

[Radim Krčmář]

2016-05-26 10:32+0300, kmeaw@yandex-team.ru:
> From: Dmitry Bilunov <kmeaw@yandex-team.ru>
> 
> Intel CPUs having Turbo Boost feature implement an MSR to provide a
> control interface via rdmsr/wrmsr instructions. One could detect the
> presence of this feature by issuing one of these instructions and
> handling the #GP exception which is generated in case the referenced MSR
> is not implemented by the CPU.
> 
> KVM's vCPU model behaves exactly as a real CPU in this case by injecting
> a fault when MSR_IA32_PERF_CTL is called (which KVM does not support).
> However, some operating systems use this register during an early boot
> stage in which their kernel is not capable of handling #GP correctly,
> causing #DP and finally a triple fault effectively resetting the vCPU.
> 
> This patch implements a dummy handler for MSR_IA32_PERF_CTL to avoid the
> crashes. Most notably it fixes an issue with MacOS X 10.10 kernel.
> 
> Signed-off-by: Dmitry Bilunov <kmeaw@yandex-team.ru>
> ---
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> @@ -983,6 +983,7 @@ static u32 emulated_msrs[] = {
>         MSR_IA32_MCG_STATUS,
>         MSR_IA32_MCG_CTL,
>         MSR_IA32_SMBASE,
> +       MSR_IA32_PERF_CTL,

The MSR value is always 0, so there is no point in putting it here.