linux-kernel - Re: [PATCH v3 12/12] KVM: arm64: selftests: arch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210906063900.t6rbykpwyau5u32s@gator.home>
Date:   Mon, 6 Sep 2021 08:39:00 +0200
From:   Andrew Jones <drjones@...hat.com>
To:     Raghavendra Rao Ananta <rananta@...gle.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, Marc Zyngier <maz@...nel.org>,
        James Morse <james.morse@....com>,
        Alexandru Elisei <alexandru.elisei@....com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        kvm@...r.kernel.org, Catalin Marinas <catalin.marinas@....com>,
        Peter Shier <pshier@...gle.com>, linux-kernel@...r.kernel.org,
        Will Deacon <will@...nel.org>, kvmarm@...ts.cs.columbia.edu,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 12/12] KVM: arm64: selftests: arch_timer: Support vCPU
 migration

On Fri, Sep 03, 2021 at 01:53:27PM -0700, Raghavendra Rao Ananta wrote:
> On Fri, Sep 3, 2021 at 4:05 AM Andrew Jones <drjones@...hat.com> wrote:
> >
> > On Wed, Sep 01, 2021 at 09:14:12PM +0000, Raghavendra Rao Ananta wrote:
> > > Since the timer stack (hardware and KVM) is per-CPU, there
> > > are potential chances for races to occur when the scheduler
> > > decides to migrate a vCPU thread to a different physical CPU.
> > > Hence, include an option to stress-test this part as well by
> > > forcing the vCPUs to migrate across physical CPUs in the
> > > system at a particular rate.
> > >
> > > Originally, the bug for the fix with commit 3134cc8beb69d0d
> > > ("KVM: arm64: vgic: Resample HW pending state on deactivation")
> > > was discovered using arch_timer test with vCPU migrations and
> > > can be easily reproduced.
> > >
> > > Signed-off-by: Raghavendra Rao Ananta <rananta@...gle.com>
> > > ---
> > >  .../selftests/kvm/aarch64/arch_timer.c        | 108 +++++++++++++++++-
> > >  1 file changed, 107 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
> > > index 1383f33850e9..de246c7afab2 100644
> > > --- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
> > > +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
> > > @@ -14,6 +14,8 @@
> > >   *
> > >   * The test provides command-line options to configure the timer's
> > >   * period (-p), number of vCPUs (-n), and iterations per stage (-i).
> > > + * To stress-test the timer stack even more, an option to migrate the
> > > + * vCPUs across pCPUs (-m), at a particular rate, is also provided.
> > >   *
> > >   * Copyright (c) 2021, Google LLC.
> > >   */
> > > @@ -24,6 +26,8 @@
> > >  #include <pthread.h>
> > >  #include <linux/kvm.h>
> > >  #include <linux/sizes.h>
> > > +#include <linux/bitmap.h>
> > > +#include <sys/sysinfo.h>
> > >
> > >  #include "kvm_util.h"
> > >  #include "processor.h"
> > > @@ -41,12 +45,14 @@ struct test_args {
> > >       int nr_vcpus;
> > >       int nr_iter;
> > >       int timer_period_ms;
> > > +     int migration_freq_ms;
> > >  };
> > >
> > >  static struct test_args test_args = {
> > >       .nr_vcpus = NR_VCPUS_DEF,
> > >       .nr_iter = NR_TEST_ITERS_DEF,
> > >       .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF,
> > > +     .migration_freq_ms = 0,         /* Turn off migrations by default */
> >
> > I'd rather we enable good tests like these by default.
> >
> Well, that was my original idea, but I was concerned about the ease
> for diagnosing
> things since it'll become too noisy. And so I let it as a personal
> preference. But I can
> include it back and see how it goes.

Break the tests into two? One run without migration and one with. If
neither work, then we can diagnose the one without first, possibly
avoiding the need to diagnose the one with.

Thanks,
drew