On Tue, 2025-07-08 at 07:34 -0700, 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. > > 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; Yeah looks good.
