[PATCH 3/4] selftests: KVM: Don't leak GIC FD across dirty log test iterations

[Jing Zhang]

On Sat, Apr 2, 2022 at 10:40 AM Oliver Upton <oupton at google.com> wrote:
>
> dirty_log_perf_test instantiates a VGICv3 for the guest (if supported by
> hardware) to reduce the overhead of guest exits. However, the test does
> not actually close the GIC fd when cleaning up the VM between test
> iterations, meaning that the VM is never actually destroyed in the
> kernel.
>
> While this is generally a bad idea, the bug was detected from the kernel
> spewing about duplicate debugfs entries as subsequent VMs happen to
> reuse the same FD even though the debugfs directory is still present.
>
> Abstract away the notion of setup/cleanup of the GIC FD from the test
> by creating arch-specific helpers for test setup/cleanup. Close the GIC
> FD on VM cleanup and do nothing for the other architectures.
>
> Fixes: c340f7899af6 ("KVM: selftests: Add vgic initialization for dirty log perf test for ARM")
> Cc: Jing Zhang <jingzhangos at google.com>
> Signed-off-by: Oliver Upton <oupton at google.com>
> ---
>  .../selftests/kvm/dirty_log_perf_test.c       | 34 +++++++++++++++++--
>  1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index c9d9e513ca04..7b47ae4f952e 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -18,11 +18,40 @@
>  #include "test_util.h"
>  #include "perf_test_util.h"
>  #include "guest_modes.h"
> +
>  #ifdef __aarch64__
>  #include "aarch64/vgic.h"
>
>  #define GICD_BASE_GPA                  0x8000000ULL
>  #define GICR_BASE_GPA                  0x80A0000ULL
> +
> +static int gic_fd;
> +
> +static void arch_setup_vm(struct kvm_vm *vm, unsigned int nr_vcpus)
> +{
> +       /*
> +        * The test can still run even if hardware does not support GICv3, as it
> +        * is only an optimization to reduce guest exits.
> +        */
> +       gic_fd = vgic_v3_setup(vm, nr_vcpus, 64, GICD_BASE_GPA, GICR_BASE_GPA);
> +}
> +
> +static void arch_cleanup_vm(struct kvm_vm *vm)
> +{
> +       if (gic_fd > 0)
> +               close(gic_fd);
> +}
> +
> +#else /* __aarch64__ */
> +
> +static void arch_setup_vm(struct kvm_vm *vm, unsigned int nr_vcpus)
> +{
> +}
> +
> +static void arch_cleanup_vm(struct kvm_vm *vm)
> +{
> +}
> +
>  #endif
>
>  /* How many host loops to run by default (one KVM_GET_DIRTY_LOG for each loop)*/
> @@ -206,9 +235,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>                 vm_enable_cap(vm, &cap);
>         }
>
> -#ifdef __aarch64__
> -       vgic_v3_setup(vm, nr_vcpus, 64, GICD_BASE_GPA, GICR_BASE_GPA);
> -#endif
> +       arch_setup_vm(vm, nr_vcpus);
>
>         /* Start the iterations */
>         iteration = 0;
> @@ -302,6 +329,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>         }
>
>         free_bitmaps(bitmaps, p->slots);
> +       arch_cleanup_vm(vm);
>         perf_test_destroy_vm(vm);
>  }
>
> --
> 2.35.1.1094.g7c7d902a7c-goog
>
Reviewed-by: Jing Zhang <jingzhangos at google.com>