Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <mhng-0a2f9574-9b23-4f26-ae76-18ed7f2c8533@palmer-si-x1c4>
 <87d0yoizv9.fsf@xps13.shealevy.com> <87zi19gjof.fsf@xps13.shealevy.com>
In-Reply-To: <87zi19gjof.fsf@xps13.shealevy.com>
From:   =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn.topel@gmail.com>
Date:   Tue, 26 Nov 2019 14:14:22 +0100
Message-ID: <CAJ+HfNhoJnGon-L9OwSfrMbmUt1ZPBB_=A8ZFrg1CgEq3ua-Sg@mail.gmail.com>
Subject: Re: [PATCH] RISC-V: Load modules within relative jump range of the
 kernel text.
To:     Shea Levy <shea@shealevy.com>
Cc:     linux-riscv@lists.infradead.org, albert@sifive.com,
        LKML <linux-kernel@vger.kernel.org>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Netdev <netdev@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 9 May 2018 at 13:22, Shea Levy <shea@shealevy.com> wrote:
>
> Hi Palmer,
>
> Shea Levy <shea@shealevy.com> writes:
>
> > Hi Palmer,
> >
> > Palmer Dabbelt <palmer@sifive.com> writes:
> >
> >> On Sun, 22 Apr 2018 05:53:56 PDT (-0700), shea@shealevy.com wrote:
> >>> Hi Palmer,
> >>>
> >>> Shea Levy <shea@shealevy.com> writes:
> >>>
> >>>> Signed-off-by: Shea Levy <shea@shealevy.com>
> >>>> ---
> >>>>
> >>>> Note that this patch worked in my old modules patchset and seems to =
be
> >>>> working now, but my kernel boot locks up on top of
> >>>> riscv-for-linus-4.17-mw0 and I don't know if it's due to this patch =
or
> >>>> something else that's changed in the mean time.
> >>>>
> >>>> ---
> >>>>  arch/riscv/include/asm/pgtable.h |  9 +++++++++
> >>>>  arch/riscv/kernel/module.c       | 11 +++++++++++
> >>>>  2 files changed, 20 insertions(+)
> >>>>
> >>>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/a=
sm/pgtable.h
> >>>> index 16301966d65b..b08ded13364a 100644
> >>>> --- a/arch/riscv/include/asm/pgtable.h
> >>>> +++ b/arch/riscv/include/asm/pgtable.h
> >>>> @@ -25,6 +25,7 @@
> >>>>  #include <asm/page.h>
> >>>>  #include <asm/tlbflush.h>
> >>>>  #include <linux/mm_types.h>
> >>>> +#include <linux/sizes.h>
> >>>>
> >>>>  #ifdef CONFIG_64BIT
> >>>>  #include <asm/pgtable-64.h>
> >>>> @@ -425,6 +426,14 @@ static inline void pgtable_cache_init(void)
> >>>>  #define TASK_SIZE VMALLOC_START
> >>>>  #endif
> >>>>
> >>>> +/*
> >>>> + * The module space lives between the addresses given by TASK_SIZE
> >>>> + * and PAGE_OFFSET - it must be within 2G of the kernel text.
> >>>> + */
> >>>> +#define MODULES_SIZE              (SZ_128M)
> >>>> +#define MODULES_VADDR             (PAGE_OFFSET - MODULES_SIZE)
> >>>> +#define MODULES_END               (VMALLOC_END)
> >>>> +
> >>>>  #include <asm-generic/pgtable.h>
> >>>>
> >>>>  #endif /* !__ASSEMBLY__ */
> >>>> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> >>>> index 5dddba301d0a..1b382c7de095 100644
> >>>> --- a/arch/riscv/kernel/module.c
> >>>> +++ b/arch/riscv/kernel/module.c
> >>>> @@ -16,6 +16,8 @@
> >>>>  #include <linux/err.h>
> >>>>  #include <linux/errno.h>
> >>>>  #include <linux/moduleloader.h>
> >>>> +#include <linux/vmalloc.h>
> >>>> +#include <asm/pgtable.h>
> >>>>
> >>>>  static int apply_r_riscv_64_rela(struct module *me, u32 *location, =
Elf_Addr v)
> >>>>  {
> >>>> @@ -382,3 +384,12 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const=
 char *strtab,
> >>>>
> >>>>    return 0;
> >>>>  }
> >>>> +
> >>>> +void *module_alloc(unsigned long size)
> >>>> +{
> >>>> +  return __vmalloc_node_range(size, 1, MODULES_VADDR,
> >>>> +                              MODULES_END, GFP_KERNEL,
> >>>> +                              PAGE_KERNEL_EXEC, 0,
> >>>> +                              NUMA_NO_NODE,
> >>>> +                              __builtin_return_address(0));
> >>>> +}
> >>>> --
> >>>> 2.16.2
> >>>
> >>> Any thoughts on this?
> >>
> >> The concept looks good, but does this actually keep the modules within=
 2GiB of
> >> the text if PAGE_OFFSET is large?
> >
> > It's been some time since I wrote this, but I thought PAGE_OFFSET was
> > where the kernel text *started*? So unless the text itself is bigger
> > than 2G - 128 M, in which case we're SOL anyway, it seems like this
> > should work. Is there something better we can do, without a large memor=
y
> > model?
> >
> > Thanks,
> > Shea
>
> Any further thoughts on this?
>
> Thanks,
> Shea

Shea,

Waking up the dead (threads)!

I'm hacking on call improvements for the RISC-V BPF JIT.
module_alloc() is used under the hood of bpf_jit_binary_alloc(), which
in turn is used to allocate the JIT image. The current JIT
implementation has to to "load imm64 + jalr" to call kernel syms,
since the relative offset is >32b. With your patch, I can use regular
jal/auipc+jalr instead. IOW, it would be great if it could be merged.
;-) I'd prefer not having the patch in my BPF JIT series, since that
will go through a different tree than Paul's RV one.

Wdyt about brushing of the dust of the patch, and re-send it?


Thanks!
Bj=C3=B6rn