On Fri, Feb 14, 2020 at 6:59 AM Andrew Jones <drjones@xxxxxxxxxx> wrote:
>
> [Fixed array index (num => i) and made some style changes.]
> Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> ---
> .../selftests/kvm/lib/aarch64/processor.c | 24 ++++---------------
> 1 file changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 839a76c96f01..f7dffccea12c 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -334,36 +334,20 @@ 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.
> - */
I'm sad to see this comment go. I realize it might be more verbose
than necessary, but calling out that the args will all be interpreted
as uint_64s and which registers are set feels like useful context to
have here.
> 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",
> - num);
> + " 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 (i = 0; i < num; i++) {
> + set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[i]),
> va_arg(ap, uint64_t));
> + }
Woops, I should have caught this in the original demand paging test
series, but didn't notice because this function was only ever called
with one argument.
Thank you for fixing this.
>
> va_end(ap);
> }
> --
> 2.21.1
>
