kvm: x86: Don't modify MSR_PLATFORM_INFO on vCPU reset
diff mbox series

Message ID 20181030192021.110384-1-jmattson@google.com
State New
Headers show
Series
  • kvm: x86: Don't modify MSR_PLATFORM_INFO on vCPU reset
Related show

Commit Message

Jim Mattson Oct. 30, 2018, 7:20 p.m. UTC
If userspace has provided a different value for this MSR (e.g with the
turbo bits set), the userspace-provided value should survive a vCPU
reset. For backwards compatibility, MSR_PLATFORM_INFO is initialized
in kvm_arch_vcpu_setup.

Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Drew Schmitt <dasch@google.com>
Cc: Abhiroop Dabral <adabral@paloaltonetworks.com>
---
 arch/x86/kvm/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jim Mattson Nov. 30, 2018, 10 p.m. UTC | #1
Ping.
On Tue, Oct 30, 2018 at 12:21 PM Jim Mattson <jmattson@google.com> wrote:
>
> If userspace has provided a different value for this MSR (e.g with the
> turbo bits set), the userspace-provided value should survive a vCPU
> reset. For backwards compatibility, MSR_PLATFORM_INFO is initialized
> in kvm_arch_vcpu_setup.
>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Drew Schmitt <dasch@google.com>
> Cc: Abhiroop Dabral <adabral@paloaltonetworks.com>
> ---
>  arch/x86/kvm/x86.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 66d66d77caee..6ab3149bf9dc 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8617,6 +8617,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
>
>  int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>  {
> +       vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
>         kvm_vcpu_mtrr_init(vcpu);
>         vcpu_load(vcpu);
>         kvm_vcpu_reset(vcpu, false);
> @@ -8719,7 +8720,6 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>                 kvm_pmu_reset(vcpu);
>                 vcpu->arch.smbase = 0x30000;
>
> -               vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
>                 vcpu->arch.msr_misc_features_enables = 0;
>
>                 vcpu->arch.xcr0 = XFEATURE_MASK_FP;
> --
> 2.19.1.568.g152ad8e336-goog
>
Wanpeng Li Dec. 11, 2018, 6:09 a.m. UTC | #2
On Wed, 31 Oct 2018 at 03:25, Jim Mattson <jmattson@google.com> wrote:
>
> If userspace has provided a different value for this MSR (e.g with the
> turbo bits set), the userspace-provided value should survive a vCPU
> reset. For backwards compatibility, MSR_PLATFORM_INFO is initialized
> in kvm_arch_vcpu_setup.
>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Drew Schmitt <dasch@google.com>
> Cc: Abhiroop Dabral <adabral@paloaltonetworks.com>
> ---
>  arch/x86/kvm/x86.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 66d66d77caee..6ab3149bf9dc 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8617,6 +8617,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
>
>  int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>  {
> +       vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
>         kvm_vcpu_mtrr_init(vcpu);
>         vcpu_load(vcpu);
>         kvm_vcpu_reset(vcpu, false);
> @@ -8719,7 +8720,6 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>                 kvm_pmu_reset(vcpu);
>                 vcpu->arch.smbase = 0x30000;
>
> -               vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;

I think you miss the init_event check?

Regards,
Wanpeng Li

>                 vcpu->arch.msr_misc_features_enables = 0;
>
>                 vcpu->arch.xcr0 = XFEATURE_MASK_FP;
> --
> 2.19.1.568.g152ad8e336-goog
>
Jim Mattson Dec. 11, 2018, 6:49 p.m. UTC | #3
On Mon, Dec 10, 2018 at 10:09 PM Wanpeng Li <kernellwp@gmail.com> wrote:
>
> On Wed, 31 Oct 2018 at 03:25, Jim Mattson <jmattson@google.com> wrote:
> >
> > If userspace has provided a different value for this MSR (e.g with the
> > turbo bits set), the userspace-provided value should survive a vCPU
> > reset. For backwards compatibility, MSR_PLATFORM_INFO is initialized
> > in kvm_arch_vcpu_setup.
> >
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > Reviewed-by: Drew Schmitt <dasch@google.com>
> > Cc: Abhiroop Dabral <adabral@paloaltonetworks.com>
> > ---
> >  arch/x86/kvm/x86.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 66d66d77caee..6ab3149bf9dc 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -8617,6 +8617,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
> >
> >  int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
> >  {
> > +       vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
> >         kvm_vcpu_mtrr_init(vcpu);
> >         vcpu_load(vcpu);
> >         kvm_vcpu_reset(vcpu, false);
> > @@ -8719,7 +8720,6 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> >                 kvm_pmu_reset(vcpu);
> >                 vcpu->arch.smbase = 0x30000;
> >
> > -               vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
>
> I think you miss the init_event check?

No; I saw it. If userspace has provided a different value for this MSR
(e.g. if it has filled in the scalable bus frequency in bits 15:8),
that value should survive a full CPU reset as well as an INIT.
Paolo Bonzini Dec. 14, 2018, 10:56 a.m. UTC | #4
On 30/11/18 23:00, Jim Mattson wrote:
> Ping.
> On Tue, Oct 30, 2018 at 12:21 PM Jim Mattson <jmattson@google.com> wrote:
>>
>> If userspace has provided a different value for this MSR (e.g with the
>> turbo bits set), the userspace-provided value should survive a vCPU
>> reset. For backwards compatibility, MSR_PLATFORM_INFO is initialized
>> in kvm_arch_vcpu_setup.
>>
>> Signed-off-by: Jim Mattson <jmattson@google.com>
>> Reviewed-by: Drew Schmitt <dasch@google.com>
>> Cc: Abhiroop Dabral <adabral@paloaltonetworks.com>
>> ---
>>  arch/x86/kvm/x86.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 66d66d77caee..6ab3149bf9dc 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -8617,6 +8617,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
>>
>>  int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>  {
>> +       vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
>>         kvm_vcpu_mtrr_init(vcpu);
>>         vcpu_load(vcpu);
>>         kvm_vcpu_reset(vcpu, false);
>> @@ -8719,7 +8720,6 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>                 kvm_pmu_reset(vcpu);
>>                 vcpu->arch.smbase = 0x30000;
>>
>> -               vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
>>                 vcpu->arch.msr_misc_features_enables = 0;
>>
>>                 vcpu->arch.xcr0 = XFEATURE_MASK_FP;
>> --
>> 2.19.1.568.g152ad8e336-goog
>>

Queued, thanks.

Paolo

Patch
diff mbox series

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 66d66d77caee..6ab3149bf9dc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8617,6 +8617,7 @@  struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
 
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
+	vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
 	kvm_vcpu_mtrr_init(vcpu);
 	vcpu_load(vcpu);
 	kvm_vcpu_reset(vcpu, false);
@@ -8719,7 +8720,6 @@  void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 		kvm_pmu_reset(vcpu);
 		vcpu->arch.smbase = 0x30000;
 
-		vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
 		vcpu->arch.msr_misc_features_enables = 0;
 
 		vcpu->arch.xcr0 = XFEATURE_MASK_FP;