Re: [PATCH] arc: fix memory initialization for systems with two memory banks

From: Vineet Gupta
Date: Fri Aug 28 2020 - 18:17:39 EST


Hi Mike,

On 8/28/20 9:39 AM, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@xxxxxxxxxxxxx>
>
> Rework if memory map initialization broke initialization of ARC systems
> with two memory banks. Before these changes, memblock was not aware of
> nodes configuration and the memory map was always allocated from the
> "lowmem" bank. After the addition of node information to memblock, the core
> mm attempts to allocate the memory map for the "highmem" bank from its
> node. The access to this memory using __va() fails because it can be only
> accessed using kmap.
>
> Anther problem that was uncovered is that {min,max}_high_pfn are calculated
> from u64 high_mem_start variable which prevents truncation to 32-bit
> physical address and the PFN values are above the node and zone boundaries.

Not sure if I quite follow this part. We should not be relying on truncation: the
pfn should be derived off of zone addresses ?

> Use phys_addr_t type for high_mem_start and high_mem_size to ensure
> correspondence between PFNs and highmem zone boundaries and reserve the
> entire highmem bank until mem_init() to avoid accesses to it before highmem
> is enabled.
>
> Fixes: 51930df5801e ("mm: free_area_init: allow defining max_zone_pfn in descend ing order")
> Signed-off-by: Mike Rapoport <rppt@xxxxxxxxxxxxx>

Thx for the fix. I verified that a 2 mem bank system with HIGHMEM enabled now
works again.
And I've also added a couple of lines to changelog to describe how to test such a
config.

|    To test this:
|    1. Enable HIGHMEM in ARC config
|    2. Enable 2 memory banks in haps_hs.dts (uncomment the 2nd bank)


> ---
> arch/arc/mm/init.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> index f886ac69d8ad..3a35b82a718e 100644
> --- a/arch/arc/mm/init.c
> +++ b/arch/arc/mm/init.c
> @@ -26,8 +26,8 @@ static unsigned long low_mem_sz;
>
> #ifdef CONFIG_HIGHMEM
> static unsigned long min_high_pfn, max_high_pfn;
> -static u64 high_mem_start;
> -static u64 high_mem_sz;
> +static phys_addr_t high_mem_start;
> +static phys_addr_t high_mem_sz;
> #endif
>
> #ifdef CONFIG_DISCONTIGMEM
> @@ -69,6 +69,7 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
> high_mem_sz = size;
> in_use = 1;
> memblock_add_node(base, size, 1);
> + memblock_reserve(base, size);
> #endif
> }
>
> @@ -157,7 +158,7 @@ void __init setup_arch_memory(void)
> min_high_pfn = PFN_DOWN(high_mem_start);
> max_high_pfn = PFN_DOWN(high_mem_start + high_mem_sz);
>
> - max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;
> + max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;
>
> high_memory = (void *)(min_high_pfn << PAGE_SHIFT);
> kmap_init();
> @@ -166,22 +167,26 @@ void __init setup_arch_memory(void)
> free_area_init(max_zone_pfn);
> }
>
> -/*
> - * mem_init - initializes memory
> - *
> - * Frees up bootmem
> - * Calculates and displays memory available/used
> - */
> -void __init mem_init(void)
> +static void __init highmem_init(void)
> {
> #ifdef CONFIG_HIGHMEM
> unsigned long tmp;
>
> - reset_all_zones_managed_pages();
> + memblock_free(high_mem_start, high_mem_sz);
> for (tmp = min_high_pfn; tmp < max_high_pfn; tmp++)
> free_highmem_page(pfn_to_page(tmp));
> #endif
> +}
>
> +/*
> + * mem_init - initializes memory
> + *
> + * Frees up bootmem
> + * Calculates and displays memory available/used
> + */
> +void __init mem_init(void)
> +{
> memblock_free_all();
> + highmem_init();
> mem_init_print_info(NULL);
> }