WARN_ON(!svm->tsc_scaling_enabled)

WARN_ON(!svm->tsc_scaling_enabled)

[Dāvis Mosāns]

Hi,

I'm using:

kvm ignore_msrs=1
kvm_amd nested=1

invtsc=on
tsc-deadline=on
tsc-scale=off
svm=on

and my dmesg gets spammed with warnings like every second.
Also sometimes guest VM freezes when booting.


if (svm->tsc_ratio_msr != kvm_default_tsc_scaling_ratio) {
    WARN_ON(!svm->tsc_scaling_enabled);
    nested_svm_update_tsc_ratio_msr(vcpu);
}

WARNING: CPU: 6 PID: 21336 at arch/x86/kvm/svm/nested.c:582
nested_vmcb02_prepare_control (arch/x86/kvm/svm/nested.c:582
(discriminator 1)) kvm_amd
RIP: 0010:nested_vmcb02_prepare_control (arch/x86/kvm/svm/nested.c:582
(discriminator 1)) kvm_amd
Call Trace:
 <TASK>
enter_svm_guest_mode (arch/x86/kvm/svm/nested.c:480 (discriminator 3)
arch/x86/kvm/svm/nested.c:491 (discriminator 3)
arch/x86/kvm/svm/nested.c:647 (discriminator 3)) kvm_amd
nested_svm_vmrun (arch/x86/kvm/svm/nested.c:726) kvm_amd
kvm_arch_vcpu_ioctl_run (arch/x86/kvm/x86.c:10243 arch/x86/kvm/x86.c:10449) kvm
kvm_vcpu_ioctl (arch/x86/kvm/../../../virt/kvm/kvm_main.c:3908) kvm
__x64_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:874 fs/ioctl.c:860 fs/ioctl.c:860)
do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
? kvm_on_user_return (./arch/x86/include/asm/paravirt.h:194
./arch/x86/include/asm/paravirt.h:227 arch/x86/kvm/x86.c:370) kvm
? fire_user_return_notifiers (kernel/user-return-notifier.c:42
(discriminator 11))
? exit_to_user_mode_prepare (./arch/x86/include/asm/entry-common.h:53
kernel/entry/common.c:209)
? syscall_exit_to_user_mode (./arch/x86/include/asm/jump_label.h:55
./arch/x86/include/asm/nospec-branch.h:302
./arch/x86/include/asm/entry-common.h:94 kernel/entry/common.c:131
kernel/entry/common.c:302)
? do_syscall_64 (arch/x86/entry/common.c:87)

Maybe this warning is wrong?

Best regards,
Dāvis

Re: WARN_ON(!svm->tsc_scaling_enabled)

[Maxim Levitsky]

On Sat, 2022-02-19 at 05:22 +0200, Dāvis Mosāns wrote:
> Hi,
> 
> I'm using:
> 
> kvm ignore_msrs=1
> kvm_amd nested=1
> 
> invtsc=on
> tsc-deadline=on
> tsc-scale=off
> svm=on
> 
> and my dmesg gets spammed with warnings like every second.
> Also sometimes guest VM freezes when booting.
> 
> 
> if (svm->tsc_ratio_msr != kvm_default_tsc_scaling_ratio) {
>     WARN_ON(!svm->tsc_scaling_enabled);
>     nested_svm_update_tsc_ratio_msr(vcpu);
> }
> 
> WARNING: CPU: 6 PID: 21336 at arch/x86/kvm/svm/nested.c:582
> nested_vmcb02_prepare_control (arch/x86/kvm/svm/nested.c:582
> (discriminator 1)) kvm_amd
> RIP: 0010:nested_vmcb02_prepare_control (arch/x86/kvm/svm/nested.c:582
> (discriminator 1)) kvm_amd
> Call Trace:
>  <TASK>
> enter_svm_guest_mode (arch/x86/kvm/svm/nested.c:480 (discriminator 3)
> arch/x86/kvm/svm/nested.c:491 (discriminator 3)
> arch/x86/kvm/svm/nested.c:647 (discriminator 3)) kvm_amd
> nested_svm_vmrun (arch/x86/kvm/svm/nested.c:726) kvm_amd
> kvm_arch_vcpu_ioctl_run (arch/x86/kvm/x86.c:10243 arch/x86/kvm/x86.c:10449) kvm
> kvm_vcpu_ioctl (arch/x86/kvm/../../../virt/kvm/kvm_main.c:3908) kvm
> __x64_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:874 fs/ioctl.c:860 fs/ioctl.c:860)
> do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
> ? kvm_on_user_return (./arch/x86/include/asm/paravirt.h:194
> ./arch/x86/include/asm/paravirt.h:227 arch/x86/kvm/x86.c:370) kvm
> ? fire_user_return_notifiers (kernel/user-return-notifier.c:42
> (discriminator 11))
> ? exit_to_user_mode_prepare (./arch/x86/include/asm/entry-common.h:53
> kernel/entry/common.c:209)
> ? syscall_exit_to_user_mode (./arch/x86/include/asm/jump_label.h:55
> ./arch/x86/include/asm/nospec-branch.h:302
> ./arch/x86/include/asm/entry-common.h:94 kernel/entry/common.c:131
> kernel/entry/common.c:302)
> ? do_syscall_64 (arch/x86/entry/common.c:87)
> 
> Maybe this warning is wrong?

