Re: [PATCH 2/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPU has been created

[Chao Gao <chao.gao@intel.com>]

>AFAICT the actual updating of kvm->arch.default_tsc_khz needs to be in the
>kvm->lock mutex too.

Yep.

>Please let me know if you found any issue?
>
>diff --git a/Documentation/virt/kvm/api.rst
>b/Documentation/virt/kvm/api.rst
>index 43ed57e048a8..86ea1e2b2737 100644
>--- a/Documentation/virt/kvm/api.rst
>+++ b/Documentation/virt/kvm/api.rst
>@@ -2006,7 +2006,7 @@ frequency is KHz.
>
> If the KVM_CAP_VM_TSC_CONTROL capability is advertised, this can also
> be used as a vm ioctl to set the initial tsc frequency of subsequently
>-created vCPUs.
>+created vCPUs.  It must be called before any vCPU is created.

		^^ remove one space here.

"must be" sounds like a mandatory action, but IIUC the vm ioctl is optional for
non-CC VMs. I'm not sure if this is just a problem of my interpretation.

To make the API documentation super clear, how about:

If the KVM_CAP_VM_TSC_CONTROL capability is advertised, this can also
be used as a vm ioctl to set the initial tsc frequency of vCPUs before
any vCPU is created. Attempting to call this vm ioctl after vCPU creation
will return an EINVAL error.

>
> 4.56 KVM_GET_TSC_KHZ
> --------------------
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 2806f7104295..4051c0cacb92 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -7199,9 +7199,12 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned
>int ioctl, unsigned long arg)
>                if (user_tsc_khz == 0)
>                        user_tsc_khz = tsc_khz;
>
>-               WRITE_ONCE(kvm->arch.default_tsc_khz, user_tsc_khz);
>-               r = 0;
>-
>+               mutex_lock(&kvm->lock);
>+               if (!kvm->created_vcpus) {
>+                       WRITE_ONCE(kvm->arch.default_tsc_khz,
>user_tsc_khz);
>+                       r = 0;
>+               }
>+               mutex_unlock(&kvm->lock);

LGTM.

>                goto out;
>        }
>        case KVM_GET_TSC_KHZ: {