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;
MIME-Version: 1.0
References: <20230326170756.3021936-1-sdonthineni@nvidia.com>
 <CAMj1kXF4MUGbuiW1bcupMs1VdxxE_74WWDt1YdUTg0o9ft+=JQ@mail.gmail.com> <159ceeab-09af-3174-5058-445bc8dcf85b@nvidia.com>
In-Reply-To: <159ceeab-09af-3174-5058-445bc8dcf85b@nvidia.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Wed, 29 Mar 2023 18:07:07 +0200
Message-ID: <CAMj1kXGGUzzyN9BS7OeQcn3tvfmsH8Y_CJu5xQx-y7V7hEgXFA@mail.gmail.com>
Subject: Re: [PATCH] arm64: mm: Increase MODULES_VSIZE to 512MB
To:     Shanker Donthineni <sdonthineni@nvidia.com>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Mark Rutland <mark.rutland@arm.com>,
        Anshuman Khandual <anshuman.khandual@arm.com>,
        Kalesh Singh <kaleshsingh@google.com>,
        Zhou Guanghui <zhouguanghui1@huawei.com>,
        linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, Vikram Sethi <vsethi@nvidia.com>,
        Thierry Reding <treding@nvidia.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, 26 Mar 2023 at 20:59, Shanker Donthineni <sdonthineni@nvidia.com> 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@nvidia.com> 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@kernel.org>
> > 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));