From: guoren@kernel.org
To: guoren@kernel.org, palmer@dabbelt.com, paul.walmsley@sifive.com,
mhiramat@kernel.org, conor.dooley@microchip.com,
penberg@kernel.org, mark.rutland@arm.com, jrtc27@jrtc27.com,
andy.chiu@sifive.com, zong.li@sifive.com
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
Greentime Hu <greentime.hu@sifive.com>
Subject: [PATCH -next V3 1/2] riscv: jump_label: Fixup unaligned arch_static_branch function
Date: Thu, 26 Jan 2023 12:06:06 -0500 [thread overview]
Message-ID: <20230126170607.1489141-2-guoren@kernel.org> (raw)
In-Reply-To: <20230126170607.1489141-1-guoren@kernel.org>
From: Andy Chiu <andy.chiu@sifive.com>
Runtime code patching must be done at a naturally aligned address, or we
may execute on a partial instruction.
We have encountered problems traced back to static jump functions during
the test. We switched the tracer randomly for every 1~5 seconds on a
dual-core QEMU setup and found the kernel sucking at a static branch
where it jumps to itself.
The reason is that the static branch was 2-byte but not 4-byte aligned.
Then, the kernel would patch the instruction, either J or NOP, with two
half-word stores if the machine does not have efficient unaligned
accesses. Thus, moments exist where half of the NOP mixes with the other
half of the J when transitioning the branch. In our particular case, on
a little-endian machine, the upper half of the NOP was mixed with the
lower part of the J when enabling the branch, resulting in a jump that
jumped to itself. Conversely, it would result in a HINT instruction when
disabling the branch, but it might not be observable.
ARM64 does not have this problem since all instructions must be 4-byte
aligned.
Fixes: ebc00dde8a97 ("riscv: Add jump-label implementation")
Link: https://lore.kernel.org/linux-riscv/20220913094252.3555240-6-andy.chiu@sifive.com/
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Greentime Hu <greentime.hu@sifive.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/include/asm/jump_label.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
index 6d58bbb5da46..14a5ea8d8ef0 100644
--- a/arch/riscv/include/asm/jump_label.h
+++ b/arch/riscv/include/asm/jump_label.h
@@ -18,6 +18,7 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
{
asm_volatile_goto(
+ " .align 2 \n\t"
" .option push \n\t"
" .option norelax \n\t"
" .option norvc \n\t"
@@ -39,6 +40,7 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
const bool branch)
{
asm_volatile_goto(
+ " .align 2 \n\t"
" .option push \n\t"
" .option norelax \n\t"
" .option norvc \n\t"
--
2.36.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: guoren@kernel.org
To: guoren@kernel.org, palmer@dabbelt.com, paul.walmsley@sifive.com,
mhiramat@kernel.org, conor.dooley@microchip.com,
penberg@kernel.org, mark.rutland@arm.com, jrtc27@jrtc27.com,
andy.chiu@sifive.com, zong.li@sifive.com
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
Greentime Hu <greentime.hu@sifive.com>
Subject: [PATCH -next V3 1/2] riscv: jump_label: Fixup unaligned arch_static_branch function
Date: Thu, 26 Jan 2023 12:06:06 -0500 [thread overview]
Message-ID: <20230126170607.1489141-2-guoren@kernel.org> (raw)
In-Reply-To: <20230126170607.1489141-1-guoren@kernel.org>
From: Andy Chiu <andy.chiu@sifive.com>
Runtime code patching must be done at a naturally aligned address, or we
may execute on a partial instruction.
We have encountered problems traced back to static jump functions during
the test. We switched the tracer randomly for every 1~5 seconds on a
dual-core QEMU setup and found the kernel sucking at a static branch
where it jumps to itself.
The reason is that the static branch was 2-byte but not 4-byte aligned.
Then, the kernel would patch the instruction, either J or NOP, with two
half-word stores if the machine does not have efficient unaligned
accesses. Thus, moments exist where half of the NOP mixes with the other
half of the J when transitioning the branch. In our particular case, on
a little-endian machine, the upper half of the NOP was mixed with the
lower part of the J when enabling the branch, resulting in a jump that
jumped to itself. Conversely, it would result in a HINT instruction when
disabling the branch, but it might not be observable.
ARM64 does not have this problem since all instructions must be 4-byte
aligned.
Fixes: ebc00dde8a97 ("riscv: Add jump-label implementation")
Link: https://lore.kernel.org/linux-riscv/20220913094252.3555240-6-andy.chiu@sifive.com/
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Greentime Hu <greentime.hu@sifive.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/include/asm/jump_label.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
index 6d58bbb5da46..14a5ea8d8ef0 100644
--- a/arch/riscv/include/asm/jump_label.h
+++ b/arch/riscv/include/asm/jump_label.h
@@ -18,6 +18,7 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
{
asm_volatile_goto(
+ " .align 2 \n\t"
" .option push \n\t"
" .option norelax \n\t"
" .option norvc \n\t"
@@ -39,6 +40,7 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
const bool branch)
{
asm_volatile_goto(
+ " .align 2 \n\t"
" .option push \n\t"
" .option norelax \n\t"
" .option norvc \n\t"
--
2.36.1
next prev parent reply other threads:[~2023-01-26 17:06 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-26 17:06 [PATCH -next V3 0/2] riscv: jump_label: Fixup & Optimization guoren
2023-01-26 17:06 ` guoren
2023-01-26 17:06 ` guoren [this message]
2023-01-26 17:06 ` [PATCH -next V3 1/2] riscv: jump_label: Fixup unaligned arch_static_branch function guoren
2023-01-30 11:57 ` Björn Töpel
2023-01-30 11:57 ` Björn Töpel
2023-01-31 13:35 ` Guo Ren
2023-01-31 13:35 ` Guo Ren
2023-02-06 8:09 ` Björn Töpel
2023-02-06 8:41 ` Guo Ren
2023-01-26 17:06 ` [PATCH -next V3 2/2] riscv: jump_label: Optimize the code size with compressed instruction guoren
2023-01-26 17:06 ` guoren
2023-02-22 15:00 ` [PATCH -next V3 0/2] riscv: jump_label: Fixup & Optimization patchwork-bot+linux-riscv
2023-02-22 15:00 ` patchwork-bot+linux-riscv
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=20230126170607.1489141-2-guoren@kernel.org \
--to=guoren@kernel.org \
--cc=andy.chiu@sifive.com \
--cc=conor.dooley@microchip.com \
--cc=greentime.hu@sifive.com \
--cc=jrtc27@jrtc27.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=mhiramat@kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=penberg@kernel.org \
--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 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.