lkml.org 
[lkml]   [2020]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] arc: fix memory initialization for systems with two memory banks
    On Fri, Aug 28, 2020 at 10:17:28PM +0000, Vineet Gupta wrote:
    > Hi Mike,
    >
    > On 8/28/20 9:39 AM, Mike Rapoport wrote:
    > > From: Mike Rapoport <rppt@linux.ibm.com>
    > >
    > > 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 ?

    Before the refactoring of the memmap initialization, we used

    free_area_init_node(1, zones_size, min_high_pfn, zones_holes);

    With min_high_pfn being u64, min_node_pfn would be 0x80000 (presuming 8k
    page and haps_hs.dts). But since we explicitly passed the min_node_pfn,
    it will be eventually used at zone->zone_start_pfn, so HIGHMEM zone
    would span from 0x80000.

    After the refactoring, we use memblock information and architectural
    limits for zone extents to detect actual zone span. Memblock uses
    phys_addr_t to represent memory banks and if we still calculate
    min_high_pfn using u64 there is a mismatch between pfn (0x80000) and
    physicall address (0x0).

    > > 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@linux.ibm.com>
    >
    > 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)

    The second bank already enabled in the dts ;-)

    I think its worthwhile adding this to the wiki [1] since it's not likely
    people could dig this from the kernel log.

    [1] https://github.com/foss-for-synopsys-dwc-arc-processors/linux/wiki/How-to-run-ARC-Linux-kernel-and-debug-(with-MetaWare-Debugger)

    > > ---
    > > 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);
    > > }
    >

    --
    Sincerely yours,
    Mike.

    \
     
     \ /
      Last update: 2020-08-29 14:50    [W:3.523 / U:0.996 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site