Messages in this thread | | | From | Guo Ren <> | Date | Tue, 31 Jan 2023 21:35:59 +0800 | Subject | Re: [PATCH -next V3 1/2] riscv: jump_label: Fixup unaligned arch_static_branch function |
| |
On Mon, Jan 30, 2023 at 7:57 PM Björn Töpel <bjorn@kernel.org> wrote: > > guoren@kernel.org writes: > > > 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. > > Reviewed-by: Björn Töpel <bjorn@kernel.org> > > Nice catch! And I guess this is an issue for kprobes as well, no? > I.e. in general replacing 32b insns with an ebreak. This is only valid > for natural aligned 32b insns? > > @Guo I don't see the point of doing a series for this, and asking the > maintainers to "pick this patch to stable, and the other for > next". Isn't that just more work for the maintainers/reviewers? If these two patches are separated, they are all fixup that issue and competed with each other. Making my patch an optimization patch must depend on it. That's why I put them in one series.
> > > Björn
-- Best Regards Guo Ren
| |