Re: [PATCH v2 093/144] KVM: selftests: Track kvm_vcpu object in tsc_scaling_sync

Andrew Jones
2022-06-08T16:14:01Z

On Fri, Jun 03, 2022 at 12:42:40AM +0000, Sean Christopherson wrote:
> Track the added 'struct kvm_vcpu' object in tsc_scaling_sync instead of
> relying purely on the VM + vcpu_id combination. Ideally, the test
> wouldn't need to manually manage vCPUs, but the need to invoke a per-VM
> ioctl before creating vCPUs is not handled by the selftests framework,
> at least not yet...
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> .../selftests/kvm/x86_64/tsc_scaling_sync.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/tsc_scaling_sync.c b/tools/testing/selftests/kvm/x86_64/tsc_scaling_sync.c
> index f0083d8cfe98..b7cd5c47fc53 100644
> --- a/tools/testing/selftests/kvm/x86_64/tsc_scaling_sync.c
> +++ b/tools/testing/selftests/kvm/x86_64/tsc_scaling_sync.c
> @@ -46,38 +46,41 @@ static void guest_code(void)
>
> static void *run_vcpu(void *_cpu_nr)
> {
> - unsigned long cpu = (unsigned long)_cpu_nr;
> + unsigned long vcpu_id = (unsigned long)_cpu_nr;
> unsigned long failures = 0;
> static bool first_cpu_done;
> + struct kvm_vcpu *vcpu;
>
> /* The kernel is fine, but vm_vcpu_add_default() needs locking */
> pthread_spin_lock(&create_lock);
>
> - vm_vcpu_add_default(vm, cpu, guest_code);
> + vm_vcpu_add_default(vm, vcpu_id, guest_code);
> + vcpu = vcpu_get(vm, vcpu_id);
>
> if (!first_cpu_done) {
> first_cpu_done = true;
> - vcpu_set_msr(vm, cpu, MSR_IA32_TSC, TEST_TSC_OFFSET);
> + vcpu_set_msr(vm, vcpu->id, MSR_IA32_TSC, TEST_TSC_OFFSET);
> }
>
> pthread_spin_unlock(&create_lock);
>
> for (;;) {
> - volatile struct kvm_run *run = vcpu_state(vm, cpu);
> + volatile struct kvm_run *run = vcpu->run;
> struct ucall uc;
>
> - vcpu_run(vm, cpu);
> + vcpu_run(vm, vcpu->id);
> TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
> "Got exit_reason other than KVM_EXIT_IO: %u (%s)\n",
> run->exit_reason,
> exit_reason_str(run->exit_reason));
>
> - switch (get_ucall(vm, cpu, &uc)) {
> + switch (get_ucall(vm, vcpu->id, &uc)) {

The two changes above show that this file had some space vs. tab issues.
I just checked and these two lines weren't the only ones, so I guess we
can add cleaning up whitespace of x86_64/tsc_scaling_sync.c to the rainy
day TODO.

Thanks,
drew

> case UCALL_DONE:
> goto out;
>
> case UCALL_SYNC:
> - printf("Guest %ld sync %lx %lx %ld\n", cpu, uc.args[2], uc.args[3], uc.args[2] - uc.args[3]);
> + printf("Guest %d sync %lx %lx %ld\n", vcpu->id,
> + uc.args[2], uc.args[3], uc.args[2] - uc.args[3]);
> failures++;
> break;
>
> --
> 2.36.1.255.ge46751e96f-goog
>