All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Frederic Weisbecker <frederic@kernel.org>
Cc: Nicolas Saenz Julienne <nsaenzju@redhat.com>,
	maz <maz@kernel.org>, Will Deacon <will@kernel.org>,
	paulmck <paulmck@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	rcu <rcu@vger.kernel.org>, Thomas Gleixner <tglx@linutronix.de>,
	kvmarm@lists.cs.columbia.edu,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: Possible nohz-full/RCU issue in arm64 KVM
Date: Tue, 4 Jan 2022 13:24:02 +0000	[thread overview]
Message-ID: <YdRJA79bs1Im7h01@FVFF77S0Q05N> (raw)
In-Reply-To: <20211220161014.GC918551@lothringen>

On Mon, Dec 20, 2021 at 05:10:14PM +0100, Frederic Weisbecker wrote:
> On Fri, Dec 17, 2021 at 01:21:39PM +0000, Mark Rutland wrote:
> > On Fri, Dec 17, 2021 at 12:51:57PM +0100, Nicolas Saenz Julienne wrote:
> > > Hi All,
> > 
> > Hi,
> > 
> > > arm64's guest entry code does the following:
> > > 
> > > int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> > > {
> > > 	[...]
> > > 
> > > 	guest_enter_irqoff();
> > > 
> > > 	ret = kvm_call_hyp_ret(__kvm_vcpu_run, vcpu);
> > > 
> > > 	[...]
> > > 
> > > 	local_irq_enable();
> > > 
> > > 	/*
> > > 	 * We do local_irq_enable() before calling guest_exit() so
> > > 	 * that if a timer interrupt hits while running the guest we
> > > 	 * account that tick as being spent in the guest.  We enable
> > > 	 * preemption after calling guest_exit() so that if we get
> > > 	 * preempted we make sure ticks after that is not counted as
> > > 	 * guest time.
> > > 	 */
> > > 	guest_exit();
> > > 	[...]
> > > }
> > > 
> > > 
> > > On a nohz-full CPU, guest_{enter,exit}() delimit an RCU extended quiescent
> > > state (EQS). Any interrupt happening between local_irq_enable() and
> > > guest_exit() should disable that EQS. Now, AFAICT all el0 interrupt handlers
> > > do the right thing if trggered in this context, but el1's won't. Is it
> > > possible to hit an el1 handler (for example __el1_irq()) there?
> > 
> > I think you're right that the EL1 handlers can trigger here and won't exit the
> > EQS.
> > 
> > I'm not immediately sure what we *should* do here. What does x86 do for an IRQ
> > taken from a guest mode? I couldn't spot any handling of that case, but I'm not
> > familiar enough with the x86 exception model to know if I'm looking in the
> > right place.
> 
> This is one of the purposes of rcu_irq_enter(). el1 handlers don't call irq_enter()?

Due to lockep/tracing/etc ordering, we don't use irq_enter() directly and
instead call rcu_irq_enter() and irq_enter_rcu() separately. Critically we only
call rcu_irq_enter() for IRQs taken from the idle thread, as this was
previously thought to be the only place where we could take an IRQ from an EL1
EQS.

See __el1_irq(), __enter_from_kernel_mode(), and __exit_to_kernel_mode() in
arch/arm64/kernel/entry-common.c. The latter two are largely analogous to the
common irqentry_enter9) and irqentry_exit() helpers in kernel/entry/common.c.

We need to either rework the KVM code or that entry code. I'll dig into this a
bit more...

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Frederic Weisbecker <frederic@kernel.org>
Cc: paulmck <paulmck@kernel.org>, maz <maz@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	kvmarm@lists.cs.columbia.edu, rcu <rcu@vger.kernel.org>,
	Nicolas Saenz Julienne <nsaenzju@redhat.com>,
	Will Deacon <will@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: Possible nohz-full/RCU issue in arm64 KVM
Date: Tue, 4 Jan 2022 13:24:02 +0000	[thread overview]
Message-ID: <YdRJA79bs1Im7h01@FVFF77S0Q05N> (raw)
In-Reply-To: <20211220161014.GC918551@lothringen>

