Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 22 Feb 2022 15:15:26 -0800 (PST)
Subject:     Re: [PATCH v2 0/7] Module relocation fixes and asm/insn.h header
In-Reply-To: <20220131182720.236065-1-kernel@esmil.dk>
CC:     linux-riscv@lists.infradead.org, kernel@esmil.dk,
        Paul Walmsley <paul.walmsley@sifive.com>,
        aou@eecs.berkeley.edu, peterz@infradead.org, jpoimboe@redhat.com,
        jbaron@akamai.com, rostedt@goodmis.org, ardb@kernel.org,
        alex@ghiti.fr, jszhang@kernel.org, linux-kernel@vger.kernel.org
From:   Palmer Dabbelt <palmer@dabbelt.com>
To:     kernel@esmil.dk
Message-ID: <mhng-8e6926b7-c690-4398-a70a-072b11d7e6fa@palmer-ri-x1c9>
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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!