On Thu, 2021-05-06 at 13:32 +0200, Paolo Bonzini wrote: > On 06/05/21 12:32, ilstam@xxxxxxxxxxx wrote: > > + if (vmcs12->cpu_based_vm_exec_control & > > CPU_BASED_USE_TSC_OFFSETTING) { > > + if (vmcs12->secondary_vm_exec_control & > > SECONDARY_EXEC_TSC_SCALING) { > > + vcpu->arch.tsc_offset = > > kvm_compute_02_tsc_offset( > > + vcpu->arch.l1_tsc_offset, > > + vmcs12->tsc_multiplier, > > + vmcs12->tsc_offset); > > + > > + vcpu->arch.tsc_scaling_ratio = > > mul_u64_u64_shr( > > + vcpu->arch.tsc_scaling_ratio, > > + vmcs12->tsc_multiplier, > > + kvm_tsc_scaling_ratio_frac_bit > > s); > > + } else { > > + vcpu->arch.tsc_offset += vmcs12->tsc_offset; > > + } > > The computation of vcpu->arch.tsc_offset is (not coincidentially) the > same that appears in patch 6 > > + (vmcs12->cpu_based_vm_exec_control & > CPU_BASED_USE_TSC_OFFSETTING)) { > + if (vmcs12->secondary_vm_exec_control & > SECONDARY_EXEC_TSC_SCALING) { > + cur_offset = kvm_compute_02_tsc_offset( > + l1_offset, > + vmcs12->tsc_multiplier, > + vmcs12->tsc_offset); > + } else { > + cur_offset = l1_offset + vmcs12->tsc_offset; > > So I think you should just pass vmcs12 and the L1 offset to > kvm_compute_02_tsc_offset, and let it handle both cases (and possibly > even set vcpu->arch.tsc_scaling_ratio in the same function). > > Paolo > That was my thinking initially too. However, kvm_compute_02_tsc_offset is defined in x86.c which is vmx-agnostic and hence 'struct vmcs12' is not defined there. The way it is now, that function can be re-used by svm code in case we add amd support later. I could try to define a wrapper in vmx.c instead, that calls kvm_compute_02_tsc_offset and move all the code there. However, I think this requires many more changes to the vmx and svm write_l1_tsc_offset functions and to their caller. I am looking at it now. Ilias