On Mon, Dec 20, 2021 at 05:10:14PM +0100, Frederic Weisbecker wrote:
> On Fri, Dec 17, 2021 at 01:21:39PM +0000, Mark Rutland wrote:
> > On Fri, Dec 17, 2021 at 12:51:57PM +0100, Nicolas Saenz Julienne wrote:
> > > Hi All,
> > 
> > Hi,
> > 
> > > arm64's guest entry code does the following:
> > > 
> > > int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> > > {
> > > 	[...]
> > > 
> > > 	guest_enter_irqoff();
> > > 
> > > 	ret = kvm_call_hyp_ret(__kvm_vcpu_run, vcpu);
> > > 
> > > 	[...]
> > > 
> > > 	local_irq_enable();
> > > 
> > > 	/*
> > > 	 * We do local_irq_enable() before calling guest_exit() so
> > > 	 * that if a timer interrupt hits while running the guest we
> > > 	 * account that tick as being spent in the guest.  We enable
> > > 	 * preemption after calling guest_exit() so that if we get
> > > 	 * preempted we make sure ticks after that is not counted as
> > > 	 * guest time.
> > > 	 */
> > > 	guest_exit();
> > > 	[...]
> > > }
> > > 
> > > 
> > > On a nohz-full CPU, guest_{enter,exit}() delimit an RCU extended quiescent
> > > state (EQS). Any interrupt happening between local_irq_enable() and
> > > guest_exit() should disable that EQS. Now, AFAICT all el0 interrupt handlers
> > > do the right thing if trggered in this context, but el1's won't. Is it
> > > possible to hit an el1 handler (for example __el1_irq()) there?
> > 
> > I think you're right that the EL1 handlers can trigger here and won't exit the
> > EQS.
> > 
> > I'm not immediately sure what we *should* do here. What does x86 do for an IRQ
> > taken from a guest mode? I couldn't spot any handling of that case, but I'm not
> > familiar enough with the x86 exception model to know if I'm looking in the
> > right place.
> 
> This is one of the purposes of rcu_irq_enter(). el1 handlers don't call irq_enter()?

Due to lockep/tracing/etc ordering, we don't use irq_enter() directly and
instead call rcu_irq_enter() and irq_enter_rcu() separately. Critically we only
call rcu_irq_enter() for IRQs taken from the idle thread, as this was
previously thought to be the only place where we could take an IRQ from an EL1
EQS.

See __el1_irq(), __enter_from_kernel_mode(), and __exit_to_kernel_mode() in
arch/arm64/kernel/entry-common.c. The latter two are largely analogous to the
common irqentry_enter9) and irqentry_exit() helpers in kernel/entry/common.c.

We need to either rework the KVM code or that entry code. I'll dig into this a
bit more...

Thanks,
Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Frederic Weisbecker <frederic@kernel.org>
Cc: Nicolas Saenz Julienne <nsaenzju@redhat.com>,
	maz <maz@kernel.org>, Will Deacon <will@kernel.org>,
	paulmck <paulmck@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	rcu <rcu@vger.kernel.org>, Thomas Gleixner <tglx@linutronix.de>,
	kvmarm@lists.cs.columbia.edu,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: Possible nohz-full/RCU issue in arm64 KVM
Date: Tue, 4 Jan 2022 13:24:02 +0000	[thread overview]
Message-ID: <YdRJA79bs1Im7h01@FVFF77S0Q05N> (raw)
In-Reply-To: <20211220161014.GC918551@lothringen>

On Mon, Dec 20, 2021 at 05:10:14PM +0100, Frederic Weisbecker wrote:
> On Fri, Dec 17, 2021 at 01:21:39PM +0000, Mark Rutland wrote:
> > On Fri, Dec 17, 2021 at 12:51:57PM +0100, Nicolas Saenz Julienne wrote:
> > > Hi All,
> > 
> > Hi,
> > 
> > > arm64's guest entry code does the following:
> > > 
> > > int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> > > {
> > > 	[...]
> > > 
> > > 	guest_enter_irqoff();
> > > 
> > > 	ret = kvm_call_hyp_ret(__kvm_vcpu_run, vcpu);
> > > 
> > > 	[...]
> > > 
> > > 	local_irq_enable();
> > > 
> > > 	/*
> > > 	 * We do local_irq_enable() before calling guest_exit() so
> > > 	 * that if a timer interrupt hits while running the guest we
> > > 	 * account that tick as being spent in the guest.  We enable
> > > 	 * preemption after calling guest_exit() so that if we get
> > > 	 * preempted we make sure ticks after that is not counted as
> > > 	 * guest time.
> > > 	 */
> > > 	guest_exit();
> > > 	[...]
> > > }
> > > 
> > > 
> > > On a nohz-full CPU, guest_{enter,exit}() delimit an RCU extended quiescent
> > > state (EQS). Any interrupt happening between local_irq_enable() and
> > > guest_exit() should disable that EQS. Now, AFAICT all el0 interrupt handlers
> > > do the right thing if trggered in this context, but el1's won't. Is it
> > > possible to hit an el1 handler (for example __el1_irq()) there?
> > 
> > I think you're right that the EL1 handlers can trigger here and won't exit the
> > EQS.
> > 
> > I'm not immediately sure what we *should* do here. What does x86 do for an IRQ
> > taken from a guest mode? I couldn't spot any handling of that case, but I'm not
> > familiar enough with the x86 exception model to know if I'm looking in the
> > right place.
> 
> This is one of the purposes of rcu_irq_enter(). el1 handlers don't call irq_enter()?

