linux-kernel - Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL1p7m7f=b_qJrU8qea2n1+-1KzEVLMOpY9ov5fH2ZZxGrDK5A@mail.gmail.com>
Date: Wed, 10 Apr 2024 11:22:45 -0400
From: Joel Savitz <jsavitz@...hat.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...nel.org>
Cc: linux-kernel@...r.kernel.org, Michael Ellerman <mpe@...erman.id.au>, 
	Nicholas Piggin <npiggin@...il.com>, Christophe Leroy <christophe.leroy@...roup.eu>, 
	"Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>, Benjamin Gray <bgray@...ux.ibm.com>, 
	Paul Mackerras <paulus@...abs.org>, linuxppc-dev@...ts.ozlabs.org, 
	Gonzalo Siero <gsierohu@...hat.com>
Subject: Re: [PATCH] powerpc: align memory_limit to 16MB in early_parse_mem

On Mon, Apr 1, 2024 at 10:17 AM Joel Savitz <jsavitz@...hat.com> wrote:
>
> On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <jsavitz@...hat.com> wrote:
> >
> > On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <aneesh.kumar@...nel.org> wrote:
> > >
> > > Joel Savitz <jsavitz@...hat.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@...hat.com>
> > > > Signed-off-by: Gonzalo Siero <gsierohu@...hat.com>
> > > > Signed-off-by: Joel Savitz <jsavitz@...hat.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@...nel.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@...nel.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