| Message ID | 20200123180436.99487-7-bgardon@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Create a userfaultfd demand paging test | expand |
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. > > 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. > + */ > +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); > + > + va_start(ap, num); > + > + for (i = 0; i < num; i++) > + set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]), > + va_arg(ap, uint64_t)); > + > + 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; >
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; > > >
On 24/01/20 10:35, Andrew Jones wrote: > 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. You can say: For ARM parts: Reviewed-by: ... Paolo >>> >>> 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; >>> >> >
On Fri, Jan 24, 2020 at 10:35:43AM +0100, Andrew Jones wrote: > > > +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' The declaration of 'i' is also missing. > > > > + 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
On 24.01.20 10:03, Paolo Bonzini wrote: > CCing Marc, Conny and Christian (plus Thomas and Drew who were already > in the list) for review. CC Thomas Huth, > > 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. [...] >> .../selftests/kvm/lib/s390x/processor.c | 35 +++++++++++++++++++ [...] >> --- 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; >> > Untested but the logic looks sane according to the ELF ABI.
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. > > 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. > + */ > +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); > + > + va_start(ap, num); > + > + for (i = 0; i < num; i++) > + set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]), > + va_arg(ap, uint64_t)); > + > + 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; > Squashing this: diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c index a2ff90a75f32..839a76c96f01 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c @@ -353,6 +353,7 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code) void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...) { va_list ap; + int i; TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n" " num: %u\n", diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c index 680f37be9dbc..a0b84235c848 100644 --- a/tools/testing/selftests/kvm/lib/s390x/processor.c +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c @@ -289,6 +289,7 @@ void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...) { va_list ap; struct kvm_regs regs; + int i; TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n" " num: %u\n", Paolo
On Sat, Jan 25, 2020 at 10:34:16AM +0100, Paolo Bonzini wrote: > 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. > > > > 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. > > + */ > > +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); > > + > > + va_start(ap, num); > > + > > + for (i = 0; i < num; i++) > > + set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]), For AArch64 you also need to squash s/num/i/ for this line. (Plus I'm still not that pleased with adding this big header to this file for this function, or the weird line breaks in the assert.) > > + va_arg(ap, uint64_t)); > > + > > + 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; > > > > Squashing this: > > diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c > index a2ff90a75f32..839a76c96f01 100644 > --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c > +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c > @@ -353,6 +353,7 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code) > void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...) > { > va_list ap; > + int i; > > TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n" > " num: %u\n", > diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c > index 680f37be9dbc..a0b84235c848 100644 > --- a/tools/testing/selftests/kvm/lib/s390x/processor.c > +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c > @@ -289,6 +289,7 @@ void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...) > { > va_list ap; > struct kvm_regs regs; > + int i; > > TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n" > " num: %u\n", > > Paolo >
On 23/01/2020 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. > > 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/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; You need an "int i;" here to make it compile. > + TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n" why the "\n" right in the middle of the string? Could you please make it one-line only? > + " 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); > +} ... but apart from the nits, this looks basically sane to me. Thomas
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. + */ +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); + + va_start(ap, num); + + for (i = 0; i < num; i++) + set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]), + va_arg(ap, uint64_t)); + + 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;
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. 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(+)