On 7/8/2025 8:04 PM, Sean Christopherson wrote: > On Tue, Jul 08, 2025, Kai Huang wrote: >>>> Even some bug results in the default_tsc_khz being 0, will the >>>> SNP_LAUNCH_START command catch this and return error? >>> >>> No, that is an invalid configuration, desired_tsc_khz is set to 0 when >>> SecureTSC is disabled. If SecureTSC is enabled, desired_tsc_khz should >>> have correct value. >> >> So it's an invalid configuration that when Secure TSC is enabled and >> desired_tsc_khz is 0. Assuming the SNP_LAUNCH_START will return an error >> if such configuration is used, wouldn't it be simpler if you remove the >> above check and depend on the SNP_LAUNCH_START command to catch the >> invalid configuration? > > Support for secure TSC should depend on tsc_khz being non-zero. That way it'll > be impossible for arch.default_tsc_khz to be zero at runtime. Then KVM can WARN > on arch.default_tsc_khz being zero during SNP_LAUNCH_START. Sure. > > I.e. > > if (sev_snp_enabled && tsc_khz && > cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC)) > sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC; Yes, this is better. Regards Nikunj
