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 <[email protected]>
Signed-off-by: Gonzalo Siero <[email protected]>
Signed-off-by: Joel Savitz <[email protected]>
---
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
Hi Joel,
Joel Savitz <[email protected]> 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 <[email protected]>
> Signed-off-by: Gonzalo Siero <[email protected]>
> Signed-off-by: Joel Savitz <[email protected]>
> ---
> 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 <[email protected]> wrote:
>
> Hi Joel,
>
> Joel Savitz <[email protected]> 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
([email protected]) (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 <[email protected]>
> > Signed-off-by: Gonzalo Siero <[email protected]>
> > Signed-off-by: Joel Savitz <[email protected]>
> > ---
> > 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 <[email protected]> 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 <[email protected]> writes:
> On Fri, Mar 1, 2024 at 6:23 PM Michael Ellerman <[email protected]> wrote:
>> Joel Savitz <[email protected]> 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
> ([email protected]) (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 <[email protected]> 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 <[email protected]>
> Signed-off-by: Gonzalo Siero <[email protected]>
> Signed-off-by: Joel Savitz <[email protected]>
> ---
> 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) <[email protected]>
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) <[email protected]>
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 <[email protected]> wrote:
>
> Joel Savitz <[email protected]> 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 <[email protected]>
> > Signed-off-by: Gonzalo Siero <[email protected]>
> > Signed-off-by: Joel Savitz <[email protected]>
> > ---
> > 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) <[email protected]>
> 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) <[email protected]>
>
> 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 <[email protected]> wrote:
>
> On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <[email protected]> wrote:
> >
> > Joel Savitz <[email protected]> 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 <[email protected]>
> > > Signed-off-by: Gonzalo Siero <[email protected]>
> > > Signed-off-by: Joel Savitz <[email protected]>
> > > ---
> > > 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) <[email protected]>
> > 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) <[email protected]>
> >
> > 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
Le 10/04/2024 à 17:22, Joel Savitz a écrit :
> [Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>
> On Mon, Apr 1, 2024 at 10:17 AM Joel Savitz <[email protected]> wrote:
>>
>> On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <[email protected]> wrote:
>>>
>>> On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <[email protected]> wrote:
>>>>
>>>> Joel Savitz <[email protected]> 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 <[email protected]>
>>>>> Signed-off-by: Gonzalo Siero <[email protected]>
>>>>> Signed-off-by: Joel Savitz <[email protected]>
>>>>> ---
>>>>> 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) <[email protected]>
>>>> 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) <[email protected]>
>>>>
>>>> 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/[email protected]/
?
On Mon, Apr 1, 2024 at 10:17 AM Joel Savitz <[email protected]> wrote:
>
> On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <[email protected]> wrote:
> >
> > On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <[email protected]> wrote:
> > >
> > > Joel Savitz <[email protected]> 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 <[email protected]>
> > > > Signed-off-by: Gonzalo Siero <[email protected]>
> > > > Signed-off-by: Joel Savitz <[email protected]>
> > > > ---
> > > > 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) <[email protected]>
> > > 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) <[email protected]>
> > >
> > > 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
On Wed, Apr 10, 2024 at 11:31 AM Christophe Leroy
<[email protected]> wrote:
>
>
>
> Le 10/04/2024 à 17:22, Joel Savitz a écrit :
> > [Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Mon, Apr 1, 2024 at 10:17 AM Joel Savitz <[email protected]> wrote:
> >>
> >> On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <[email protected]> wrote:
> >>>
> >>> On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <[email protected]> wrote:
> >>>>
> >>>> Joel Savitz <[email protected]> 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 <[email protected]>
> >>>>> Signed-off-by: Gonzalo Siero <[email protected]>
> >>>>> Signed-off-by: Joel Savitz <[email protected]>
> >>>>> ---
> >>>>> 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) <[email protected]>
> >>>> 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) <[email protected]>
> >>>>
> >>>> 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/[email protected]/
> ?
Yes that appears to be the case.