Commit bd024e82 authored by Will Deacon's avatar Will Deacon

asm-generic/mmiowb: Allow mmiowb_set_pending() when preemptible()

Although mmiowb() is concerned only with serialising MMIO writes occuring
in contexts where a spinlock is held, the call to mmiowb_set_pending()
from the MMIO write accessors can occur in preemptible contexts, such
as during driver probe() functions where ordering between CPUs is not
usually a concern, assuming that the task migration path provides the
necessary ordering guarantees.

Unfortunately, the default implementation of mmiowb_set_pending() is not
preempt-safe, as it makes use of a a per-cpu variable to track its
internal state. This has been reported to generate the following splat
on riscv:

 | BUG: using smp_processor_id() in preemptible [00000000] code: swapper/0/1
 | caller is regmap_mmio_write32le+0x1c/0x46
 | CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-hfu+ #1
 | Call Trace:
 |  walk_stackframe+0x0/0x7a
 |  dump_stack+0x6e/0x88
 |  regmap_mmio_write32le+0x18/0x46
 |  check_preemption_disabled+0xa4/0xaa
 |  regmap_mmio_write32le+0x18/0x46
 |  regmap_mmio_write+0x26/0x44
 |  regmap_write+0x28/0x48
 |  sifive_gpio_probe+0xc0/0x1da

Although it's possible to fix the driver in this case, other splats have
been seen from other drivers, including the infamous 8250 UART, and so
it's better to address this problem in the mmiowb core itself.

Fix mmiowb_set_pending() by using the raw_cpu_ptr() to get at the mmiowb
state and then only updating the 'mmiowb_pending' field if we are not
preemptible (i.e. we have a non-zero nesting count).

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Guo Ren <guoren@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Reported-by: default avatarPalmer Dabbelt <palmer@dabbelt.com>
Reported-by: default avatarEmil Renner Berthing <kernel@esmil.dk>
Tested-by: default avatarEmil Renner Berthing <kernel@esmil.dk>
Reviewed-by: default avatarPalmer Dabbelt <palmerdabbelt@google.com>
Acked-by: default avatarPalmer Dabbelt <palmerdabbelt@google.com>
Link: https://lore.kernel.org/r/20200716112816.7356-1-will@kernel.orgSigned-off-by: default avatarWill Deacon <will@kernel.org>
parent 5afc7855
...@@ -27,7 +27,7 @@ ...@@ -27,7 +27,7 @@
#include <asm/smp.h> #include <asm/smp.h>
DECLARE_PER_CPU(struct mmiowb_state, __mmiowb_state); DECLARE_PER_CPU(struct mmiowb_state, __mmiowb_state);
#define __mmiowb_state() this_cpu_ptr(&__mmiowb_state) #define __mmiowb_state() raw_cpu_ptr(&__mmiowb_state)
#else #else
#define __mmiowb_state() arch_mmiowb_state() #define __mmiowb_state() arch_mmiowb_state()
#endif /* arch_mmiowb_state */ #endif /* arch_mmiowb_state */
...@@ -35,7 +35,9 @@ DECLARE_PER_CPU(struct mmiowb_state, __mmiowb_state); ...@@ -35,7 +35,9 @@ DECLARE_PER_CPU(struct mmiowb_state, __mmiowb_state);
static inline void mmiowb_set_pending(void) static inline void mmiowb_set_pending(void)
{ {
struct mmiowb_state *ms = __mmiowb_state(); struct mmiowb_state *ms = __mmiowb_state();
ms->mmiowb_pending = ms->nesting_count;
if (likely(ms->nesting_count))
ms->mmiowb_pending = ms->nesting_count;
} }
static inline void mmiowb_spin_lock(void) static inline void mmiowb_spin_lock(void)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment