All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: dvhart <dvhart@infradead.org>,
	"Russell King, ARM Linux" <linux@armlinux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Guo Ren <guoren@kernel.org>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Heiko Carstens <hca@linux.ibm.com>, gor <gor@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@redhat.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>, paulmck <paulmck@kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>, shuah <shuah@kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-csky <linux-csky@vger.kernel.org>,
	linux-mips <linux-mips@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	KVM list <kvm@vger.kernel.org>,
	linux-kselftest <linux-kselftest@vger.kernel.org>,
	Peter Foley <pefoley@google.com>,
	Shakeel Butt <shakeelb@google.com>,
	Ben Gardon <bgardon@google.com>
Subject: Re: [PATCH v2 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs
Date: Fri, 27 Aug 2021 23:23:30 +0000	[thread overview]
Message-ID: <YSlz8h9SWgeuicak@google.com> (raw)
In-Reply-To: <339641531.29941.1630091374065.JavaMail.zimbra@efficios.com>

On Fri, Aug 27, 2021, Mathieu Desnoyers wrote:
> > So there are effectively three reasons we want a delay:
> > 
> >  1. To allow sched_setaffinity() to coincide with ioctl(KVM_RUN) before KVM can
> >     enter the guest so that the guest doesn't need an arch-specific VM-Exit source.
> > 
> >  2. To let ioctl(KVM_RUN) make its way back to the test before the next round
> >     of migration.
> > 
> >  3. To ensure the read-side can make forward progress, e.g. if sched_getcpu()
> >     involves a syscall.
> > 
> > 
> > After looking at KVM for arm64 and s390, #1 is a bit tenuous because x86 is the
> > only arch that currently uses xfer_to_guest_mode_work(), i.e. the test could be
> > tweaked to be overtly x86-specific.  But since a delay is needed for #2 and #3,
> > I'd prefer to rely on it for #1 as well in the hopes that this test provides
> > coverage for arm64 and/or s390 if they're ever converted to use the common
> > xfer_to_guest_mode_work().
> 
> Now that we have this understanding of why we need the delay, it would be good to
> write this down in a comment within the test.

Ya, I'll get a new version out next week.

> Does it reproduce if we randomize the delay to have it picked randomly from 0us
> to 100us (with 1us step) ? It would remove a lot of the needs for arch-specific
> magic delay value.

My less-than-scientific testing shows that it can reproduce at delays up to ~500us,
but above ~10us the reproducibility starts to drop.  The bug still reproduces
reliably, it just takes more iterations, and obviously the test runs a bit slower.

Any objection to using a 1-10us delay, e.g. a simple usleep((i % 10) + 1)?

WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: KVM list <kvm@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Will Deacon <will@kernel.org>, Guo Ren <guoren@kernel.org>,
	linux-kselftest <linux-kselftest@vger.kernel.org>,
	Ben Gardon <bgardon@google.com>, shuah <shuah@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	linux-s390 <linux-s390@vger.kernel.org>, gor <gor@linux.ibm.com>,
	"Russell King, ARM Linux" <linux@armlinux.org.uk>,
	linux-csky <linux-csky@vger.kernel.org>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Ingo Molnar <mingo@redhat.com>, dvhart <dvhart@infradead.org>,
	linux-mips <linux-mips@vger.kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>, paulmck <paulmck@kernel.org>,
	Heiko Carstens <hca@linux.ibm.com>, rostedt <rostedt@goodmis.org>,
	Shakeel Butt <shakeelb@google.com>,
	Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Foley <pefoley@google.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Oleg Nesterov <oleg@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v2 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs
Date: Fri, 27 Aug 2021 23:23:30 +0000	[thread overview]
Message-ID: <YSlz8h9SWgeuicak@google.com> (raw)
In-Reply-To: <339641531.29941.1630091374065.JavaMail.zimbra@efficios.com>

On Fri, Aug 27, 2021, Mathieu Desnoyers wrote:
> > So there are effectively three reasons we want a delay:
> > 
> >  1. To allow sched_setaffinity() to coincide with ioctl(KVM_RUN) before KVM can
> >     enter the guest so that the guest doesn't need an arch-specific VM-Exit source.
> > 
> >  2. To let ioctl(KVM_RUN) make its way back to the test before the next round
> >     of migration.
> > 
> >  3. To ensure the read-side can make forward progress, e.g. if sched_getcpu()
> >     involves a syscall.
> > 
> > 
> > After looking at KVM for arm64 and s390, #1 is a bit tenuous because x86 is the
> > only arch that currently uses xfer_to_guest_mode_work(), i.e. the test could be
> > tweaked to be overtly x86-specific.  But since a delay is needed for #2 and #3,
> > I'd prefer to rely on it for #1 as well in the hopes that this test provides
> > coverage for arm64 and/or s390 if they're ever converted to use the common
> > xfer_to_guest_mode_work().
> 
> Now that we have this understanding of why we need the delay, it would be good to
> write this down in a comment within the test.

Ya, I'll get a new version out next week.

> Does it reproduce if we randomize the delay to have it picked randomly from 0us
> to 100us (with 1us step) ? It would remove a lot of the needs for arch-specific
> magic delay value.

My less-than-scientific testing shows that it can reproduce at delays up to ~500us,
but above ~10us the reproducibility starts to drop.  The bug still reproduces
reliably, it just takes more iterations, and obviously the test runs a bit slower.

Any objection to using a 1-10us delay, e.g. a simple usleep((i % 10) + 1)?

WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: dvhart <dvhart@infradead.org>,
	"Russell King, ARM Linux" <linux@armlinux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Guo Ren <guoren@kernel.org>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Heiko Carstens <hca@linux.ibm.com>, gor <gor@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@redhat.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>, paulmck <paulmck@kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>, shuah <shuah@kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-csky <linux-csky@vger.kernel.org>,
	linux-mips <linux-mips@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	KVM list <kvm@vger.kernel.org>,
	linux-kselftest <linux-kselftest@vger.kernel.org>,
	Peter Foley <pefoley@google.com>,
	Shakeel Butt <shakeelb@google.com>,
	Ben Gardon <bgardon@google.com>
Subject: Re: [PATCH v2 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs
Date: Fri, 27 Aug 2021 23:23:30 +0000	[thread overview]
Message-ID: <YSlz8h9SWgeuicak@google.com> (raw)
In-Reply-To: <339641531.29941.1630091374065.JavaMail.zimbra@efficios.com>

On Fri, Aug 27, 2021, Mathieu Desnoyers wrote:
> > So there are effectively three reasons we want a delay:
> > 
> >  1. To allow sched_setaffinity() to coincide with ioctl(KVM_RUN) before KVM can
> >     enter the guest so that the guest doesn't need an arch-specific VM-Exit source.
> > 
> >  2. To let ioctl(KVM_RUN) make its way back to the test before the next round
> >     of migration.
> > 
> >  3. To ensure the read-side can make forward progress, e.g. if sched_getcpu()
> >     involves a syscall.
> > 
> > 
> > After looking at KVM for arm64 and s390, #1 is a bit tenuous because x86 is the
> > only arch that currently uses xfer_to_guest_mode_work(), i.e. the test could be
> > tweaked to be overtly x86-specific.  But since a delay is needed for #2 and #3,
> > I'd prefer to rely on it for #1 as well in the hopes that this test provides
> > coverage for arm64 and/or s390 if they're ever converted to use the common
> > xfer_to_guest_mode_work().
> 
> Now that we have this understanding of why we need the delay, it would be good to
> write this down in a comment within the test.

Ya, I'll get a new version out next week.

> Does it reproduce if we randomize the delay to have it picked randomly from 0us
> to 100us (with 1us step) ? It would remove a lot of the needs for arch-specific
> magic delay value.

My less-than-scientific testing shows that it can reproduce at delays up to ~500us,
but above ~10us the reproducibility starts to drop.  The bug still reproduces
reliably, it just takes more iterations, and obviously the test runs a bit slower.

Any objection to using a 1-10us delay, e.g. a simple usleep((i % 10) + 1)?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-08-27 23:23 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20 22:49 [PATCH v2 0/5] KVM: rseq: Fix and a test for a KVM+rseq bug Sean Christopherson
2021-08-20 22:49 ` Sean Christopherson
2021-08-20 22:49 ` Sean Christopherson
2021-08-20 22:49 ` [PATCH v2 1/5] KVM: rseq: Update rseq when processing NOTIFY_RESUME on xfer to KVM guest Sean Christopherson
2021-08-20 22:49   ` Sean Christopherson
2021-08-20 22:49   ` Sean Christopherson
2021-08-23 15:00   ` Mathieu Desnoyers
2021-08-23 15:00     ` Mathieu Desnoyers
2021-08-23 15:00     ` Mathieu Desnoyers
2021-08-20 22:49 ` [PATCH v2 2/5] entry: rseq: Call rseq_handle_notify_resume() in tracehook_notify_resume() Sean Christopherson
2021-08-20 22:49   ` Sean Christopherson
2021-08-20 22:49   ` Sean Christopherson
2021-08-20 22:50 ` [PATCH v2 3/5] tools: Move x86 syscall number fallbacks to .../uapi/ Sean Christopherson
2021-08-20 22:50   ` Sean Christopherson
2021-08-20 22:50   ` Sean Christopherson
2021-08-20 22:50 ` [PATCH v2 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs Sean Christopherson
2021-08-20 22:50   ` Sean Christopherson
2021-08-20 22:50   ` Sean Christopherson
2021-08-23 15:18   ` Mathieu Desnoyers
2021-08-23 15:18     ` Mathieu Desnoyers
2021-08-23 15:18     ` Mathieu Desnoyers
2021-08-23 15:20     ` Mathieu Desnoyers
2021-08-23 15:20       ` Mathieu Desnoyers
2021-08-23 15:20       ` Mathieu Desnoyers
2021-08-26  0:51       ` Sean Christopherson
2021-08-26  0:51         ` Sean Christopherson
2021-08-26  0:51         ` Sean Christopherson
2021-08-26 18:42         ` Mathieu Desnoyers
2021-08-26 18:42           ` Mathieu Desnoyers
2021-08-26 18:42           ` Mathieu Desnoyers
2021-08-26 23:54           ` Sean Christopherson
2021-08-26 23:54             ` Sean Christopherson
2021-08-26 23:54             ` Sean Christopherson
2021-08-27 19:09             ` Mathieu Desnoyers
2021-08-27 19:09               ` Mathieu Desnoyers
2021-08-27 19:09               ` Mathieu Desnoyers
2021-08-27 23:23               ` Sean Christopherson [this message]
2021-08-27 23:23                 ` Sean Christopherson
2021-08-27 23:23                 ` Sean Christopherson
2021-08-28  0:06                 ` Mathieu Desnoyers
2021-08-28  0:06                   ` Mathieu Desnoyers
2021-08-28  0:06                   ` Mathieu Desnoyers
2021-08-20 22:50 ` [PATCH v2 5/5] KVM: selftests: Remove __NR_userfaultfd syscall fallback Sean Christopherson
2021-08-20 22:50   ` Sean Christopherson
2021-08-20 22:50   ` Sean Christopherson
2021-08-23 23:46   ` Ben Gardon
2021-08-23 23:46     ` Ben Gardon
2021-08-23 23:46     ` Ben Gardon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YSlz8h9SWgeuicak@google.com \
    --to=seanjc@google.com \
    --cc=benh@kernel.crashing.org \
    --cc=bgardon@google.com \
    --cc=boqun.feng@gmail.com \
    --cc=borntraeger@de.ibm.com \
    --cc=catalin.marinas@arm.com \
    --cc=dvhart@infradead.org \
    --cc=gor@linux.ibm.com \
    --cc=guoren@kernel.org \
    --cc=hca@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-csky@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=luto@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=oleg@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=paulus@samba.org \
    --cc=pbonzini@redhat.com \
    --cc=pefoley@google.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=shakeelb@google.com \
    --cc=shuah@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tsbogend@alpha.franken.de \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.