The warning is not wrong. The svm->tsc_ratio_msr is the nested TSC scale ratio,
which should never have non default value if you don't exposed TSC scaling to the guest.

What I think is happening though is that svm_set_msr does allow to set MSR_AMD64_TSC_RATIO even if
the guest cpuid doesn't support TSC scaling if the write comes from the  host (that is qemu).

On qemu side I see that when guest tsc scaling is disabled, MSR_AMD64_TSC_RATIO ends up beeing
0, and still uploaded to KVM, which I think triggers this warning.

So the warning should IMHO be removed, but also the code should be changed to ignore
the value of the MSR_AMD64_TSC_RATIO when the nested tsc scaling is disabled.

I'll send a patch to fix this soon.

Best regards,
	Maxim Levitsky

> 
> Best regards,
> Dāvis
> 

[PATCH] KVM: nSVM: fix nested tsc scaling when not enabled but MSR_AMD64_TSC_RATIO set

[Maxim Levitsky]

For compatibility with userspace the MSR_AMD64_TSC_RATIO can be set
even when the feature is not exposed to the guest, which breaks assumptions
that it has the default value in this case.

Fixes: 5228eb96a487 ("KVM: x86: nSVM: implement nested TSC scaling")
Cc: stable@vger.kernel.org

Reported-by: Dāvis Mosāns <davispuh@gmail.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/svm/nested.c | 10 ++++------
 arch/x86/kvm/svm/svm.c    |  5 +++--
 arch/x86/kvm/svm/svm.h    |  1 +
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 1218b5a342fc..a2e2436057dc 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -574,14 +574,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
 	vcpu->arch.tsc_offset = kvm_calc_nested_tsc_offset(
 			vcpu->arch.l1_tsc_offset,
 			svm->nested.ctl.tsc_offset,
-			svm->tsc_ratio_msr);
+			svm_get_l2_tsc_multiplier(vcpu));
 
 	svm->vmcb->control.tsc_offset = vcpu->arch.tsc_offset;
 
-	if (svm->tsc_ratio_msr != kvm_default_tsc_scaling_ratio) {
-		WARN_ON(!svm->tsc_scaling_enabled);
+	if (svm_get_l2_tsc_multiplier(vcpu) != kvm_default_tsc_scaling_ratio)
 		nested_svm_update_tsc_ratio_msr(vcpu);
-	}
 
 	svm->vmcb->control.int_ctl             =
 		(svm->nested.ctl.int_ctl & int_ctl_vmcb12_bits) |
@@ -867,8 +865,7 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
 		vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
 	}
 
-	if (svm->tsc_ratio_msr != kvm_default_tsc_scaling_ratio) {
-		WARN_ON(!svm->tsc_scaling_enabled);
+	if (svm_get_l2_tsc_multiplier(vcpu) != kvm_default_tsc_scaling_ratio) {
 		vcpu->arch.tsc_scaling_ratio = vcpu->arch.l1_tsc_scaling_ratio;
 		svm_write_tsc_multiplier(vcpu, vcpu->arch.tsc_scaling_ratio);
 	}
@@ -1264,6 +1261,7 @@ void nested_svm_update_tsc_ratio_msr(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
+	WARN_ON_ONCE(!svm->tsc_scaling_enabled);
 	vcpu->arch.tsc_scaling_ratio =
 		kvm_calc_nested_tsc_multiplier(vcpu->arch.l1_tsc_scaling_ratio,
 					       svm->tsc_ratio_msr);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 975be872cd1a..5cf6bb5bfd3e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -911,11 +911,12 @@ static u64 svm_get_l2_tsc_offset(struct kvm_vcpu *vcpu)
 	return svm->nested.ctl.tsc_offset;
 }
 
-static u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu)
+u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	return svm->tsc_ratio_msr;
+	return svm->tsc_scaling_enabled ? svm->tsc_ratio_msr :
+	       kvm_default_tsc_scaling_ratio;
 }
 
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 852b12aee03d..006571dd30df 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -542,6 +542,7 @@ int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
 			       bool has_error_code, u32 error_code);
 int nested_svm_exit_special(struct vcpu_svm *svm);
 void nested_svm_update_tsc_ratio_msr(struct kvm_vcpu *vcpu);
