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

From: Joel Savitz
Date: Wed Apr 10 2024 - 12:53:29 EST


On Wed, Apr 10, 2024 at 11:31 AM Christophe Leroy
<christophe.leroy@xxxxxxxxxx> wrote:
>
>
>
> Le 10/04/2024 à 17:22, Joel Savitz a écrit :
> > [Vous ne recevez pas souvent de courriers de jsavitz@xxxxxxxxxx. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Mon, Apr 1, 2024 at 10:17 AM Joel Savitz <jsavitz@xxxxxxxxxx> wrote:
> >>
> >> On Tue, Mar 26, 2024 at 12:45 AM Joel Savitz <jsavitz@xxxxxxxxxx> wrote:
> >>>
> >>> On Fri, Mar 8, 2024 at 5:18 AM Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxx> wrote:
> >>>>
> >>>> Joel Savitz <jsavitz@xxxxxxxxxx> 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@xxxxxxxxxx>
> >>>>> Signed-off-by: Gonzalo Siero <gsierohu@xxxxxxxxxx>
> >>>>> Signed-off-by: Joel Savitz <jsavitz@xxxxxxxxxx>
> >>>>> ---
> >>>>> 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@xxxxxxxxxx>
> >>>> 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@xxxxxxxxxx>
> >>>>
> >>>> 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@xxxxxxxxxx/
> ?

Yes that appears to be the case.