| Message ID | 20210512150945.4591-9-ilstam@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Implement nested TSC scaling | expand |
On Wed, May 12, 2021, Ilias Stamatis wrote: > Now that nested TSC scaling is supported we need to calculate the > correct 02 values for both the offset and the multiplier using the > corresponding functions. On L2's exit the L1 values are restored. > > Signed-off-by: Ilias Stamatis <ilstam@amazon.com> > --- > arch/x86/kvm/vmx/nested.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 6058a65a6ede..f1dff1ebaccb 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -3354,8 +3354,9 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, > } > > enter_guest_mode(vcpu); > - if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) > - vcpu->arch.tsc_offset += vmcs12->tsc_offset; > + > + kvm_set_02_tsc_offset(vcpu); > + kvm_set_02_tsc_multiplier(vcpu); Please move this code into prepare_vmcs02() to co-locate it with the relevant vmcs02 logic. If there's something in prepare_vmcs02() that consumes vcpu->arch.tsc_offset() other than the obvious VMWRITE, I vote to move things around to fix that rather than create this weird split. > if (prepare_vmcs02(vcpu, vmcs12, &entry_failure_code)) { > exit_reason.basic = EXIT_REASON_INVALID_STATE; > @@ -4463,8 +4464,12 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason, > if (nested_cpu_has_preemption_timer(vmcs12)) > hrtimer_cancel(&to_vmx(vcpu)->nested.preemption_timer); > > - if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) > - vcpu->arch.tsc_offset -= vmcs12->tsc_offset; > + if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) { > + vcpu->arch.tsc_offset = vcpu->arch.l1_tsc_offset; > + > + if (vmcs12->secondary_vm_exec_control & SECONDARY_EXEC_TSC_SCALING) > + vcpu->arch.tsc_scaling_ratio = vcpu->arch.l1_tsc_scaling_ratio; > + } > > if (likely(!vmx->fail)) { > sync_vmcs02_to_vmcs12(vcpu, vmcs12); > -- > 2.17.1 >
On Wed, 2021-05-19 at 00:07 +0000, Sean Christopherson wrote: > On Wed, May 12, 2021, Ilias Stamatis wrote: > > Now that nested TSC scaling is supported we need to calculate the > > correct 02 values for both the offset and the multiplier using the > > corresponding functions. On L2's exit the L1 values are restored. > > > > Signed-off-by: Ilias Stamatis <ilstam@amazon.com> > > --- > > arch/x86/kvm/vmx/nested.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > index 6058a65a6ede..f1dff1ebaccb 100644 > > --- a/arch/x86/kvm/vmx/nested.c > > +++ b/arch/x86/kvm/vmx/nested.c > > @@ -3354,8 +3354,9 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, > > } > > > > enter_guest_mode(vcpu); > > - if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) > > - vcpu->arch.tsc_offset += vmcs12->tsc_offset; > > + > > + kvm_set_02_tsc_offset(vcpu); > > + kvm_set_02_tsc_multiplier(vcpu); > > Please move this code into prepare_vmcs02() to co-locate it with the relevant > vmcs02 logic. If there's something in prepare_vmcs02() that consumes > vcpu->arch.tsc_offset() other than the obvious VMWRITE, I vote to move things > around to fix that rather than create this weird split. > Agreed. It makes more sense. > > if (prepare_vmcs02(vcpu, vmcs12, &entry_failure_code)) { > > exit_reason.basic = EXIT_REASON_INVALID_STATE; > > @@ -4463,8 +4464,12 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason, > > if (nested_cpu_has_preemption_timer(vmcs12)) > > hrtimer_cancel(&to_vmx(vcpu)->nested.preemption_timer); > > > > - if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) > > - vcpu->arch.tsc_offset -= vmcs12->tsc_offset; > > + if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) { > > + vcpu->arch.tsc_offset = vcpu->arch.l1_tsc_offset; > > + > > + if (vmcs12->secondary_vm_exec_control & SECONDARY_EXEC_TSC_SCALING) > > + vcpu->arch.tsc_scaling_ratio = vcpu->arch.l1_tsc_scaling_ratio; > > + } I guess these need to stay where they are though. And thinking about it the two if conditions are unnecessary really. Thanks for the reviews! Ilias > > > > if (likely(!vmx->fail)) { > > sync_vmcs02_to_vmcs12(vcpu, vmcs12); > > -- > > 2.17.1 > >
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 6058a65a6ede..f1dff1ebaccb 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -3354,8 +3354,9 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, } enter_guest_mode(vcpu); - if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) - vcpu->arch.tsc_offset += vmcs12->tsc_offset; + + kvm_set_02_tsc_offset(vcpu); + kvm_set_02_tsc_multiplier(vcpu); if (prepare_vmcs02(vcpu, vmcs12, &entry_failure_code)) { exit_reason.basic = EXIT_REASON_INVALID_STATE; @@ -4463,8 +4464,12 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason, if (nested_cpu_has_preemption_timer(vmcs12)) hrtimer_cancel(&to_vmx(vcpu)->nested.preemption_timer); - if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) - vcpu->arch.tsc_offset -= vmcs12->tsc_offset; + if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING) { + vcpu->arch.tsc_offset = vcpu->arch.l1_tsc_offset; + + if (vmcs12->secondary_vm_exec_control & SECONDARY_EXEC_TSC_SCALING) + vcpu->arch.tsc_scaling_ratio = vcpu->arch.l1_tsc_scaling_ratio; + } if (likely(!vmx->fail)) { sync_vmcs02_to_vmcs12(vcpu, vmcs12);
Now that nested TSC scaling is supported we need to calculate the correct 02 values for both the offset and the multiplier using the corresponding functions. On L2's exit the L1 values are restored. Signed-off-by: Ilias Stamatis <ilstam@amazon.com> --- arch/x86/kvm/vmx/nested.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)