Re: [PATCH v8 2/2] KVM: SVM: Enable Secure TSC for SNP guests

[Sean Christopherson <seanjc@google.com>]

On Tue, Jul 08, 2025, Kai Huang wrote:
> 
> > > > -		svm->vcpu.arch.guest_state_protected = true;
> > > > +		vcpu->arch.guest_state_protected = true;
> > > > +		vcpu->arch.guest_tsc_protected = snp_secure_tsc_enabled(kvm);
> > > > +
> > > 
> > > + Xiaoyao.
> > > 
> > > The KVM_SET_TSC_KHZ can also be a vCPU ioctl (in fact, the support of VM
> > > ioctl of it was added later).  I am wondering whether we should reject
> > > this vCPU ioctl for TSC protected guests, like:

Yes, we definitely should.  And if it's not too ugly, KVM should also reject the
VM-scoped KVM_SET_TSC_KHZ if vCPUs have been created with guest_tsc_protected=true.
(or maybe we could get greedy and try to disallow KVM_SET_TSC_KHZ if vCPUs have
been created for any VM shape?)

> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 2806f7104295..699ca5e74bba 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -6186,6 +6186,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> > >                  u32 user_tsc_khz;
> > >   
> > >                  r = -EINVAL;
> > > +
> > > +               if (vcpu->arch.guest_tsc_protected)
> > > +                       goto out;
> > > +
> > >                  user_tsc_khz = (u32)arg;
> > >   
> > >                  if (kvm_caps.has_tsc_control &&
> > 
> > It seems to need to be opt-in since it changes the ABI somehow. E.g., it 
> > at least works before when the VMM calls KVM_SET_TSC_KHZ at vcpu with 
> > the same value passed to KVM_SET_TSC_KHZ at vm. But with the above 
> > change, it would fail.
> > 
> > Well, in reality, it's OK for QEMU since QEMU explicitly doesn't call 
> > KVM_SET_TSC_KHZ at vcpu for TDX VMs. But I'm not sure about the impact 
> > on other VMMs. Considering KVM TDX support just gets in from v6.16-rc1, 
> > maybe it doesn't have real impact for other VMMs as well?

6.16 hasn't officially release yet, so any impact to userspace is irrelevant,
i.e. there is no established ABI at this time.

Can someone send a proper patch?