[PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Joel Savitz]

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

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Michael Ellerman]

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

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Joel Savitz]

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
>

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Michael Ellerman]

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

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Aneesh Kumar K.V]

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

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Aneesh Kumar K.V]

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
 	}
 

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Joel Savitz]

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.

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Joel Savitz]

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

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[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

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Christophe Leroy]

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/ 
?

Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

[Joel Savitz]

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.