Re: [PATCH v4 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled

Tom Lendacky
2025-03-10T15:23:47Z

On 3/10/25 01:45, Nikunj A Dadhania wrote:
> Disallow writes to MSR_IA32_TSC for Secure TSC enabled SNP guests. Even if
> KVM attempts to emulate such writes, TSC calculation will ignore the

s/TSC calculation/the TSC calculation performed by hardware/

> TSC_SCALE and TSC_OFFSET present in the VMCB. Instead, it will use

s/present/values present/
s/VMCB. Instead, it will use/VMCB and instead use the/
> GUEST_TSC_SCALE and GUEST_TSC_OFFSET stored in the VMSA.

s/stored/values stored/

>
> Additionally, incorporate a check for protected guest state to allow the
> VMM to initialize the TSC MSR.

I don't see this in the patch.

>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>

With cleanup to the commit message and a formatting nit below:

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
> arch/x86/kvm/svm/svm.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index e65721db1f81..1652848b0240 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3161,6 +3161,25 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>
> svm->tsc_aux = data;
> break;
> + case MSR_IA32_TSC:
> + /*
> + * For Secure TSC enabled VM, do not emulate TSC write as the
> + * TSC calculation ignores the TSC_OFFSET and TSC_SCALE control
> + * fields.
> + *
> + * Guest writes: Record the error and return a #GP.
> + * Host writes are ignored.
> + */
> + if (snp_secure_tsc_enabled(vcpu->kvm)) {
> + if (!msr->host_initiated) {
> + vcpu_unimpl(vcpu, "unimplemented IA32_TSC for Secure TSC\n");
> + return 1;
> + } else
> + return 0;

You need "{" and "}" around this.

Thanks,
Tom

> + }
> +
> + ret = kvm_set_msr_common(vcpu, msr);
> + break;
> case MSR_IA32_DEBUGCTLMSR:
> if (!lbrv) {
> kvm_pr_unimpl_wrmsr(vcpu, ecx, data);