| Message ID | 20220131182720.236065-1-kernel@esmil.dk (mailing list archive) |
|---|---|
| Headers | show |
| Series | Module relocation fixes and asm/insn.h header | expand |
On Mon, 31 Jan 2022 10:27:13 PST (-0800), kernel@esmil.dk wrote: > Apologies! I messed up v1. Please consider this patch set only. > > The first patch removes a bunch of code from the asm/module.h which is > included in almost all drivers through linux/module.h. Next are two > patches to fix unaligned access when doing module relocations and do > proper range checks for auipc+jalr offsets. > > I'm a little less confident about the following patches, so consider > this more of an RFC for those. The idea is to consolidate the RISC-V > instruction generation and manipulation similar to arm64's asm/insn.h > header. > > /Emil > > Emil Renner Berthing (7): > riscv: Remove unneeded definitions from asm/module.h > riscv: Avoid unaligned access when relocating modules > riscv: Fix auipc+jalr relocation range checks > riscv: Add asm/insn.h header > riscv: Use asm/insn.h for module relocations > riscv: Use asm/insn.h to generate plt entries > riscv: Use asm/insn.h for jump labels > > arch/riscv/include/asm/insn.h | 121 ++++++++++++++ > arch/riscv/include/asm/module.h | 87 ---------- > arch/riscv/kernel/jump_label.c | 12 +- > arch/riscv/kernel/module-sections.c | 71 +++++++++ > arch/riscv/kernel/module.c | 237 +++++++++++++--------------- > 5 files changed, 306 insertions(+), 222 deletions(-) > create mode 100644 arch/riscv/include/asm/insn.h These generally look good to me, though there's a lot of bit-field twiddling so I'll take another look before merging it. There's a handful of minor issues: * There's a fix in here, mixed into the cleanups. It's generally best to split those out. * There's another copy of the insn patterns in our BPF JIT, it'd be nice to clean that up too. That can be a follow-on, though. * It's 2022, but there's some 2020 copyrights. If this really is old stuff that's OK, I just wanted to check. I'm usually OK just re-ordering patches myself, but I figured I'd have to ask about the copyright dates anyway. LMK if you want to send a v2 with the fix pulled to the front, and what you want me to do about the copyright dates (if you're going to send a v2 then just fix them, but if you're not then just telling me is OK). Thanks!
On Wed, 23 Feb 2022 at 00:15, Palmer Dabbelt <palmer@dabbelt.com> wrote: > On Mon, 31 Jan 2022 10:27:13 PST (-0800), kernel@esmil.dk wrote: > > Apologies! I messed up v1. Please consider this patch set only. > > > > The first patch removes a bunch of code from the asm/module.h which is > > included in almost all drivers through linux/module.h. Next are two > > patches to fix unaligned access when doing module relocations and do > > proper range checks for auipc+jalr offsets. > > > > I'm a little less confident about the following patches, so consider > > this more of an RFC for those. The idea is to consolidate the RISC-V > > instruction generation and manipulation similar to arm64's asm/insn.h > > header. > > > > /Emil > > > > Emil Renner Berthing (7): > > riscv: Remove unneeded definitions from asm/module.h > > riscv: Avoid unaligned access when relocating modules > > riscv: Fix auipc+jalr relocation range checks > > riscv: Add asm/insn.h header > > riscv: Use asm/insn.h for module relocations > > riscv: Use asm/insn.h to generate plt entries > > riscv: Use asm/insn.h for jump labels > > > > arch/riscv/include/asm/insn.h | 121 ++++++++++++++ > > arch/riscv/include/asm/module.h | 87 ---------- > > arch/riscv/kernel/jump_label.c | 12 +- > > arch/riscv/kernel/module-sections.c | 71 +++++++++ > > arch/riscv/kernel/module.c | 237 +++++++++++++--------------- > > 5 files changed, 306 insertions(+), 222 deletions(-) > > create mode 100644 arch/riscv/include/asm/insn.h > > These generally look good to me, though there's a lot of bit-field > twiddling so I'll take another look before merging it. There's a > handful of minor issues: > > * There's a fix in here, mixed into the cleanups. It's generally best > to split those out. There are two fixes. The 32bit range check on rv64 and unaligned 32bit access. The code has been like that for years so I was unsure if they were worth splitting out and adding early. Since you only mention one I guess that's the range check. I'll send that separately. > * There's another copy of the insn patterns in our BPF JIT, it'd be nice > to clean that up too. That can be a follow-on, though. > * It's 2022, but there's some 2020 copyrights. If this really is old > stuff that's OK, I just wanted to check. Nice catch, but the year is actually correct. These patches have been well aged in my local repo. The reason is exactly that I never got around to doing the BPF conversion, so now I decided to just send them and see if it was worth finishing. > I'm usually OK just re-ordering patches myself, but I figured I'd have > to ask about the copyright dates anyway. LMK if you want to send a v2 > with the fix pulled to the front, and what you want me to do about the > copyright dates (if you're going to send a v2 then just fix them, but if > you're not then just telling me is OK). Thank you. I'll send the range check separately and a v2 converting the "if (IS_ENABLED(CONFIG_32BIT))" to an #ifdef to avoid the warning the kernel test robot found.