| Message ID | 20250217102237.16434-5-nikunj@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable Secure TSC for SEV-SNP | expand |
On 2/17/25 04:22, 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 > TSC_SCALE and TSC_OFFSET present in the VMCB. Instead, it will use > GUEST_TSC_SCALE and GUEST_TSC_OFFSET stored in the VMSA. > > Additionally, incorporate a check for protected guest state to allow the > VMM to initialize the TSC MSR. > > Signed-off-by: Nikunj A Dadhania <nikunj@amd.com> > --- > arch/x86/kvm/svm/svm.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 93cf508f983c..7463466f5126 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -3161,6 +3161,20 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) > > svm->tsc_aux = data; > break; > + case MSR_IA32_TSC: > + /* > + * If Secure TSC is enabled, do not emulate TSC write as TSC calculation > + * ignores the TSC_OFFSET and TSC_SCALE control fields, record the error > + * and return a #GP. Allow the TSC to be initialized until the guest state > + * is protected to prevent unexpected VMM errors. > + */ > + if (vcpu->arch.guest_state_protected && snp_secure_tsc_enabled(vcpu->kvm)) { I'm not sure if it matters, but do we need to differentiate between guest and host write in this situation at all in regards to the message or return code? > + vcpu_unimpl(vcpu, "unimplemented IA32_TSC for secure tsc\n"); s/secure tsc/Secure TSC/ ? Thanks, Tom > + return 1; > + } > + > + ret = kvm_set_msr_common(vcpu, msr); > + break; > case MSR_IA32_DEBUGCTLMSR: > if (!lbrv) { > kvm_pr_unimpl_wrmsr(vcpu, ecx, data);
Tom Lendacky <thomas.lendacky@amd.com> writes: > On 2/17/25 04:22, Nikunj A Dadhania wrote: >> @@ -3161,6 +3161,20 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) >> >> svm->tsc_aux = data; >> break; >> + case MSR_IA32_TSC: >> + /* >> + * If Secure TSC is enabled, do not emulate TSC write as TSC calculation >> + * ignores the TSC_OFFSET and TSC_SCALE control fields, record the error >> + * and return a #GP. Allow the TSC to be initialized until the guest state >> + * is protected to prevent unexpected VMM errors. >> + */ >> + if (vcpu->arch.guest_state_protected && snp_secure_tsc_enabled(vcpu->kvm)) { > > I'm not sure if it matters, but do we need to differentiate between > guest and host write in this situation at all in regards to the message > or return code? > Yes, I think we can have something like the below: + 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; + } + + ret = kvm_set_msr_common(vcpu, msr); + break; >> + vcpu_unimpl(vcpu, "unimplemented IA32_TSC for secure tsc\n"); > > s/secure tsc/Secure TSC/ ? > Ack, Thanks Nikunj
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 93cf508f983c..7463466f5126 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -3161,6 +3161,20 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) svm->tsc_aux = data; break; + case MSR_IA32_TSC: + /* + * If Secure TSC is enabled, do not emulate TSC write as TSC calculation + * ignores the TSC_OFFSET and TSC_SCALE control fields, record the error + * and return a #GP. Allow the TSC to be initialized until the guest state + * is protected to prevent unexpected VMM errors. + */ + if (vcpu->arch.guest_state_protected && snp_secure_tsc_enabled(vcpu->kvm)) { + vcpu_unimpl(vcpu, "unimplemented IA32_TSC for secure tsc\n"); + return 1; + } + + ret = kvm_set_msr_common(vcpu, msr); + break; case MSR_IA32_DEBUGCTLMSR: if (!lbrv) { kvm_pr_unimpl_wrmsr(vcpu, ecx, data);
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 TSC_SCALE and TSC_OFFSET present in the VMCB. Instead, it will use GUEST_TSC_SCALE and GUEST_TSC_OFFSET stored in the VMSA. Additionally, incorporate a check for protected guest state to allow the VMM to initialize the TSC MSR. Signed-off-by: Nikunj A Dadhania <nikunj@amd.com> --- arch/x86/kvm/svm/svm.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)