+u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu);
 void svm_write_tsc_multiplier(struct kvm_vcpu *vcpu, u64 multiplier);
 void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
 				       struct vmcb_control_area *control);
-- 
2.26.3

Re: [PATCH] KVM: nSVM: fix nested tsc scaling when not enabled but MSR_AMD64_TSC_RATIO set

[Paolo Bonzini]

On 2/21/22 11:33, Maxim Levitsky wrote:
> For compatibility with userspace the MSR_AMD64_TSC_RATIO can be set
> even when the feature is not exposed to the guest, which breaks assumptions
> that it has the default value in this case.
> 
> Fixes: 5228eb96a487 ("KVM: x86: nSVM: implement nested TSC scaling")
> Cc: stable@vger.kernel.org
> 
> Reported-by: Dāvis Mosāns <davispuh@gmail.com>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>

It's not clear how QEMU ends up writing MSR_AMD64_TSC_RATIO_DEFAULT 
rather than 0, but we clearly have a bug in KVM.  It should not allow 
writing 0 in the first place if tsc-ratio is not available in the VM.

If QEMU really can get itself in this situation, we cannot fix this 
except with KVM_CAP_DISABLE_QUIRKS (a quirk that would accept and ignore 
host-initiated writes if the CPUID bit is not available) or perhaps with 
a pr_warn_ratelimited and a quick deprecation cycle, until some time 
after 6.2.1 is released.

Hmmm... maybe the issue is actually that KVM *returns* 0 from 
KVM_GET_MSRS?  And in this case, fixing KVM would also prevent QEMU from 
sending the bogus KVM_SET_MSRS?

Thanks,

Paolo

> ---
>   arch/x86/kvm/svm/nested.c | 10 ++++------
>   arch/x86/kvm/svm/svm.c    |  5 +++--
>   arch/x86/kvm/svm/svm.h    |  1 +
>   3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 1218b5a342fc..a2e2436057dc 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -574,14 +574,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
>   	vcpu->arch.tsc_offset = kvm_calc_nested_tsc_offset(
>   			vcpu->arch.l1_tsc_offset,
>   			svm->nested.ctl.tsc_offset,
> -			svm->tsc_ratio_msr);
> +			svm_get_l2_tsc_multiplier(vcpu));
>   
>   	svm->vmcb->control.tsc_offset = vcpu->arch.tsc_offset;
>   
> -	if (svm->tsc_ratio_msr != kvm_default_tsc_scaling_ratio) {
> -		WARN_ON(!svm->tsc_scaling_enabled);
> +	if (svm_get_l2_tsc_multiplier(vcpu) != kvm_default_tsc_scaling_ratio)
>   		nested_svm_update_tsc_ratio_msr(vcpu);
> -	}
>   
>   	svm->vmcb->control.int_ctl             =
>   		(svm->nested.ctl.int_ctl & int_ctl_vmcb12_bits) |
> @@ -867,8 +865,7 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
>   		vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
>   	}
>   
> -	if (svm->tsc_ratio_msr != kvm_default_tsc_scaling_ratio) {
> -		WARN_ON(!svm->tsc_scaling_enabled);
> +	if (svm_get_l2_tsc_multiplier(vcpu) != kvm_default_tsc_scaling_ratio) {
>   		vcpu->arch.tsc_scaling_ratio = vcpu->arch.l1_tsc_scaling_ratio;
>   		svm_write_tsc_multiplier(vcpu, vcpu->arch.tsc_scaling_ratio);
>   	}
> @@ -1264,6 +1261,7 @@ void nested_svm_update_tsc_ratio_msr(struct kvm_vcpu *vcpu)
>   {
>   	struct vcpu_svm *svm = to_svm(vcpu);
>   
> +	WARN_ON_ONCE(!svm->tsc_scaling_enabled);
>   	vcpu->arch.tsc_scaling_ratio =
>   		kvm_calc_nested_tsc_multiplier(vcpu->arch.l1_tsc_scaling_ratio,
>   					       svm->tsc_ratio_msr);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 975be872cd1a..5cf6bb5bfd3e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -911,11 +911,12 @@ static u64 svm_get_l2_tsc_offset(struct kvm_vcpu *vcpu)
>   	return svm->nested.ctl.tsc_offset;
>   }
>   
> -static u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu)
> +u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu)
>   {
>   	struct vcpu_svm *svm = to_svm(vcpu);
>   
> -	return svm->tsc_ratio_msr;
> +	return svm->tsc_scaling_enabled ? svm->tsc_ratio_msr :
> +	       kvm_default_tsc_scaling_ratio;
>   }
>   
>   static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 852b12aee03d..006571dd30df 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -542,6 +542,7 @@ int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
>   			       bool has_error_code, u32 error_code);
>   int nested_svm_exit_special(struct vcpu_svm *svm);
>   void nested_svm_update_tsc_ratio_msr(struct kvm_vcpu *vcpu);
> +u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu);
>   void svm_write_tsc_multiplier(struct kvm_vcpu *vcpu, u64 multiplier);
>   void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
>   				       struct vmcb_control_area *control);

