Re: [PATCH] arm64: mm: Increase MODULES_VSIZE to 512MB

[Shanker Donthineni]

On 3/29/23 11:07, Ard Biesheuvel wrote:
> External email: Use caution opening links or attachments
>
>
> On Sun, 26 Mar 2023 at 20:59, Shanker Donthineni <sdonthineni@xxxxxxxxxx> wrote:
> > Thanks Ard for a quick feedback.
> >
> > On 3/26/23 12:35, Ard Biesheuvel wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Sun, 26 Mar 2023 at 19:08, Shanker Donthineni <sdonthineni@xxxxxxxxxx> wrote:
> > > > The allocation of modules occurs in two regions: the first region
> > > > MODULES_VSIZE (128MB) is shared with the core kernel, while the
> > > > the second region (2GB) is shared with other vmalloc callers.
> > > > Depending on the size of the core kernel, the 128MB region may
> > > > quickly fill up after loading a few modules, causing the system
> > > > to switch to the 2GB region.
> > >
> > > How much module space are you actually using? This 128 MiB region is
> > > not shared with vmalloc() so it should be dedicated to modules
> > > entirely.
> > Is it correct to say that if the KASLR feature is disabled, 128MB is
> > being shared between the kernel and modules? Approximately 110MB used
> > by the NVIDIA GPU driver, resulting in the usage of more than 128MB.
> >
> > root@localhost:~# cat /proc/kallsyms | grep -wE '_etext|_stext|_end'
> > ffff8000081d0000 T _stext
> > ffff800009390000 D _etext
> > ffff80000b4d0000 B _end
> >
> > root@localhost:~# cat /proc/vmallocinfo | more
> > 0xffff800001390000-0xffff800001450000  786432 move_module+0x2c/0x190 pages=11 vmalloc N0=11
> > 0xffff800001450000-0xffff8000014b0000  393216 move_module+0x2c/0x190 pages=5 vmalloc N0=5
> > 0xffff8000014f0000-0xffff800001550000  393216 move_module+0x2c/0x190 pages=5 vmalloc N0=5
> > 0xffff800001590000-0xffff8000015f0000  393216 move_module+0x2c/0x190 pages=5 vmalloc N0=5
> > 0xffff800001630000-0xffff800001690000  393216 move_module+0x2c/0x190 pages=5 vmalloc N0=5
> > 0xffff8000016d0000-0xffff800001740000  458752 move_module+0x2c/0x190 pages=6 vmalloc N0=6
> > 0xffff800001780000-0xffff8000017e0000  393216 move_module+0x2c/0x190 pages=5 vmalloc N0=5
> > 0xffff800001820000-0xffff800001880000  393216 move_module+0x2c/0x190 pages=5 vmalloc N0=5
> > ...
> >
> > The first modules loaded at the address 0xffff800001390000.
> >
> > Less than 128MB is available for modules if KASLR is disabled.
> >
> > > If you are doing EFI boot, you may need to following patch to ensure
> > > that the 128 MiB region is actually the one being used.
> > >
> > > commit 010338d729c1090036eb40d2a60b7b7bce2445b8
> > > Author: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > Date:   Thu Feb 23 21:41:01 2023 +0100
> > >
> > >       arm64: kaslr: don't pretend KASLR is enabled if offset < MIN_KIMG_ALIGN
> > I have included your patch to prevent the incorrect detection of the
> > KASLR feature. Otherwise, experiencing the different error
> > "overflow in relocation type 261", R_AARCH64_PREL32. Seems this is
> > due to the incorrect initialization of module_alloc_base.
>
> Hmm, not sure - there was a report about this a while ago but I forgot
> the details.
>
> In any case, could we perhaps try something like the below? That way,
> we still prefer allocating from the 128 MiB region that is within
> direct branching range from the core kernel.
>
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -46,7 +46,7 @@
>   #define KIMAGE_VADDR           (MODULES_END)
>   #define MODULES_END            (MODULES_VADDR + MODULES_VSIZE)
>   #define MODULES_VADDR          (_PAGE_END(VA_BITS_MIN))
> -#define MODULES_VSIZE          (SZ_128M)
> +#define MODULES_VSIZE          (SZ_2G)
>   #define VMEMMAP_START          (-(UL(1) << (VA_BITS - VMEMMAP_SHIFT)))
>   #define VMEMMAP_END            (VMEMMAP_START + VMEMMAP_SIZE)
>   #define PCI_IO_END             (VMEMMAP_START - SZ_8M)
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
> index 5af4975caeb58ff7..b4affe775f23e84f 100644
> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -37,7 +37,7 @@ void *module_alloc(unsigned long size)
>                  /* don't exceed the static module region - see below */
>                  module_alloc_end = MODULES_END;
>
> -       p = __vmalloc_node_range(size, MODULE_ALIGN, module_alloc_base,
> +       p = __vmalloc_node_range(size, MODULE_ALIGN, module_alloc_end - SZ_128M,
>                                  module_alloc_end, gfp_mask,
> PAGE_KERNEL, VM_DEFER_KMEMLEAK,
>                                  NUMA_NO_NODE, __builtin_return_address(0));

Thanks Ard, I'll include your suggested changes in v2 patch.