All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	Saravana Kannan <saravanak@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2] arm64: spectre: Prevent lockdep splat on v4 mitigation enable path
Date: Thu, 18 Feb 2021 15:37:48 +0000	[thread overview]
Message-ID: <6ba6be0cfdb905706de6863d8a6768d7@kernel.org> (raw)
In-Reply-To: <20210218140346.5224-1-will@kernel.org>

On 2021-02-18 14:03, Will Deacon wrote:
> The Spectre-v4 workaround is re-configured when resuming from suspend,
> as the firmware may have re-enabled the mitigation despite the user
> previously asking for it to be disabled.
> 
> Enabling or disabling the workaround can result in an undefined
> instruction exception on CPUs which implement PSTATE.SSBS but only 
> allow
> it to be configured by adjusting the SPSR on exception return. We 
> handle
> this by installing an 'undef hook' which effectively emulates the 
> access.
> 
> Installing this hook requires us to take a couple of spinlocks both to
> avoid corrupting the internal list of hooks but also to ensure that we
> don't run into an unhandled exception. Unfortunately, when resuming 
> from
> suspend, we haven't yet called rcu_idle_exit() and so lockdep gets 
> angry
> about "suspicious RCU usage". In doing so, it tries to print a warning,
> which leads it to get even more suspicious, this time about itself:
> 
>  |  rcu_scheduler_active = 2, debug_locks = 1
>  |  RCU used illegally from extended quiescent state!
>  |  1 lock held by swapper/0:
>  |   #0: (logbuf_lock){-.-.}-{2:2}, at: vprintk_emit+0x88/0x198
>  |
>  |  Call trace:
>  |   dump_backtrace+0x0/0x1d8
>  |   show_stack+0x18/0x24
>  |   dump_stack+0xe0/0x17c
>  |   lockdep_rcu_suspicious+0x11c/0x134
>  |   trace_lock_release+0xa0/0x160
>  |   lock_release+0x3c/0x290
>  |   _raw_spin_unlock+0x44/0x80
>  |   vprintk_emit+0xbc/0x198
>  |   vprintk_default+0x44/0x6c
>  |   vprintk_func+0x1f4/0x1fc
>  |   printk+0x54/0x7c
>  |   lockdep_rcu_suspicious+0x30/0x134
>  |   trace_lock_acquire+0xa0/0x188
>  |   lock_acquire+0x50/0x2fc
>  |   _raw_spin_lock+0x68/0x80
>  |   spectre_v4_enable_mitigation+0xa8/0x30c
>  |   __cpu_suspend_exit+0xd4/0x1a8
>  |   cpu_suspend+0xa0/0x104
>  |   psci_cpu_suspend_enter+0x3c/0x5c
>  |   psci_enter_idle_state+0x44/0x74
>  |   cpuidle_enter_state+0x148/0x2f8
>  |   cpuidle_enter+0x38/0x50
>  |   do_idle+0x1f0/0x2b4
> 
> Prevent these splats by running __cpu_suspend_exit() with RCU watching.
> 
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Saravana Kannan <saravanak@google.com>
> Suggested-by: "Paul E . McKenney" <paulmck@kernel.org>
> Reported-by: Sami Tolvanen <samitolvanen@google.com>
> Fixes: c28762070ca6 ("arm64: Rewrite Spectre-v4 mitigation code")
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
> 
> v2: Use RCU_NONIDLE() instead of eliding the spinlock
> 
>  arch/arm64/kernel/suspend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index a67b37a7a47e..d7564891ffe1 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -119,7 +119,7 @@ int cpu_suspend(unsigned long arg, int 
> (*fn)(unsigned long))
>  		if (!ret)
>  			ret = -EOPNOTSUPP;
>  	} else {
> -		__cpu_suspend_exit();
> +		RCU_NONIDLE(__cpu_suspend_exit());
>  	}
> 
>  	unpause_graph_tracing();

Ah, looks so much better!

Acked-by: Marc Zyngier <maz@kernel.org>

         M.
-- 
Jazz is not dead. It just smells funny...

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

  parent reply	other threads:[~2021-02-18 15:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-18 14:03 [PATCH v2] arm64: spectre: Prevent lockdep splat on v4 mitigation enable path Will Deacon
2021-02-18 15:08 ` Paul E. McKenney
2021-02-18 15:37 ` Marc Zyngier [this message]
2021-02-18 15:44 ` Mark Rutland
2021-02-19 19:16 ` Will Deacon

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=6ba6be0cfdb905706de6863d8a6768d7@kernel.org \
    --to=maz@kernel.org \
    --cc=boqun.feng@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    --cc=saravanak@google.com \
    --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.