Re: [PATCH] KVM: nSVM: fix nested tsc scaling when not enabled but MSR_AMD64_TSC_RATIO set

[Maxim Levitsky]

On Mon, 2022-02-21 at 13:33 +0100, Paolo Bonzini wrote:
> On 2/21/22 11:33, Maxim Levitsky wrote:
> > For compatibility with userspace the MSR_AMD64_TSC_RATIO can be set
> > even when the feature is not exposed to the guest, which breaks assumptions
> > that it has the default value in this case.
> > 
> > Fixes: 5228eb96a487 ("KVM: x86: nSVM: implement nested TSC scaling")
> > Cc: stable@vger.kernel.org
> > 
> > Reported-by: Dāvis Mosāns <davispuh@gmail.com>
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> 
> It's not clear how QEMU ends up writing MSR_AMD64_TSC_RATIO_DEFAULT 
> rather than 0, but we clearly have a bug in KVM.  It should not allow 
> writing 0 in the first place if tsc-ratio is not available in the VM.

Qemu currently (the code is very new so it can be changed) writes the initial value of
0 to this msr if tsc scaling is disabled in the guest, or MSR_AMD64_TSC_RATIO_DEFAULT
if the tsc scaling is enabled.

The guest can change it only when TSC scaling is enabled for it.
If tsc scaling is not enabled, both guest reads or writes of this MSR get #GP.

I only allowed qemu writes of this msr because I thought that qemu might
first set the MSR and then set guest CPUID.

Also, for example the MSR_IA32_XSS uses the same logic in KVM.

As for why qemu sets this msr regardless of guest CPUID bit,
it seemed to be cleaner this way - kvm_put_msrs in qemu seems not to 
check guest CPUID but rather only check that KVM supports this msr,
which will be true regardless of guest's CPUID bit.

What do you think?

Best regards,
	Maxim Levitsky


