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, greentime.hu@sifive.com, bjorn@kernel.org Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH RESEND] riscv: jump_label: Fixup unaligned arch_static_branch function Date: Mon, 6 Feb 2023 04:04:40 -0500 [thread overview] Message-ID: <20230206090440.1255001-1-guoren@kernel.org> (raw) 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/ Reviewed-by: Greentime Hu <greentime.hu@sifive.com> Signed-off-by: Andy Chiu <andy.chiu@sifive.com> Signed-off-by: Guo Ren <guoren@kernel.org> --- This patch is independent from: https://lore.kernel.org/linux-riscv/87pmangqpt.fsf@all.your.base.are.belong.to.us/ --- 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
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, greentime.hu@sifive.com, bjorn@kernel.org Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH RESEND] riscv: jump_label: Fixup unaligned arch_static_branch function Date: Mon, 6 Feb 2023 04:04:40 -0500 [thread overview] Message-ID: <20230206090440.1255001-1-guoren@kernel.org> (raw) 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/ Reviewed-by: Greentime Hu <greentime.hu@sifive.com> Signed-off-by: Andy Chiu <andy.chiu@sifive.com> Signed-off-by: Guo Ren <guoren@kernel.org> --- This patch is independent from: https://lore.kernel.org/linux-riscv/87pmangqpt.fsf@all.your.base.are.belong.to.us/ --- 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
next reply other threads:[~2023-02-06 9:04 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-06 9:04 guoren [this message] 2023-02-06 9:04 ` [PATCH RESEND] riscv: jump_label: Fixup unaligned arch_static_branch function guoren 2023-02-22 15:00 ` 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=20230206090440.1255001-1-guoren@kernel.org \ --to=guoren@kernel.org \ --cc=andy.chiu@sifive.com \ --cc=bjorn@kernel.org \ --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: linkBe 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.