Re: [PATCH] RISC-V: Load modules within relative jump range of the kernel text.

[Shea Levy]

Hi BjÃrn,

Unfortunately I'm not sure what more is needed to get this in, and I'm
in the middle of a move and won't have easy access to my RISC-V setup
for testing. I don't think you can count on me for this one.

Thanks,
Shea

BjÃrn TÃpel <bjorn.topel@xxxxxxxxx> writes:

> On Wed, 9 May 2018 at 13:22, Shea Levy <shea@xxxxxxxxxxxx> wrote:
>>
>> Hi Palmer,
>>
>> Shea Levy <shea@xxxxxxxxxxxx> writes:
>>
>> > Hi Palmer,
>> >
>> > Palmer Dabbelt <palmer@xxxxxxxxxx> writes:
>> >
>> >> On Sun, 22 Apr 2018 05:53:56 PDT (-0700), shea@xxxxxxxxxxxx wrote:
>> >>> Hi Palmer,
>> >>>
>> >>> Shea Levy <shea@xxxxxxxxxxxx> writes:
>> >>>
>> >>>> Signed-off-by: Shea Levy <shea@xxxxxxxxxxxx>
>> >>>> ---
>> >>>>
>> >>>> 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/asm/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 memory
>> > 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Ãrn

Attachment: signature.asc
Description: PGP signature