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