Re: [PATCH] KVM: x86: Block TSC multiplier writes for protected guest TSC

Re: [PATCH] KVM: x86: Block TSC multiplier writes for protected guest TSC

[Huang, Kai]

On Tue, 2026-05-12 at 07:18 -0400, Jun Miao wrote:
> Commit d5a4e408e69b ("KVM: x86: Add infrastructure for secure TSC")

The hash isn't correct.  It should be adafea110600 AFAICT.

> added the guest_tsc_protected flag to prevent KVM from changing the
> TSC offset/multiplier of vCPUs whose TSC is managed by a confidential
> computing module (e.g. TDX, SEV-SNP Secure TSC).  However only the TSC
> offset write path was guarded; kvm_vcpu_write_tsc_multiplier() was
> left unprotected.
> 
> As a result, userspace can still change the TSC scaling ratio of a
> TDX vCPU via the KVM_SET_TSC_KHZ ioctl path:
> 
>   KVM_SET_TSC_KHZ
>     -> kvm_arch_vcpu_ioctl()
>     -> kvm_set_tsc_khz()
>     -> set_tsc_khz()
>     -> kvm_vcpu_write_tsc_multiplier()   <-- not guarded

Commit b24bbb534c2d ("KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC
protected guest") already added code to bail out early in kvm_arch_vcpu_ioctl()
for TSC protected vCPU.

> 
> and similarly during kvm_arch_vcpu_create() -> kvm_set_tsc_khz()
> which can reset the multiplier to default_tsc_scaling_ratio.

This is more like initialization of KVM's internal vCPU data structure based on
the default TSC (which is configured to the TD before creating any vCPU [*]), so
I think it is OK from functionality perspective.  Note vt_write_tsc_multiplier()
simply does nothing for TD so it's safe.  But since it is called after vcpu-
>arch.guest_tsc_protected is set, I agree it's kinda not nice.

[*] TDX's flow is: 1) userspace uses KVM_SET_TSC_KHZ vm-ioctl to configure the
default TSC; 2) userspace invokes KVM_TDX_INIT_VM, which configures the TD's TSC
using the default TSC; 3) userspace creates vCPUs for the TD, in which step vcpu
is marked as TSC protected.

After 3), both KVM_SET_TSC_KHZ vm-ioctl and vcpu-ioctl will fail to change the
TSC.

Theoretically, AFAICT there's a chance that if 1) gets called again with a
different TSC "after 2), but before 3)", KVM will have a wrong default TSC from
what is configured the the TD.  But no sane userspace should ever do that.

> 
> Make kvm_vcpu_write_tsc_multiplier() symmetric with
> kvm_vcpu_write_tsc_offset() by skipping the update when
> guest_tsc_protected is set.  This single chokepoint covers all
> existing callers (set_tsc_khz() in both ioctl and vCPU create paths).

Fine with making kvm_vcpu_write_tsc_multiplier() symmetric to TSC offset,
though.