Due to lockep/tracing/etc ordering, we don't use irq_enter() directly and
instead call rcu_irq_enter() and irq_enter_rcu() separately. Critically we only
call rcu_irq_enter() for IRQs taken from the idle thread, as this was
previously thought to be the only place where we could take an IRQ from an EL1
EQS.

See __el1_irq(), __enter_from_kernel_mode(), and __exit_to_kernel_mode() in
arch/arm64/kernel/entry-common.c. The latter two are largely analogous to the
common irqentry_enter9) and irqentry_exit() helpers in kernel/entry/common.c.

We need to either rework the KVM code or that entry code. I'll dig into this a
bit more...

Thanks,
Mark.

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

  reply	other threads:[~2022-01-04 13:24 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-17 11:51 Possible nohz-full/RCU issue in arm64 KVM Nicolas Saenz Julienne
2021-12-17 11:51 ` Nicolas Saenz Julienne
2021-12-17 11:51 ` Nicolas Saenz Julienne
2021-12-17 13:21 ` Mark Rutland
2021-12-17 13:21   ` Mark Rutland
2021-12-17 13:21   ` Mark Rutland
2021-12-17 14:15   ` Nicolas Saenz Julienne
2021-12-17 14:15     ` Nicolas Saenz Julienne
2021-12-17 14:15     ` Nicolas Saenz Julienne
2021-12-17 14:38     ` Mark Rutland
2021-12-17 14:38       ` Mark Rutland
2021-12-17 14:38       ` Mark Rutland
2021-12-17 15:54       ` Paolo Bonzini
2021-12-17 15:54         ` Paolo Bonzini
2021-12-17 15:54         ` Paolo Bonzini
2021-12-17 16:07         ` Paul E. McKenney
2021-12-17 16:07           ` Paul E. McKenney
2021-12-17 16:07           ` Paul E. McKenney
2021-12-17 16:20           ` Nicolas Saenz Julienne
2021-12-17 16:20             ` Nicolas Saenz Julienne
2021-12-17 16:20             ` Nicolas Saenz Julienne
2021-12-17 16:43             ` Paul E. McKenney
2021-12-17 16:43               ` Paul E. McKenney
2021-12-17 16:43               ` Paul E. McKenney
2021-12-17 16:34           ` Paolo Bonzini
2021-12-17 16:34             ` Paolo Bonzini
2021-12-17 16:34             ` Paolo Bonzini
2021-12-17 16:45             ` Paul E. McKenney
2021-12-17 16:45               ` Paul E. McKenney
2021-12-17 16:45               ` Paul E. McKenney
2021-12-17 17:02               ` Paolo Bonzini
2021-12-17 17:02                 ` Paolo Bonzini
2021-12-17 17:02                 ` Paolo Bonzini
2021-12-17 17:12                 ` Paul E. McKenney
2021-12-17 17:12                   ` Paul E. McKenney
2021-12-17 17:12                   ` Paul E. McKenney
2021-12-17 17:23                   ` Paolo Bonzini
2021-12-17 17:23                     ` Paolo Bonzini
2021-12-17 17:23                     ` Paolo Bonzini
2021-12-17 17:47                     ` Paul E. McKenney
2021-12-17 17:47                       ` Paul E. McKenney
2021-12-17 17:47                       ` Paul E. McKenney
2022-01-04 16:39         ` Mark Rutland
2022-01-04 16:39           ` Mark Rutland
2022-01-04 16:39           ` Mark Rutland
2022-01-04 17:07           ` Paolo Bonzini
2022-01-04 17:07             ` Paolo Bonzini
2022-01-04 17:07             ` Paolo Bonzini
2022-01-11 11:32           ` Nicolas Saenz Julienne
2022-01-11 11:32             ` Nicolas Saenz Julienne
2022-01-11 11:32             ` Nicolas Saenz Julienne
2022-01-11 12:23             ` Mark Rutland
2022-01-11 12:23               ` Mark Rutland
2022-01-11 12:23               ` Mark Rutland
2021-12-17 14:51   ` Paolo Bonzini
2021-12-17 14:51     ` Paolo Bonzini
2021-12-17 14:51     ` Paolo Bonzini
2021-12-20 14:28   ` Marc Zyngier
2021-12-20 14:28     ` Marc Zyngier
2021-12-20 14:28     ` Marc Zyngier
2021-12-20 16:10   ` Frederic Weisbecker
2021-12-20 16:10     ` Frederic Weisbecker
2021-12-20 16:10     ` Frederic Weisbecker
2022-01-04 13:24     ` Mark Rutland [this message]
2022-01-04 13:24       ` Mark Rutland
2022-01-04 13:24       ` Mark Rutland

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=YdRJA79bs1Im7h01@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=frederic@kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=nsaenzju@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=rcu@vger.kernel.org \
    --cc=tglx@linutronix.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.