> 
> If QEMU really can get itself in this situation, we cannot fix this 
> except with KVM_CAP_DISABLE_QUIRKS (a quirk that would accept and ignore 
> host-initiated writes if the CPUID bit is not available) or perhaps with 
> a pr_warn_ratelimited and a quick deprecation cycle, until some time 
> after 6.2.1 is released.
> 
> Hmmm... maybe the issue is actually that KVM *returns* 0 from 
> KVM_GET_MSRS?  And in this case, fixing KVM would also prevent QEMU from 
> sending the bogus KVM_SET_MSRS?
> 
> Thanks,
> 
> Paolo
> 
> > ---
> >   arch/x86/kvm/svm/nested.c | 10 ++++------
> >   arch/x86/kvm/svm/svm.c    |  5 +++--
> >   arch/x86/kvm/svm/svm.h    |  1 +
> >   3 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 1218b5a342fc..a2e2436057dc 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -574,14 +574,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
> >   	vcpu->arch.tsc_offset = kvm_calc_nested_tsc_offset(
> >   			vcpu->arch.l1_tsc_offset,
> >   			svm->nested.ctl.tsc_offset,
> > -			svm->tsc_ratio_msr);
> > +			svm_get_l2_tsc_multiplier(vcpu));
> >   
> >   	svm->vmcb->control.tsc_offset = vcpu->arch.tsc_offset;
> >   
> > -	if (svm->tsc_ratio_msr != kvm_default_tsc_scaling_ratio) {
> > -		WARN_ON(!svm->tsc_scaling_enabled);
> > +	if (svm_get_l2_tsc_multiplier(vcpu) != kvm_default_tsc_scaling_ratio)
> >   		nested_svm_update_tsc_ratio_msr(vcpu);
> > -	}
> >   
> >   	svm->vmcb->control.int_ctl             =
> >   		(svm->nested.ctl.int_ctl & int_ctl_vmcb12_bits) |
> > @@ -867,8 +865,7 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> >   		vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
> >   	}
> >   
> > -	if (svm->tsc_ratio_msr != kvm_default_tsc_scaling_ratio) {
> > -		WARN_ON(!svm->tsc_scaling_enabled);
> > +	if (svm_get_l2_tsc_multiplier(vcpu) != kvm_default_tsc_scaling_ratio) {
> >   		vcpu->arch.tsc_scaling_ratio = vcpu->arch.l1_tsc_scaling_ratio;
> >   		svm_write_tsc_multiplier(vcpu, vcpu->arch.tsc_scaling_ratio);
> >   	}
> > @@ -1264,6 +1261,7 @@ void nested_svm_update_tsc_ratio_msr(struct kvm_vcpu *vcpu)
> >   {
> >   	struct vcpu_svm *svm = to_svm(vcpu);
> >   
> > +	WARN_ON_ONCE(!svm->tsc_scaling_enabled);
> >   	vcpu->arch.tsc_scaling_ratio =
> >   		kvm_calc_nested_tsc_multiplier(vcpu->arch.l1_tsc_scaling_ratio,
> >   					       svm->tsc_ratio_msr);
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 975be872cd1a..5cf6bb5bfd3e 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -911,11 +911,12 @@ static u64 svm_get_l2_tsc_offset(struct kvm_vcpu *vcpu)
> >   	return svm->nested.ctl.tsc_offset;
> >   }
> >   
> > -static u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu)
> > +u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu)
> >   {
> >   	struct vcpu_svm *svm = to_svm(vcpu);
> >   
> > -	return svm->tsc_ratio_msr;
> > +	return svm->tsc_scaling_enabled ? svm->tsc_ratio_msr :
> > +	       kvm_default_tsc_scaling_ratio;
> >   }
> >   
> >   static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index 852b12aee03d..006571dd30df 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -542,6 +542,7 @@ int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
> >   			       bool has_error_code, u32 error_code);
> >   int nested_svm_exit_special(struct vcpu_svm *svm);
> >   void nested_svm_update_tsc_ratio_msr(struct kvm_vcpu *vcpu);
> > +u64 svm_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu);
> >   void svm_write_tsc_multiplier(struct kvm_vcpu *vcpu, u64 multiplier);
> >   void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
> >   				       struct vmcb_control_area *control);

Re: [PATCH] KVM: nSVM: fix nested tsc scaling when not enabled but MSR_AMD64_TSC_RATIO set

[Paolo Bonzini]

On 2/21/22 14:17, Maxim Levitsky wrote:
>> It's not clear how QEMU ends up writing MSR_AMD64_TSC_RATIO_DEFAULT
>> rather than 0, but we clearly have a bug in KVM.  It should not allow
>> writing 0 in the first place if tsc-ratio is not available in the VM.
> 
> Qemu currently (the code is very new so it can be changed) writes the initial value of
> 0 to this msr if tsc scaling is disabled in the guest, or MSR_AMD64_TSC_RATIO_DEFAULT
> if the tsc scaling is enabled.

It's released in 6.2, though, right?

> The guest can change it only when TSC scaling is enabled for it.
> If tsc scaling is not enabled, both guest reads or writes of this MSR get #GP.
> I only allowed qemu writes of this msr because I thought that qemu might
> first set the MSR and then set guest CPUID.
> 
> Also, for example the MSR_IA32_XSS uses the same logic in KVM.
> 
> As for why qemu sets this msr regardless of guest CPUID bit,
> it seemed to be cleaner this way - kvm_put_msrs in qemu seems not to
> check guest CPUID but rather only check that KVM supports this msr,
> which will be true regardless of guest's CPUID bit.

Yes, I agree it's cleaner in QEMU and consistent with others in KVM. 
However the value that is written should be the default one (which is 
usually zero, but not always and not in this case).

Paolo