linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Luke Nelson <lukenels@cs.washington.edu>
To: Emil Renner Berthing <kernel@esmil.dk>
Cc: "Björn Töpel" <bjorn.topel@gmail.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"Zong Li" <zong@andestech.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Andreas Schwab" <schwab@suse.de>,
	linux-riscv@lists.infradead.org, "Xi Wang" <xi.wang@gmail.com>
Subject: Re: [PATCH v1 2/2] riscv: Clean up module relocations
Date: Thu, 30 Jul 2020 11:53:06 -0700	[thread overview]
Message-ID: <CADasFoDfRwcfFHM_Sa-HzwSDyXEr2PNu3Wfe5riwZJM9XsELBQ@mail.gmail.com> (raw)
In-Reply-To: <20200722152422.72532-2-kernel@esmil.dk>

Thanks for the patch!

> Also RISC-V has a number of instruction pairs to
> generate 32bit immediates or jump/call offsets. Eg.:
>
> lui   rd, hi20
> addi  rd, rd, lo12

On RV64, both hi20 from lui and lo12 from addi are sign-extended to 64 bits.
This means that there are some 32-bit signed offsets (in the range
[2^31-2^11, 2^31-1])
that are not encodable using (lui+addi), (auipc+jalr), etc. (see
discussion at [1]).

The following note is from the ISA manual:
>>> Note that the set of address offsets that can be formed by pairing LUI with LD,
>>> AUIPC with JALR, etc. in RV64I is [−2^31−2^11, 2^31−2^11−1].

The existing code and the new code both seem buggy if the offset happens to
be a 32-bit int but falls outside of the encodable range.

> +       if (offset != (s32)offset) {
> [...]
> +       if (offset != (s32)offset) {
> [...]

These checks should probably be replaced with something similar to
what's used in the RV64 BPF JIT here: [2],
except that this code should check if using RV32 or RV64, since the
encodable range differs for each.

> My hope is that we can eventually factor out the code to generate
> immediates and instructions so it can be reused both here, in the
> jump-label code and in the bpf-jit code, but let's take it
> one step at a time.

This sounds great! Having fewer copies of RISC-V encoding logic around will
hopefully decrease the likelihood of bugs :) Some other archs already
have shared
infrastructure for doing instruction encoding (e.g., in
arch/arm64/kernel/insn.c);
we should consider doing something similar for RISC-V.

- Luke Nelson

[1]: https://groups.google.com/a/groups.riscv.org/forum/#!topic/isa-dev/bwWFhBnnZFQ
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=489553dd13a88d8a882db10622ba8b9b58582ce4

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2020-07-30 18:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-22 15:24 [PATCH v1 1/2] riscv: Avoid unaligned access when relocating modules Emil Renner Berthing
2020-07-22 15:24 ` [PATCH v1 2/2] riscv: Clean up module relocations Emil Renner Berthing
2020-07-30 18:53   ` Luke Nelson [this message]
2020-08-05 17:28     ` Emil Renner Berthing

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=CADasFoDfRwcfFHM_Sa-HzwSDyXEr2PNu3Wfe5riwZJM9XsELBQ@mail.gmail.com \
    --to=lukenels@cs.washington.edu \
    --cc=bjorn.topel@gmail.com \
    --cc=kernel@esmil.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=schwab@suse.de \
    --cc=xi.wang@gmail.com \
    --cc=zong@andestech.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).