On Fri, Jan 24, 2020 at 10:03:08AM +0100, Paolo Bonzini wrote:
CCing Marc, Conny and Christian (plus Thomas and Drew who were already in the list) for review.
Thanks,
Paolo
On 23/01/20 19:04, Ben Gardon wrote:
Currently vcpu_args_set is only implemented for x86. This makes writing tests with multiple vCPUs difficult as each guest vCPU must either a.) do the same thing or b.) derive some kind of unique token from it's registers or the architecture. To simplify the process of writing tests with multiple vCPUs for s390 and aarch64, add set args functions for those architectures.
It'd be nice to keep the separate architecture changes in separate patches. Otherwise I can't really give an r-b.
Signed-off-by: Ben Gardon bgardon@google.com
.../selftests/kvm/lib/aarch64/processor.c | 33 +++++++++++++++++ .../selftests/kvm/lib/s390x/processor.c | 35 +++++++++++++++++++ 2 files changed, 68 insertions(+)
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c index 86036a59a668e..a2ff90a75f326 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c @@ -333,3 +333,36 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code) { aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code); }
+/* VM VCPU Args Set
- Input Args:
- vm - Virtual Machine
- vcpuid - VCPU ID
- num - number of arguments
- ... - arguments, each of type uint64_t
- Output Args: None
- Return: None
- Sets the first num function input arguments to the values
- given as variable args. Each of the variable args is expected to
- be of type uint64_t. The registers set by this function are r0-r7.
- */
lib/aarch64/processor.c so far doesn't have big function headers like this. Also, since this function is common for all architectures [now], I feel like the documentation should be in common code - so in the header file.
+void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...) +{
- va_list ap;
- TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
" num: %u\n",
num);
Weird line breaking. I see it came from the x86 implementation, but it's weird there too... Personally I'd just put it all on one line, because my vt100 died two decades ago.
- va_start(ap, num);
- for (i = 0; i < num; i++)
set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
^^ should be 'i'
va_arg(ap, uint64_t));
nit: I'd use {} because of the line break. Or just not break the line and bust the 80 char "limit" (RIP vt100).
Thanks, drew
- va_end(ap);
+} diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c index 32a02360b1eb0..680f37be9dbc9 100644 --- a/tools/testing/selftests/kvm/lib/s390x/processor.c +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c @@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code) run->psw_addr = (uintptr_t)guest_code; } +/* VM VCPU Args Set
- Input Args:
- vm - Virtual Machine
- vcpuid - VCPU ID
- num - number of arguments
- ... - arguments, each of type uint64_t
- Output Args: None
- Return: None
- Sets the first num function input arguments to the values
- given as variable args. Each of the variable args is expected to
- be of type uint64_t. The registers set by this function are r2-r6.
- */
+void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...) +{
- va_list ap;
- struct kvm_regs regs;
- TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
" num: %u\n",
num);
- va_start(ap, num);
- vcpu_regs_get(vm, vcpuid, ®s);
- for (i = 0; i < num; i++)
regs.gprs[i + 2] = va_arg(ap, uint64_t);
- vcpu_regs_set(vm, vcpuid, ®s);
- va_end(ap);
+}
void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent) { struct vcpu *vcpu = vm->vcpu_head;