On Sat, Apr 2, 2022 at 10:40 AM Oliver Upton <oupton@xxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Oliver Upton <oupton@xxxxxxxxxx>
> ---
> .../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@xxxxxxxxxx>
