Message ID | 20240301203023.2197451-1-jsavitz@redhat.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | powerpc: align memory_limit to 16MB in early_parse_mem | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
Hi Joel, Joel Savitz <jsavitz@redhat.com> writes: > On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel > cmdline parameter results in a system hang at boot. Can you give us any more details on that? It might be a bug we can fix. > For example, using 'mem=4198400K' will always reproduce this issue. > > This patch fixes the problem by aligning any argument to mem= to 16MB > corresponding with the large page size on powerpc. The large page size depends on the MMU, with Radix it's 2MB or 1GB. So depending on what's happening 16MB may not be enough. What system are you testing on? cheers > Fixes: 2babf5c2ec2f ("[PATCH] powerpc: Unify mem= handling") > Co-developed-by: Gonzalo Siero <gsierohu@redhat.com> > Signed-off-by: Gonzalo Siero <gsierohu@redhat.com> > Signed-off-by: Joel Savitz <jsavitz@redhat.com> > --- > arch/powerpc/kernel/prom.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 0b5878c3125b..8cd3e2445d8a 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) > { > if (!p) > return 1; > - > +#ifdef CONFIG_PPC64 > + /* Align to 16 MB == size of ppc64 large page */ > + memory_limit = ALIGN(memparse(p, &p), 0x1000000); > +#else > memory_limit = PAGE_ALIGN(memparse(p, &p)); > +#endif > DBG("memory limit = 0x%llx\n", memory_limit); > > return 0; > -- > 2.43.0
On Fri, Mar 1, 2024 at 6:23 PM Michael Ellerman <mpe@ellerman.id.au> wrote: > > Hi Joel, > > Joel Savitz <jsavitz@redhat.com> writes: > > On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel > > cmdline parameter results in a system hang at boot. > > Can you give us any more details on that? It might be a bug we can fix. The console freezes after the following output: Booting a command list OF stdout device is: /vdevice/vty@30000000 Preparing to boot Linux version 6.8.0-rc6.memNOfix-00120-g87adedeba51a (root@ibm-p9z-26-lp11.virt.pnr.lab.eng.rdu2.redhat.com) (gcc (GCC) 11.4.1 20231218 (Red Hat 11.4.1-3), GNU ld version 2.35.2-43.el9) #3 SMP Fri Mar 1 10:45:45 EST 2024 Detected machine type: 0000000000000101 command line: BOOT_IMAGE=(ieee1275//vdevice/v-scsi@30000003/disk@8100000000000000,msdos2)/vmlinuz-6.8.0-rc6.memNOfix-00120-g87adedeba51a root=/dev/mapper/rhel_ibm--p9z--26--lp11-root ro crashkernel=2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G rd.lvm.lv=rhel_ibm-p9z-26-lp11/root rd.lvm.lv=rhel_ibm-p9z-26-lp11/swap mem=4198400K Max number of cores passed to firmware: 256 (NR_CPUS = 2048) Calling ibm,client-architecture-support... done Ignoring mem=0000000101000000 >= ram_top. memory layout at init: memory_limit : 0000000000000000 (16 MB aligned) alloc_bottom : 00000000114f0000 alloc_top : 0000000020000000 alloc_top_hi : 0000000020000000 rmo_top : 0000000020000000 ram_top : 0000000020000000 instantiating rtas at 0x000000001ecb0000... done prom_hold_cpus: skipped copying OF device tree... Building dt strings... Building dt structure... Device tree strings 0x0000000011500000 -> 0x00000000115017b7 Device tree struct 0x0000000011510000 -> 0x0000000011520000 Quiescing Open Firmware ... Booting Linux via __start() @ 0x000000000a6e0000 ... > > > For example, using 'mem=4198400K' will always reproduce this issue. > > > > This patch fixes the problem by aligning any argument to mem= to 16MB > > corresponding with the large page size on powerpc. > > The large page size depends on the MMU, with Radix it's 2MB or 1GB. So > depending on what's happening 16MB may not be enough. > > What system are you testing on? I'm running a virtual system in PowerVM on an IBM Z mainframe, 8375-42A model. Best, Joel Savitz > > cheers > > > Fixes: 2babf5c2ec2f ("[PATCH] powerpc: Unify mem= handling") > > Co-developed-by: Gonzalo Siero <gsierohu@redhat.com> > > Signed-off-by: Gonzalo Siero <gsierohu@redhat.com> > > Signed-off-by: Joel Savitz <jsavitz@redhat.com> > > --- > > arch/powerpc/kernel/prom.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > > index 0b5878c3125b..8cd3e2445d8a 100644 > > --- a/arch/powerpc/kernel/prom.c > > +++ b/arch/powerpc/kernel/prom.c > > @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) > > { > > if (!p) > > return 1; > > - > > +#ifdef CONFIG_PPC64 > > + /* Align to 16 MB == size of ppc64 large page */ > > + memory_limit = ALIGN(memparse(p, &p), 0x1000000); > > +#else > > memory_limit = PAGE_ALIGN(memparse(p, &p)); > > +#endif > > DBG("memory limit = 0x%llx\n", memory_limit); > > > > return 0; > > -- > > 2.43.0 >
On 3/2/24 4:53 AM, Michael Ellerman wrote: > Hi Joel, > > Joel Savitz <jsavitz@redhat.com> writes: >> On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel >> cmdline parameter results in a system hang at boot. > > Can you give us any more details on that? It might be a bug we can fix. > >> For example, using 'mem=4198400K' will always reproduce this issue. >> >> This patch fixes the problem by aligning any argument to mem= to 16MB >> corresponding with the large page size on powerpc. > > The large page size depends on the MMU, with Radix it's 2MB or 1GB. So > depending on what's happening 16MB may not be enough. > > What system are you testing on? > htab_bolt_mapping should have aligned things to a lower value that is 16MB aligned. /* Carefully map only the possible range */ vaddr = ALIGN(vstart, step); paddr = ALIGN(pstart, step); vend = ALIGN_DOWN(vend, step); -aneesh
Joel Savitz <jsavitz@redhat.com> writes: > On Fri, Mar 1, 2024 at 6:23 PM Michael Ellerman <mpe@ellerman.id.au> wrote: >> Joel Savitz <jsavitz@redhat.com> writes: >> > On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel >> > cmdline parameter results in a system hang at boot. >> >> Can you give us any more details on that? It might be a bug we can fix. > > The console freezes after the following output: > > Booting a command list > > OF stdout device is: /vdevice/vty@30000000 > Preparing to boot Linux version 6.8.0-rc6.memNOfix-00120-g87adedeba51a > (root@ibm-p9z-26-lp11.virt.pnr.lab.eng.rdu2.redhat.com) (gcc (GCC) > 11.4.1 20231218 (Red Hat 11.4.1-3), GNU ld version 2.35.2-43.el9) #3 > SMP Fri Mar 1 10:45:45 EST 2024 > Detected machine type: 0000000000000101 > command line: BOOT_IMAGE=(ieee1275//vdevice/v-scsi@30000003/disk@8100000000000000,msdos2)/vmlinuz-6.8.0-rc6.memNOfix-00120-g87adedeba51a > root=/dev/mapper/rhel_ibm--p9z--26--lp11-root ro > crashkernel=2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G > rd.lvm.lv=rhel_ibm-p9z-26-lp11/root > rd.lvm.lv=rhel_ibm-p9z-26-lp11/swap mem=4198400K > Max number of cores passed to firmware: 256 (NR_CPUS = 2048) > Calling ibm,client-architecture-support... done > Ignoring mem=0000000101000000 >= ram_top. > memory layout at init: > memory_limit : 0000000000000000 (16 MB aligned) > alloc_bottom : 00000000114f0000 > alloc_top : 0000000020000000 > alloc_top_hi : 0000000020000000 > rmo_top : 0000000020000000 > ram_top : 0000000020000000 > instantiating rtas at 0x000000001ecb0000... done > prom_hold_cpus: skipped > copying OF device tree... > Building dt strings... > Building dt structure... > Device tree strings 0x0000000011500000 -> 0x00000000115017b7 > Device tree struct 0x0000000011510000 -> 0x0000000011520000 > Quiescing Open Firmware ... > Booting Linux via __start() @ 0x000000000a6e0000 ... Thanks. I haven't been able to reproduce this unfortunately, and I don't see the bug. As Aneesh pointed out the code should be aligning later anyway. Can you build a kernel with CONFIG_PPC_EARLY_DEBUG_LPAR=y and boot it without the patch? That should hopefully give you some more output. cheers
Joel Savitz <jsavitz@redhat.com> writes: > On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel > cmdline parameter results in a system hang at boot. > > For example, using 'mem=4198400K' will always reproduce this issue. > > This patch fixes the problem by aligning any argument to mem= to 16MB > corresponding with the large page size on powerpc. > > Fixes: 2babf5c2ec2f ("[PATCH] powerpc: Unify mem= handling") > Co-developed-by: Gonzalo Siero <gsierohu@redhat.com> > Signed-off-by: Gonzalo Siero <gsierohu@redhat.com> > Signed-off-by: Joel Savitz <jsavitz@redhat.com> > --- > arch/powerpc/kernel/prom.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 0b5878c3125b..8cd3e2445d8a 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) > { > if (!p) > return 1; > - > +#ifdef CONFIG_PPC64 > + /* Align to 16 MB == size of ppc64 large page */ > + memory_limit = ALIGN(memparse(p, &p), 0x1000000); > +#else > memory_limit = PAGE_ALIGN(memparse(p, &p)); > +#endif > DBG("memory limit = 0x%llx\n", memory_limit); > > return 0; > -- > 2.43.0 Can you try this change? commit 5555bc55e1aa71f545cff31e1eccdb4a2e39df84 Author: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> Date: Fri Mar 8 14:45:26 2024 +0530 powerpc/mm: Align memory_limit value specified using mem= kernel parameter The value specified for the memory limit is used to set a restriction on memory usage. It is important to ensure that this restriction is within the linear map kernel address space range. The hash page table translation uses a 16MB page size to map the kernel linear map address space. htab_bolt_mapping() function aligns down the size of the range while mapping kernel linear address space. Since the memblock limit is enforced very early during boot, before we can detect the type of memory translation (radix vs hash), we align the memory limit value specified as a kernel parameter to 16MB. This alignment value will work for both hash and radix translations. Signed-off-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 0b5878c3125b..9bd965d35352 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -824,8 +824,11 @@ void __init early_init_devtree(void *params) reserve_crashkernel(); early_reserve_mem(); - /* Ensure that total memory size is page-aligned. */ - limit = ALIGN(memory_limit ?: memblock_phys_mem_size(), PAGE_SIZE); + if (memory_limit > memblock_phys_mem_size()) + memory_limit = 0; + + /* Align down to 16 MB which is large page size with hash page translation */ + limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(), SZ_16M); memblock_enforce_memory_limit(limit); #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_4K_PAGES) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index e67effdba85c..d6410549e141 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -817,8 +817,8 @@ static void __init early_cmdline_parse(void) opt += 4; prom_memory_limit = prom_memparse(opt, (const char **)&opt); #ifdef CONFIG_PPC64 - /* Align to 16 MB == size of ppc64 large page */ - prom_memory_limit = ALIGN(prom_memory_limit, 0x1000000); + /* Align down to 16 MB which is large page size with hash page translation */ + prom_memory_limit = ALIGN_DOWN(prom_memory_limit, SZ_16M); #endif }
On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote: > > Joel Savitz <jsavitz@redhat.com> writes: > > > On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel > > cmdline parameter results in a system hang at boot. > > > > For example, using 'mem=4198400K' will always reproduce this issue. > > > > This patch fixes the problem by aligning any argument to mem= to 16MB > > corresponding with the large page size on powerpc. > > > > Fixes: 2babf5c2ec2f ("[PATCH] powerpc: Unify mem= handling") > > Co-developed-by: Gonzalo Siero <gsierohu@redhat.com> > > Signed-off-by: Gonzalo Siero <gsierohu@redhat.com> > > Signed-off-by: Joel Savitz <jsavitz@redhat.com> > > --- > > arch/powerpc/kernel/prom.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > > index 0b5878c3125b..8cd3e2445d8a 100644 > > --- a/arch/powerpc/kernel/prom.c > > +++ b/arch/powerpc/kernel/prom.c > > @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) > > { > > if (!p) > > return 1; > > - > > +#ifdef CONFIG_PPC64 > > + /* Align to 16 MB == size of ppc64 large page */ > > + memory_limit = ALIGN(memparse(p, &p), 0x1000000); > > +#else > > memory_limit = PAGE_ALIGN(memparse(p, &p)); > > +#endif > > DBG("memory limit = 0x%llx\n", memory_limit); > > > > return 0; > > -- > > 2.43.0 > > Can you try this change? > > commit 5555bc55e1aa71f545cff31e1eccdb4a2e39df84 > Author: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> > Date: Fri Mar 8 14:45:26 2024 +0530 > > powerpc/mm: Align memory_limit value specified using mem= kernel parameter > > The value specified for the memory limit is used to set a restriction on > memory usage. It is important to ensure that this restriction is within > the linear map kernel address space range. The hash page table > translation uses a 16MB page size to map the kernel linear map address > space. htab_bolt_mapping() function aligns down the size of the range > while mapping kernel linear address space. Since the memblock limit is > enforced very early during boot, before we can detect the type of memory > translation (radix vs hash), we align the memory limit value specified > as a kernel parameter to 16MB. This alignment value will work for both > hash and radix translations. > > Signed-off-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 0b5878c3125b..9bd965d35352 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -824,8 +824,11 @@ void __init early_init_devtree(void *params) > reserve_crashkernel(); > early_reserve_mem(); > > - /* Ensure that total memory size is page-aligned. */ > - limit = ALIGN(memory_limit ?: memblock_phys_mem_size(), PAGE_SIZE); > + if (memory_limit > memblock_phys_mem_size()) > + memory_limit = 0; > + > + /* Align down to 16 MB which is large page size with hash page translation */ > + limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(), SZ_16M); > memblock_enforce_memory_limit(limit); > > #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_4K_PAGES) > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c > index e67effdba85c..d6410549e141 100644 > --- a/arch/powerpc/kernel/prom_init.c > +++ b/arch/powerpc/kernel/prom_init.c > @@ -817,8 +817,8 @@ static void __init early_cmdline_parse(void) > opt += 4; > prom_memory_limit = prom_memparse(opt, (const char **)&opt); > #ifdef CONFIG_PPC64 > - /* Align to 16 MB == size of ppc64 large page */ > - prom_memory_limit = ALIGN(prom_memory_limit, 0x1000000); > + /* Align down to 16 MB which is large page size with hash page translation */ > + prom_memory_limit = ALIGN_DOWN(prom_memory_limit, SZ_16M); > #endif > } > > Sorry for the delayed reply. I just tested this patch and it fixes the bug for me.
On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <jsavitz@redhat.com> wrote: > > On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote: > > > > Joel Savitz <jsavitz@redhat.com> writes: > > > > > On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel > > > cmdline parameter results in a system hang at boot. > > > > > > For example, using 'mem=4198400K' will always reproduce this issue. > > > > > > This patch fixes the problem by aligning any argument to mem= to 16MB > > > corresponding with the large page size on powerpc. > > > > > > Fixes: 2babf5c2ec2f ("[PATCH] powerpc: Unify mem= handling") > > > Co-developed-by: Gonzalo Siero <gsierohu@redhat.com> > > > Signed-off-by: Gonzalo Siero <gsierohu@redhat.com> > > > Signed-off-by: Joel Savitz <jsavitz@redhat.com> > > > --- > > > arch/powerpc/kernel/prom.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > > > index 0b5878c3125b..8cd3e2445d8a 100644 > > > --- a/arch/powerpc/kernel/prom.c > > > +++ b/arch/powerpc/kernel/prom.c > > > @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) > > > { > > > if (!p) > > > return 1; > > > - > > > +#ifdef CONFIG_PPC64 > > > + /* Align to 16 MB == size of ppc64 large page */ > > > + memory_limit = ALIGN(memparse(p, &p), 0x1000000); > > > +#else > > > memory_limit = PAGE_ALIGN(memparse(p, &p)); > > > +#endif > > > DBG("memory limit = 0x%llx\n", memory_limit); > > > > > > return 0; > > > -- > > > 2.43.0 > > > > Can you try this change? > > > > commit 5555bc55e1aa71f545cff31e1eccdb4a2e39df84 > > Author: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> > > Date: Fri Mar 8 14:45:26 2024 +0530 > > > > powerpc/mm: Align memory_limit value specified using mem= kernel parameter > > > > The value specified for the memory limit is used to set a restriction on > > memory usage. It is important to ensure that this restriction is within > > the linear map kernel address space range. The hash page table > > translation uses a 16MB page size to map the kernel linear map address > > space. htab_bolt_mapping() function aligns down the size of the range > > while mapping kernel linear address space. Since the memblock limit is > > enforced very early during boot, before we can detect the type of memory > > translation (radix vs hash), we align the memory limit value specified > > as a kernel parameter to 16MB. This alignment value will work for both > > hash and radix translations. > > > > Signed-off-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> > > > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > > index 0b5878c3125b..9bd965d35352 100644 > > --- a/arch/powerpc/kernel/prom.c > > +++ b/arch/powerpc/kernel/prom.c > > @@ -824,8 +824,11 @@ void __init early_init_devtree(void *params) > > reserve_crashkernel(); > > early_reserve_mem(); > > > > - /* Ensure that total memory size is page-aligned. */ > > - limit = ALIGN(memory_limit ?: memblock_phys_mem_size(), PAGE_SIZE); > > + if (memory_limit > memblock_phys_mem_size()) > > + memory_limit = 0; > > + > > + /* Align down to 16 MB which is large page size with hash page translation */ > > + limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(), SZ_16M); > > memblock_enforce_memory_limit(limit); > > > > #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_4K_PAGES) > > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c > > index e67effdba85c..d6410549e141 100644 > > --- a/arch/powerpc/kernel/prom_init.c > > +++ b/arch/powerpc/kernel/prom_init.c > > @@ -817,8 +817,8 @@ static void __init early_cmdline_parse(void) > > opt += 4; > > prom_memory_limit = prom_memparse(opt, (const char **)&opt); > > #ifdef CONFIG_PPC64 > > - /* Align to 16 MB == size of ppc64 large page */ > > - prom_memory_limit = ALIGN(prom_memory_limit, 0x1000000); > > + /* Align down to 16 MB which is large page size with hash page translation */ > > + prom_memory_limit = ALIGN_DOWN(prom_memory_limit, SZ_16M); > > #endif > > } > > > > > > Sorry for the delayed reply. I just tested this patch and it fixes the > bug for me. Hi, Just a quick follow up on this. The above patch fixed the bug for me. How do we want to proceed? Best, Joel Savitz
On Mon, Apr 1, 2024 at 10:17 AM Joel Savitz <jsavitz@redhat.com> wrote: > > On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <jsavitz@redhat.com> wrote: > > > > On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote: > > > > > > Joel Savitz <jsavitz@redhat.com> writes: > > > > > > > On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel > > > > cmdline parameter results in a system hang at boot. > > > > > > > > For example, using 'mem=4198400K' will always reproduce this issue. > > > > > > > > This patch fixes the problem by aligning any argument to mem= to 16MB > > > > corresponding with the large page size on powerpc. > > > > > > > > Fixes: 2babf5c2ec2f ("[PATCH] powerpc: Unify mem= handling") > > > > Co-developed-by: Gonzalo Siero <gsierohu@redhat.com> > > > > Signed-off-by: Gonzalo Siero <gsierohu@redhat.com> > > > > Signed-off-by: Joel Savitz <jsavitz@redhat.com> > > > > --- > > > > arch/powerpc/kernel/prom.c | 6 +++++- > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > > > > index 0b5878c3125b..8cd3e2445d8a 100644 > > > > --- a/arch/powerpc/kernel/prom.c > > > > +++ b/arch/powerpc/kernel/prom.c > > > > @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) > > > > { > > > > if (!p) > > > > return 1; > > > > - > > > > +#ifdef CONFIG_PPC64 > > > > + /* Align to 16 MB == size of ppc64 large page */ > > > > + memory_limit = ALIGN(memparse(p, &p), 0x1000000); > > > > +#else > > > > memory_limit = PAGE_ALIGN(memparse(p, &p)); > > > > +#endif > > > > DBG("memory limit = 0x%llx\n", memory_limit); > > > > > > > > return 0; > > > > -- > > > > 2.43.0 > > > > > > Can you try this change? > > > > > > commit 5555bc55e1aa71f545cff31e1eccdb4a2e39df84 > > > Author: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> > > > Date: Fri Mar 8 14:45:26 2024 +0530 > > > > > > powerpc/mm: Align memory_limit value specified using mem= kernel parameter > > > > > > The value specified for the memory limit is used to set a restriction on > > > memory usage. It is important to ensure that this restriction is within > > > the linear map kernel address space range. The hash page table > > > translation uses a 16MB page size to map the kernel linear map address > > > space. htab_bolt_mapping() function aligns down the size of the range > > > while mapping kernel linear address space. Since the memblock limit is > > > enforced very early during boot, before we can detect the type of memory > > > translation (radix vs hash), we align the memory limit value specified > > > as a kernel parameter to 16MB. This alignment value will work for both > > > hash and radix translations. > > > > > > Signed-off-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> > > > > > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > > > index 0b5878c3125b..9bd965d35352 100644 > > > --- a/arch/powerpc/kernel/prom.c > > > +++ b/arch/powerpc/kernel/prom.c > > > @@ -824,8 +824,11 @@ void __init early_init_devtree(void *params) > > > reserve_crashkernel(); > > > early_reserve_mem(); > > > > > > - /* Ensure that total memory size is page-aligned. */ > > > - limit = ALIGN(memory_limit ?: memblock_phys_mem_size(), PAGE_SIZE); > > > + if (memory_limit > memblock_phys_mem_size()) > > > + memory_limit = 0; > > > + > > > + /* Align down to 16 MB which is large page size with hash page translation */ > > > + limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(), SZ_16M); > > > memblock_enforce_memory_limit(limit); > > > > > > #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_4K_PAGES) > > > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c > > > index e67effdba85c..d6410549e141 100644 > > > --- a/arch/powerpc/kernel/prom_init.c > > > +++ b/arch/powerpc/kernel/prom_init.c > > > @@ -817,8 +817,8 @@ static void __init early_cmdline_parse(void) > > > opt += 4; > > > prom_memory_limit = prom_memparse(opt, (const char **)&opt); > > > #ifdef CONFIG_PPC64 > > > - /* Align to 16 MB == size of ppc64 large page */ > > > - prom_memory_limit = ALIGN(prom_memory_limit, 0x1000000); > > > + /* Align down to 16 MB which is large page size with hash page translation */ > > > + prom_memory_limit = ALIGN_DOWN(prom_memory_limit, SZ_16M); > > > #endif > > > } > > > > > > > > > > Sorry for the delayed reply. I just tested this patch and it fixes the > > bug for me. > > Hi, > > Just a quick follow up on this. > > The above patch fixed the bug for me. > > How do we want to proceed? > > Best, > Joel Savitz Hi, I haven't heard anything on this thread so I'm just sending a quick follow up. Do we want to merge this Best, Joel Savitz
Le 10/04/2024 à 17:22, Joel Savitz a écrit : > [Vous ne recevez pas souvent de courriers de jsavitz@redhat.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > On Mon, Apr 1, 2024 at 10:17 AM Joel Savitz <jsavitz@redhat.com> wrote: >> >> On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <jsavitz@redhat.com> wrote: >>> >>> On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote: >>>> >>>> Joel Savitz <jsavitz@redhat.com> writes: >>>> >>>>> On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel >>>>> cmdline parameter results in a system hang at boot. >>>>> >>>>> For example, using 'mem=4198400K' will always reproduce this issue. >>>>> >>>>> This patch fixes the problem by aligning any argument to mem= to 16MB >>>>> corresponding with the large page size on powerpc. >>>>> >>>>> Fixes: 2babf5c2ec2f ("[PATCH] powerpc: Unify mem= handling") >>>>> Co-developed-by: Gonzalo Siero <gsierohu@redhat.com> >>>>> Signed-off-by: Gonzalo Siero <gsierohu@redhat.com> >>>>> Signed-off-by: Joel Savitz <jsavitz@redhat.com> >>>>> --- >>>>> arch/powerpc/kernel/prom.c | 6 +++++- >>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c >>>>> index 0b5878c3125b..8cd3e2445d8a 100644 >>>>> --- a/arch/powerpc/kernel/prom.c >>>>> +++ b/arch/powerpc/kernel/prom.c >>>>> @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) >>>>> { >>>>> if (!p) >>>>> return 1; >>>>> - >>>>> +#ifdef CONFIG_PPC64 >>>>> + /* Align to 16 MB == size of ppc64 large page */ >>>>> + memory_limit = ALIGN(memparse(p, &p), 0x1000000); >>>>> +#else >>>>> memory_limit = PAGE_ALIGN(memparse(p, &p)); >>>>> +#endif >>>>> DBG("memory limit = 0x%llx\n", memory_limit); >>>>> >>>>> return 0; >>>>> -- >>>>> 2.43.0 >>>> >>>> Can you try this change? >>>> >>>> commit 5555bc55e1aa71f545cff31e1eccdb4a2e39df84 >>>> Author: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> >>>> Date: Fri Mar 8 14:45:26 2024 +0530 >>>> >>>> powerpc/mm: Align memory_limit value specified using mem= kernel parameter >>>> >>>> The value specified for the memory limit is used to set a restriction on >>>> memory usage. It is important to ensure that this restriction is within >>>> the linear map kernel address space range. The hash page table >>>> translation uses a 16MB page size to map the kernel linear map address >>>> space. htab_bolt_mapping() function aligns down the size of the range >>>> while mapping kernel linear address space. Since the memblock limit is >>>> enforced very early during boot, before we can detect the type of memory >>>> translation (radix vs hash), we align the memory limit value specified >>>> as a kernel parameter to 16MB. This alignment value will work for both >>>> hash and radix translations. >>>> >>>> Signed-off-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> >>>> >>>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c >>>> index 0b5878c3125b..9bd965d35352 100644 >>>> --- a/arch/powerpc/kernel/prom.c >>>> +++ b/arch/powerpc/kernel/prom.c >>>> @@ -824,8 +824,11 @@ void __init early_init_devtree(void *params) >>>> reserve_crashkernel(); >>>> early_reserve_mem(); >>>> >>>> - /* Ensure that total memory size is page-aligned. */ >>>> - limit = ALIGN(memory_limit ?: memblock_phys_mem_size(), PAGE_SIZE); >>>> + if (memory_limit > memblock_phys_mem_size()) >>>> + memory_limit = 0; >>>> + >>>> + /* Align down to 16 MB which is large page size with hash page translation */ >>>> + limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(), SZ_16M); >>>> memblock_enforce_memory_limit(limit); >>>> >>>> #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_4K_PAGES) >>>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c >>>> index e67effdba85c..d6410549e141 100644 >>>> --- a/arch/powerpc/kernel/prom_init.c >>>> +++ b/arch/powerpc/kernel/prom_init.c >>>> @@ -817,8 +817,8 @@ static void __init early_cmdline_parse(void) >>>> opt += 4; >>>> prom_memory_limit = prom_memparse(opt, (const char **)&opt); >>>> #ifdef CONFIG_PPC64 >>>> - /* Align to 16 MB == size of ppc64 large page */ >>>> - prom_memory_limit = ALIGN(prom_memory_limit, 0x1000000); >>>> + /* Align down to 16 MB which is large page size with hash page translation */ >>>> + prom_memory_limit = ALIGN_DOWN(prom_memory_limit, SZ_16M); >>>> #endif >>>> } >>>> >>>> >>> >>> Sorry for the delayed reply. I just tested this patch and it fixes the >>> bug for me. >> >> Hi, >> >> Just a quick follow up on this. >> >> The above patch fixed the bug for me. >> >> How do we want to proceed? >> >> Best, >> Joel Savitz > > Hi, > > I haven't heard anything on this thread so I'm just sending a quick follow up. > > Do we want to merge this > Is it the same as https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20240403083611.172833-1-aneesh.kumar@kernel.org/ ?
On Wed, Apr 10, 2024 at 11:31 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > > > Le 10/04/2024 à 17:22, Joel Savitz a écrit : > > [Vous ne recevez pas souvent de courriers de jsavitz@redhat.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > > > On Mon, Apr 1, 2024 at 10:17 AM Joel Savitz <jsavitz@redhat.com> wrote: > >> > >> On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <jsavitz@redhat.com> wrote: > >>> > >>> On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote: > >>>> > >>>> Joel Savitz <jsavitz@redhat.com> writes: > >>>> > >>>>> On 64-bit powerpc, usage of a non-16MB-aligned value for the mem= kernel > >>>>> cmdline parameter results in a system hang at boot. > >>>>> > >>>>> For example, using 'mem=4198400K' will always reproduce this issue. > >>>>> > >>>>> This patch fixes the problem by aligning any argument to mem= to 16MB > >>>>> corresponding with the large page size on powerpc. > >>>>> > >>>>> Fixes: 2babf5c2ec2f ("[PATCH] powerpc: Unify mem= handling") > >>>>> Co-developed-by: Gonzalo Siero <gsierohu@redhat.com> > >>>>> Signed-off-by: Gonzalo Siero <gsierohu@redhat.com> > >>>>> Signed-off-by: Joel Savitz <jsavitz@redhat.com> > >>>>> --- > >>>>> arch/powerpc/kernel/prom.c | 6 +++++- > >>>>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > >>>>> index 0b5878c3125b..8cd3e2445d8a 100644 > >>>>> --- a/arch/powerpc/kernel/prom.c > >>>>> +++ b/arch/powerpc/kernel/prom.c > >>>>> @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) > >>>>> { > >>>>> if (!p) > >>>>> return 1; > >>>>> - > >>>>> +#ifdef CONFIG_PPC64 > >>>>> + /* Align to 16 MB == size of ppc64 large page */ > >>>>> + memory_limit = ALIGN(memparse(p, &p), 0x1000000); > >>>>> +#else > >>>>> memory_limit = PAGE_ALIGN(memparse(p, &p)); > >>>>> +#endif > >>>>> DBG("memory limit = 0x%llx\n", memory_limit); > >>>>> > >>>>> return 0; > >>>>> -- > >>>>> 2.43.0 > >>>> > >>>> Can you try this change? > >>>> > >>>> commit 5555bc55e1aa71f545cff31e1eccdb4a2e39df84 > >>>> Author: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> > >>>> Date: Fri Mar 8 14:45:26 2024 +0530 > >>>> > >>>> powerpc/mm: Align memory_limit value specified using mem= kernel parameter > >>>> > >>>> The value specified for the memory limit is used to set a restriction on > >>>> memory usage. It is important to ensure that this restriction is within > >>>> the linear map kernel address space range. The hash page table > >>>> translation uses a 16MB page size to map the kernel linear map address > >>>> space. htab_bolt_mapping() function aligns down the size of the range > >>>> while mapping kernel linear address space. Since the memblock limit is > >>>> enforced very early during boot, before we can detect the type of memory > >>>> translation (radix vs hash), we align the memory limit value specified > >>>> as a kernel parameter to 16MB. This alignment value will work for both > >>>> hash and radix translations. > >>>> > >>>> Signed-off-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org> > >>>> > >>>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > >>>> index 0b5878c3125b..9bd965d35352 100644 > >>>> --- a/arch/powerpc/kernel/prom.c > >>>> +++ b/arch/powerpc/kernel/prom.c > >>>> @@ -824,8 +824,11 @@ void __init early_init_devtree(void *params) > >>>> reserve_crashkernel(); > >>>> early_reserve_mem(); > >>>> > >>>> - /* Ensure that total memory size is page-aligned. */ > >>>> - limit = ALIGN(memory_limit ?: memblock_phys_mem_size(), PAGE_SIZE); > >>>> + if (memory_limit > memblock_phys_mem_size()) > >>>> + memory_limit = 0; > >>>> + > >>>> + /* Align down to 16 MB which is large page size with hash page translation */ > >>>> + limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(), SZ_16M); > >>>> memblock_enforce_memory_limit(limit); > >>>> > >>>> #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_4K_PAGES) > >>>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c > >>>> index e67effdba85c..d6410549e141 100644 > >>>> --- a/arch/powerpc/kernel/prom_init.c > >>>> +++ b/arch/powerpc/kernel/prom_init.c > >>>> @@ -817,8 +817,8 @@ static void __init early_cmdline_parse(void) > >>>> opt += 4; > >>>> prom_memory_limit = prom_memparse(opt, (const char **)&opt); > >>>> #ifdef CONFIG_PPC64 > >>>> - /* Align to 16 MB == size of ppc64 large page */ > >>>> - prom_memory_limit = ALIGN(prom_memory_limit, 0x1000000); > >>>> + /* Align down to 16 MB which is large page size with hash page translation */ > >>>> + prom_memory_limit = ALIGN_DOWN(prom_memory_limit, SZ_16M); > >>>> #endif > >>>> } > >>>> > >>>> > >>> > >>> Sorry for the delayed reply. I just tested this patch and it fixes the > >>> bug for me. > >> > >> Hi, > >> > >> Just a quick follow up on this. > >> > >> The above patch fixed the bug for me. > >> > >> How do we want to proceed? > >> > >> Best, > >> Joel Savitz > > > > Hi, > > > > I haven't heard anything on this thread so I'm just sending a quick follow up. > > > > Do we want to merge this > > > > Is it the same as > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20240403083611.172833-1-aneesh.kumar@kernel.org/ > ? Yes that appears to be the case.
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 0b5878c3125b..8cd3e2445d8a 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -82,8 +82,12 @@ static int __init early_parse_mem(char *p) { if (!p) return 1; - +#ifdef CONFIG_PPC64 + /* Align to 16 MB == size of ppc64 large page */ + memory_limit = ALIGN(memparse(p, &p), 0x1000000); +#else memory_limit = PAGE_ALIGN(memparse(p, &p)); +#endif DBG("memory limit = 0x%llx\n", memory_limit); return 0;