Re: [PATCH 4/8] KVM: VMX: Adjust the TSC-related VMCS fields on L2 entry and exit

["Stamatis, Ilias" <ilstam@amazon.com>]

On Thu, 2021-05-06 at 13:32 +0200, Paolo Bonzini wrote:
> On 06/05/21 12:32, ilstam@mailbox.org 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