mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Palmer Dabbelt <palmer@dabbelt.com>
To: linux-riscv@lists.infradead.org
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	aou@eecs.berkeley.edu, peterz@infradead.org, jpoimboe@redhat.com,
	jbaron@akamai.com, rostedt@goodmis.org, ardb@kernel.org,
	Atish Patra <Atish.Patra@wdc.com>,
	Anup Patel <Anup.Patel@wdc.com>,
	akpm@linux-foundation.org, rppt@kernel.org, mhiramat@kernel.org,
	zong.li@sifive.com, guoren@linux.alibaba.com,
	wangkefeng.wang@huawei.com, 0x7f454c46@gmail.com,
	chenhuang5@huawei.com, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org, kernel-team@android.com,
	Palmer Dabbelt <palmerdabbelt@google.com>,
	Changbin Du <changbin.du@gmail.com>
Subject: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT_POKE*
Date: Wed, 28 Apr 2021 23:17:13 -0700	[thread overview]
Message-ID: <20210429061713.783628-1-palmer@dabbelt.com> (raw)

From: Palmer Dabbelt <palmerdabbelt@google.com>

We currently use text_mutex to protect the fixmap sections from
concurrent callers.  This is convienent for kprobes as the generic code
already holds text_mutex, but ftrace doesn't which triggers a lockdep
assertion.  We could take text_mutex for ftrace, but the jump label
implementation (which is currently taking text_mutex) isn't explicitly
listed as being sleepable and it's called from enough places it seems
safer to just avoid sleeping.

arm64 and parisc, the other two TEXT_POKE-style patching
implemnetations, already use raw spinlocks.  abffa6f3b157 ("arm64:
convert patch_lock to raw lock") lays out the case for a raw spinlock as
opposed to a regular spinlock, and while I don't know of anyone using rt
on RISC-V I'm sure it'll eventually show up and I don't see any reason
to wait.

Fixes: ebc00dde8a97 ("riscv: Add jump-label implementation")
Reported-by: Changbin Du <changbin.du@gmail.com>
Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
 arch/riscv/include/asm/fixmap.h |  3 +++
 arch/riscv/kernel/jump_label.c  |  2 --
 arch/riscv/kernel/patch.c       | 13 +++++++++----
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/fixmap.h b/arch/riscv/include/asm/fixmap.h
index 54cbf07fb4e9..d1c0a1f123cf 100644
--- a/arch/riscv/include/asm/fixmap.h
+++ b/arch/riscv/include/asm/fixmap.h
@@ -24,8 +24,11 @@ enum fixed_addresses {
 	FIX_HOLE,
 	FIX_PTE,
 	FIX_PMD,
+
+	/* Only used in kernel/insn.c */
 	FIX_TEXT_POKE1,
 	FIX_TEXT_POKE0,
+
 	FIX_EARLYCON_MEM_BASE,
 
 	__end_of_permanent_fixed_addresses,
diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
index 20e09056d141..45bb32f91b5c 100644
--- a/arch/riscv/kernel/jump_label.c
+++ b/arch/riscv/kernel/jump_label.c
@@ -35,9 +35,7 @@ void arch_jump_label_transform(struct jump_entry *entry,
 		insn = RISCV_INSN_NOP;
 	}
 
-	mutex_lock(&text_mutex);
 	patch_text_nosync(addr, &insn, sizeof(insn));
-	mutex_unlock(&text_mutex);
 }
 
 void arch_jump_label_transform_static(struct jump_entry *entry,
diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 0b552873a577..dfa7ee8eb63f 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -19,6 +19,8 @@ struct patch_insn {
 	atomic_t cpu_count;
 };
 
+static DEFINE_RAW_SPINLOCK(patch_lock);
+
 #ifdef CONFIG_MMU
 /*
  * The fix_to_virt(, idx) needs a const value (not a dynamic variable of
@@ -54,13 +56,14 @@ static int patch_insn_write(void *addr, const void *insn, size_t len)
 	void *waddr = addr;
 	bool across_pages = (((uintptr_t) addr & ~PAGE_MASK) + len) > PAGE_SIZE;
 	int ret;
+	unsigned long flags = 0;
 
 	/*
-	 * Before reaching here, it was expected to lock the text_mutex
-	 * already, so we don't need to give another lock here and could
-	 * ensure that it was safe between each cores.
+	 * FIX_TEXT_POKE{0,1} are only used for text patching, but we must
+	 * ensure that concurrent callers do not re-map these before we're done
+	 * with them.
 	 */
-	lockdep_assert_held(&text_mutex);
+	raw_spin_lock_irqsave(&patch_lock, flags);
 
 	if (across_pages)
 		patch_map(addr + len, FIX_TEXT_POKE1);
@@ -74,6 +77,8 @@ static int patch_insn_write(void *addr, const void *insn, size_t len)
 	if (across_pages)
 		patch_unmap(FIX_TEXT_POKE1);
 
+	raw_spin_unlock_irqrestore(&patch_lock, flags);
+
 	return ret;
 }
 NOKPROBE_SYMBOL(patch_insn_write);
-- 
2.31.1.498.g6c1eba8ee3d-goog


             reply	other threads:[~2021-04-29  6:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-29  6:17 Palmer Dabbelt [this message]
2021-04-29  7:55 ` Anup Patel
2021-04-29 16:30 ` Steven Rostedt
2021-04-29 21:54   ` Changbin Du
2021-04-30  2:47     ` Steven Rostedt
2021-04-30  4:06       ` Anup Patel
2021-04-30 11:34         ` Steven Rostedt
2021-04-30 19:44           ` Palmer Dabbelt
2021-05-06  7:11             ` Palmer Dabbelt

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=20210429061713.783628-1-palmer@dabbelt.com \
    --to=palmer@dabbelt.com \
    --cc=0x7f454c46@gmail.com \
    --cc=Anup.Patel@wdc.com \
    --cc=Atish.Patra@wdc.com \
    --cc=akpm@linux-foundation.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=ardb@kernel.org \
    --cc=changbin.du@gmail.com \
    --cc=chenhuang5@huawei.com \
    --cc=guoren@linux.alibaba.com \
    --cc=jbaron@akamai.com \
    --cc=jpoimboe@redhat.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mhiramat@kernel.org \
    --cc=palmerdabbelt@google.com \
    --cc=paul.walmsley@sifive.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=rppt@kernel.org \
    --cc=wangkefeng.wang@huawei.com \
    --cc=zong.li@sifive.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®