> 
> Fixes: d5a4e408e69b ("KVM: x86: Add infrastructure for secure TSC")
> Signed-off-by: Jun Miao <jun.miao@intel.com>
> ---
>  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 0a1b63c63d1a..e935fe33d9c2 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2736,6 +2736,9 @@ static void kvm_vcpu_write_tsc_offset(struct kvm_vcpu *vcpu, u64 l1_offset)
>  
>  static void kvm_vcpu_write_tsc_multiplier(struct kvm_vcpu *vcpu, u64 l1_multiplier)
>  {
> +	if (vcpu->arch.guest_tsc_protected)
> +		return;
> +
>  	vcpu->arch.l1_tsc_scaling_ratio = l1_multiplier;
>  
>  	/* Userspace is changing the multiplier while L2 is active */

RE: [PATCH] KVM: x86: Block TSC multiplier writes for protected guest TSC

[Miao, Jun]

>On Tue, 2026-05-12 at 07:18 -0400, Jun Miao wrote:
>> Commit d5a4e408e69b ("KVM: x86: Add infrastructure for secure TSC")
>
>The hash isn't correct.  It should be adafea110600 AFAICT.
>
>> added the guest_tsc_protected flag to prevent KVM from changing the
>> TSC offset/multiplier of vCPUs whose TSC is managed by a confidential
>> computing module (e.g. TDX, SEV-SNP Secure TSC).  However only the TSC
>> offset write path was guarded; kvm_vcpu_write_tsc_multiplier() was
>> left unprotected.
>>
>> As a result, userspace can still change the TSC scaling ratio of a TDX
>> vCPU via the KVM_SET_TSC_KHZ ioctl path:
>>
>>   KVM_SET_TSC_KHZ
>>     -> kvm_arch_vcpu_ioctl()
>>     -> kvm_set_tsc_khz()
>>     -> set_tsc_khz()
>>     -> kvm_vcpu_write_tsc_multiplier()   <-- not guarded
>
>Commit b24bbb534c2d ("KVM: x86: Reject KVM_SET_TSC_KHZ vCPU ioctl for TSC
>protected guest") already added code to bail out early in kvm_arch_vcpu_ioctl()
>for TSC protected vCPU.

Indeed,  It's better to avoid it earlier / upfront. This move of mine was a bit unnecessary.
Please ignore this revision. Thank you for pointing that out.

>>
>> and similarly during kvm_arch_vcpu_create() -> kvm_set_tsc_khz() which
>> can reset the multiplier to default_tsc_scaling_ratio.
>
>This is more like initialization of KVM's internal vCPU data structure based on the
>default TSC (which is configured to the TD before creating any vCPU [*]), so I think
>it is OK from functionality perspective.  Note vt_write_tsc_multiplier() simply does
>nothing for TD so it's safe.  But since it is called after vcpu-
>>arch.guest_tsc_protected is set, I agree it's kinda not nice.
>
>[*] TDX's flow is: 1) userspace uses KVM_SET_TSC_KHZ vm-ioctl to configure the
>default TSC; 2) userspace invokes KVM_TDX_INIT_VM, which configures the TD's
>TSC using the default TSC; 3) userspace creates vCPUs for the TD, in which step
>vcpu is marked as TSC protected.
>
>After 3), both KVM_SET_TSC_KHZ vm-ioctl and vcpu-ioctl will fail to change the
>TSC.
>
>Theoretically, AFAICT there's a chance that if 1) gets called again with a different
>TSC "after 2), but before 3)", KVM will have a wrong default TSC from what is
>configured the the TD.  But no sane userspace should ever do that.
>
>>
>> Make kvm_vcpu_write_tsc_multiplier() symmetric with
>> kvm_vcpu_write_tsc_offset() by skipping the update when
>> guest_tsc_protected is set.  This single chokepoint covers all
>> existing callers (set_tsc_khz() in both ioctl and vCPU create paths).
>
>Fine with making kvm_vcpu_write_tsc_multiplier() symmetric to TSC offset,
>though.
>
>>
>> Fixes: d5a4e408e69b ("KVM: x86: Add infrastructure for secure TSC")
>> Signed-off-by: Jun Miao <jun.miao@intel.com>
>> ---
>>  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
>> 0a1b63c63d1a..e935fe33d9c2 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -2736,6 +2736,9 @@ static void kvm_vcpu_write_tsc_offset(struct
>> kvm_vcpu *vcpu, u64 l1_offset)
>>
>>  static void kvm_vcpu_write_tsc_multiplier(struct kvm_vcpu *vcpu, u64
>> l1_multiplier)  {
>> +	if (vcpu->arch.guest_tsc_protected)
>> +		return;
>> +
>>  	vcpu->arch.l1_tsc_scaling_ratio = l1_multiplier;
>>
>>  	/* Userspace is changing the multiplier while L2 is active */

[PATCH] KVM: x86: Block TSC multiplier writes for protected guest TSC

[Jun Miao]

Commit d5a4e408e69b ("KVM: x86: Add infrastructure for secure TSC")
added the guest_tsc_protected flag to prevent KVM from changing the
TSC offset/multiplier of vCPUs whose TSC is managed by a confidential
computing module (e.g. TDX, SEV-SNP Secure TSC).  However only the TSC
offset write path was guarded; kvm_vcpu_write_tsc_multiplier() was
left unprotected.

As a result, userspace can still change the TSC scaling ratio of a
TDX vCPU via the KVM_SET_TSC_KHZ ioctl path:

  KVM_SET_TSC_KHZ
    -> kvm_arch_vcpu_ioctl()
    -> kvm_set_tsc_khz()
    -> set_tsc_khz()
    -> kvm_vcpu_write_tsc_multiplier()   <-- not guarded

and similarly during kvm_arch_vcpu_create() -> kvm_set_tsc_khz()
which can reset the multiplier to default_tsc_scaling_ratio.

Make kvm_vcpu_write_tsc_multiplier() symmetric with
kvm_vcpu_write_tsc_offset() by skipping the update when
guest_tsc_protected is set.  This single chokepoint covers all
existing callers (set_tsc_khz() in both ioctl and vCPU create paths).

Fixes: d5a4e408e69b ("KVM: x86: Add infrastructure for secure TSC")
Signed-off-by: Jun Miao <jun.miao@intel.com>
---
 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 0a1b63c63d1a..e935fe33d9c2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2736,6 +2736,9 @@ static void kvm_vcpu_write_tsc_offset(struct kvm_vcpu *vcpu, u64 l1_offset)
 
 static void kvm_vcpu_write_tsc_multiplier(struct kvm_vcpu *vcpu, u64 l1_multiplier)
 {
+	if (vcpu->arch.guest_tsc_protected)
+		return;
+
 	vcpu->arch.l1_tsc_scaling_ratio = l1_multiplier;
 
 	/* Userspace is changing the multiplier while L2 is active */
-- 
2.43.0