Messages in this thread | | | From | Paolo Bonzini <> | Subject | Re: [PATCH] selftests/kvm: add set_boot_cpu_id test | Date | Wed, 17 Mar 2021 12:01:46 +0100 |
| |
On 17/03/21 08:44, Emanuele Giuseppe Esposito wrote: > + printf("vcpu executing...\n"); > + vcpu_run(vm, vcpuid); > + printf("vcpu executed\n"); > + > + switch (get_ucall(vm, vcpuid, &uc)) { > + case UCALL_SYNC: > + printf("stage %d sync %ld\n", stage, uc.args[1]); > + TEST_ASSERT(!strcmp((const char *)uc.args[0], "hello") && > + uc.args[1] == stage + 1, > + "Stage %d: Unexpected register values vmexit, got %lx", > + stage + 1, (ulong)uc.args[1]); > + return; > + case UCALL_DONE: > + printf("got done\n");
You can remove the printfs here.
> + return; > + case UCALL_ABORT: > + TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx", (const char *)uc.args[0], > + __FILE__, uc.args[1], uc.args[2], uc.args[3]); > + default: > + TEST_ASSERT(false, "Unexpected exit: %s", > + exit_reason_str(vcpu_state(vm, vcpuid)->exit_reason)); > + } > +} > + > +static void add_x86_vcpu(struct kvm_vm *vm, uint32_t vcpuid, void *code) > +{ > + vm_vcpu_add_default(vm, vcpuid, code); > + vcpu_set_cpuid(vm, vcpuid, kvm_get_supported_cpuid()); > +} > + > +static void run_vm_bsp(uint32_t bsp_vcpu) > +{ > + struct kvm_vm *vm; > + int stage; > + void *vcpu0_code, *vcpu1_code; > + > + vm = create_vm(); > + > + vcpu0_code = guest_bsp_vcpu; > + vcpu1_code = guest_not_bsp_vcpu; > + > + if (bsp_vcpu == VCPU_ID1) { > + vcpu0_code = guest_not_bsp_vcpu; > + vcpu1_code = guest_bsp_vcpu; > + > + vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *) VCPU_ID1); > + } > + > + add_x86_vcpu(vm, VCPU_ID0, vcpu0_code); > + add_x86_vcpu(vm, VCPU_ID1, vcpu1_code); > + > + for (stage = 0; stage < 2; stage++) { > + run_vcpu(vm, VCPU_ID0, stage); > + run_vcpu(vm, VCPU_ID1, stage); > + }
These are just stylistic nits, but:
1) you could pass a bool to add_x86_vcpu, like
add_x86_vcpu(vm, VCPU_ID0, bsp_vcpu == VCPU_ID0); add_x86_vcpu(vm, VCPU_ID1, bsp_vcpu == VCPU_ID1);
instead of having the vcpu0_code and vcpu1_code pointers.
2) you could move the for loop inside run_vcpu. This way you can assert that you get UCALL_DONE when stage == 2.
> + kvm_vm_free(vm); > +} > + > +int main(int argc, char *argv[]) > +{ > + if (!kvm_check_cap(KVM_CAP_SET_BOOT_CPU_ID)) { > + print_skip("set_boot_cpu_id not available"); > + return 0; > + } > + > + run_vm_bsp(VCPU_ID0); > + run_vm_bsp(VCPU_ID1); > + run_vm_bsp(VCPU_ID0);
Looks good! Just another small change: since this third test is not adding much, you could instead invoke KVM_SET_BOOT_CPU_ID(VCPU_ID1) *after* the creation of the vCPUs instead of before. This should fail with -EBUSY, and vcpu 0 will remain the BSP.
Paolo
| |