Re: [PATCH 8/8] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Tom Lendacky <thomas.lendacky@amd.com>]

On 9/18/25 05:27, Naveen N Rao (AMD) wrote:
> Add support for configuring the TSC frequency when Secure TSC is enabled
> in SEV-SNP guests through a new "tsc-frequency" property on SEV-SNP
> guest objects, similar to the vCPU-specific property used by regular
> guests and TDX. A new property is needed since SEV-SNP guests require
> the TSC frequency to be specified during early SNP_LAUNCH_START command
> before any vCPUs are created.
> 
> The user-provided TSC frequency is set through KVM_SET_TSC_KHZ before
> issuing KVM_SEV_SNP_LAUNCH_START.

A sample command line like in the previous patches would be consistent.

> 
> Co-developed-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
> Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
> Co-developed-by: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>

It would be nice to have a follow-up patch that cleans up
check_sev_features() so that there aren't a bunch of checks with "if
(sev_snp_enabled() ...". Having all the SNP related checks under one "if"
block might be cleaner as more SNP only features get added. Just a thought.

> ---
>  target/i386/sev.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  qapi/qom.json     |  6 +++++-
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 679bedb63c3a..ef54265f4e46 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -178,6 +178,7 @@ struct SevSnpGuestState {
>      char *id_auth_base64;
>      uint8_t *id_auth;
>      char *host_data;
> +    uint32_t tsc_khz;
>  
>      struct kvm_sev_snp_launch_start kvm_start_conf;
>      struct kvm_sev_snp_launch_finish kvm_finish_conf;
> @@ -536,6 +537,13 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
>                     __func__, sev_features, sev_common->supported_sev_features);
>          return -1;
>      }
> +    if (sev_snp_enabled() && SEV_SNP_GUEST(sev_common)->tsc_khz &&
> +        !(sev_features & SVM_SEV_FEAT_SECURE_TSC)) {
> +        error_setg(errp,
> +                   "%s: TSC frequency can only be set if Secure TSC is enabled",
> +                   __func__);
> +        return -1;
> +    }
>      return 0;
>  }
>  
> @@ -1085,6 +1093,18 @@ sev_snp_launch_start(SevCommonState *sev_common)
>              return 1;
>      }
>  
> +    if (is_sev_feature_set(sev_common, SVM_SEV_FEAT_SECURE_TSC)) {
> +        rc = -EINVAL;
> +        if (kvm_check_extension(kvm_state, KVM_CAP_VM_TSC_CONTROL)) {
> +            rc = kvm_vm_ioctl(kvm_state, KVM_SET_TSC_KHZ, sev_snp_guest->tsc_khz);
> +        }
> +        if (rc < 0) {
> +            error_report("%s: Unable to set Secure TSC frequency to %u kHz ret=%d",
> +                         __func__, sev_snp_guest->tsc_khz, rc);
> +            return 1;
> +        }

It looks like KVM_CAP_VM_TSC_CONTROL is required for Secure TSC. Should
this cap check be part of check_sev_features() then, rather than waiting
until launch start?

And does KVM_SET_TSC_KHZ have to be called if "tsc-frequency" wasn't set?

Thanks,
Tom

> +    }
> +
>      rc = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_START,
>                     start, &fw_error);
>      if (rc < 0) {
> @@ -3127,6 +3147,28 @@ static void sev_snp_guest_set_secure_tsc(Object *obj, bool value, Error **errp)
>      sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_TSC, value);
>  }
>  
> +static void
> +sev_snp_guest_get_tsc_frequency(Object *obj, Visitor *v, const char *name,
> +                                void *opaque, Error **errp)
> +{
> +    uint32_t value = SEV_SNP_GUEST(obj)->tsc_khz * 1000;
> +
> +    visit_type_uint32(v, name, &value, errp);
> +}
> +
> +static void
> +sev_snp_guest_set_tsc_frequency(Object *obj, Visitor *v, const char *name,
> +                                void *opaque, Error **errp)
> +{
> +    uint32_t value;
> +
> +    if (!visit_type_uint32(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    SEV_SNP_GUEST(obj)->tsc_khz = value / 1000;
> +}
> +
>  static void
>  sev_snp_guest_class_init(ObjectClass *oc, const void *data)
>  {
> @@ -3165,6 +3207,9 @@ sev_snp_guest_class_init(ObjectClass *oc, const void *data)
>      object_class_property_add_bool(oc, "secure-tsc",
>                                    sev_snp_guest_get_secure_tsc,
>                                    sev_snp_guest_set_secure_tsc);
> +    object_class_property_add(oc, "tsc-frequency", "uint32",
> +                              sev_snp_guest_get_tsc_frequency,
> +                              sev_snp_guest_set_tsc_frequency, NULL, NULL);
>  }
>  
>  static void
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 52c23e85e349..c01ae70dd43d 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -1103,6 +1103,9 @@
>  # @secure-tsc: enable Secure TSC
>  #     (default: false) (since 10.2)
>  #
> +# @tsc-frequency: set secure TSC frequency.  Only valid if Secure TSC
> +#     is enabled (default: zero) (since 10.2)
> +#
>  # Since: 9.1
>  ##
>  { 'struct': 'SevSnpGuestProperties',
> @@ -1115,7 +1118,8 @@
>              '*author-key-enabled': 'bool',
>              '*host-data': 'str',
>              '*vcek-disabled': 'bool',
> -            '*secure-tsc': 'bool' } }
> +            '*secure-tsc': 'bool',
> +            '*tsc-frequency': 'uint32' } }
>  
>  ##
>  # @TdxGuestProperties: