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
