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@xxxxxxxxx>
>> ---
>>  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 */