Re: [PATCH v2 21/58] i386/tdx: Implement user specified tsc frequency

[Daniel P . Berrangé]

On Fri, Aug 18, 2023 at 05:50:04AM -0400, Xiaoyao Li wrote:
> Reuse "-cpu,tsc-frequency=" to get user wanted tsc frequency and call VM
> scope VM_SET_TSC_KHZ to set the tsc frequency of TD before KVM_TDX_INIT_VM.
> 
> Besides, sanity check the tsc frequency to be in the legal range and
> legal granularity (required by TDX module).
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> Changes from RFC v4:
>   - Use VM scope VM_SET_TSC_KHZ to set the TSC frequency of TD since KVM
>     side drop the @tsc_khz field in struct kvm_tdx_init_vm
> ---
>  target/i386/kvm/kvm.c |  9 +++++++++
>  target/i386/kvm/tdx.c | 24 ++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index d51067fdc12a..4a146bc42f63 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -859,6 +859,15 @@ static int kvm_arch_set_tsc_khz(CPUState *cs)
>      int r, cur_freq;
>      bool set_ioctl = false;
>  
> +    /*
> +     * TSC of TD vcpu is immutable, it cannot be set/changed via vcpu scope
> +     * VM_SET_TSC_KHZ, but only be initialized via VM scope VM_SET_TSC_KHZ
> +     * before ioctl KVM_TDX_INIT_VM in tdx_pre_create_vcpu()
> +     */
> +    if (is_tdx_vm()) {
> +        return 0;
> +    }
> +
>      if (!env->tsc_khz) {
>          return 0;
>      }
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 33d015a08c34..a72badfbfd65 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -32,6 +32,9 @@
>                                       (1U << KVM_FEATURE_PV_SCHED_YIELD) | \
>                                       (1U << KVM_FEATURE_MSI_EXT_DEST_ID))
>  
> +#define TDX_MIN_TSC_FREQUENCY_KHZ   (100 * 1000)
> +#define TDX_MAX_TSC_FREQUENCY_KHZ   (10 * 1000 * 1000)
> +
>  #define TDX_TD_ATTRIBUTES_DEBUG             BIT_ULL(0)
>  #define TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE   BIT_ULL(28)
>  #define TDX_TD_ATTRIBUTES_PKS               BIT_ULL(30)
> @@ -513,6 +516,27 @@ int tdx_pre_create_vcpu(CPUState *cpu)
>          goto out_free;
>      }
>  
> +    r = -EINVAL;
> +    if (env->tsc_khz && (env->tsc_khz < TDX_MIN_TSC_FREQUENCY_KHZ ||
> +                         env->tsc_khz > TDX_MAX_TSC_FREQUENCY_KHZ)) {
> +        error_report("Invalid TSC %ld KHz, must specify cpu_frequency 
> between [%d, %d] kHz",
> +                      env->tsc_khz, TDX_MIN_TSC_FREQUENCY_KHZ,
> +                      TDX_MAX_TSC_FREQUENCY_KHZ);
> +        goto out;
> +    }
> +
> +    if (env->tsc_khz % (25 * 1000)) {
> +        error_report("Invalid TSC %ld KHz, it must be multiple of 25MHz", 
> env->tsc_khz);
> +        goto out;
> +    }
> +
> +    /* it's safe even env->tsc_khz is 0. KVM uses host's tsc_khz in this 
> case */
> +    r = kvm_vm_ioctl(kvm_state, KVM_SET_TSC_KHZ, env->tsc_khz);
> +    if (r < 0) {
> +        error_report("Unable to set TSC frequency to %" PRId64 " kHz", 
> env->tsc_khz);
> +        goto out;
> +    }

error_setg(errp, ....) in all of these cases.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|