Re: [PATCH V2] Octeon: fix broken plat_mem_setup()

[David Daney <ddaney@caviumnetworks.com>]

On 04/12/2013 09:52 AM, Alexander Sverdlin wrote:
> Octeon: fix broken plat_mem_setup()
>
> Upstream patch abe77f90dc9c65a7c9a4d61c2cbb8db4d5566e4f (MIPS: Octeon:
> Add kexec
> and kdump support) seems to be untested and broken Linux 3.8 on Octeon
> -- in
> comparison with 3.7 Linux crashes with
>
[...]
> [    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
>
> There are at least couple of issues in the patch:
>
> 1. reason for add_memory_region(memory, mem_alloc_size, BOOT_MEM_RAM)
> instead of
> add_memory_region(memory, size, BOOT_MEM_RAM) is unclear, especially
> because it
> will discard corrections performed by two preceding calls to
> memory_exclude_page().
> This is fixed using size again instead of mem_alloc_size, moreover,
> block sizes
> calculation could be simplified.
>
> 2. add_memory_region(kernel_start, kernel_size, BOOT_MEM_RAM) marks
> kernel body
> as RAM available for allocation, that's why the kernel later crashes
> overwritting
> itself. Marking it as BOOT_MEM_INIT_RAM solves the crash and still
> allows to load
> the same kernel with kexec (tested also).
>

I have a different patch I am testing that gets rid of all this crap.

The strategy we are using is to use cvmx_bootmem named blocks for all 
memory in a KEXEC environment.

David Daney


> Signed-off-by: Alexander Sverdlin <alexander.sverdlin.ext@nsn.com>
> Cc: David Daney <david.daney@cavium.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
>
> Changes in V2:
> * corrected "end" calculation, simplified "size" calculation
> * Mapped the kernel correctly as INIT RAM, instead of throwing this code
> out
>
> As a result, kernel not only boots succefully, but also able to load itself
> with "kexec -l <image>".
>
> --- linux.orig/arch/mips/cavium-octeon/setup.c
> +++ linux/arch/mips/cavium-octeon/setup.c
> @@ -936,14 +936,14 @@ void __init plat_mem_setup(void)
>                           CVMX_PCIE_BAR1_PHYS_SIZE,
>                           &memory, &size);
>   #ifdef CONFIG_KEXEC
> -            end = memory + mem_alloc_size;
> +            end = memory + size;
>
>               /*
>                * This function automatically merges address regions
>                * next to each other if they are received in
>                * incrementing order
>                */
> -            if (memory < crashk_base && end >  crashk_end) {
> +            if (memory < crashk_base && end > crashk_end) {
>                   /* region is fully in */
>                   add_memory_region(memory,
>                             crashk_base - memory,
> @@ -969,20 +969,20 @@ void __init plat_mem_setup(void)
>                    * Overlap with the beginning of the region,
>                    * reserve the beginning.
>                     */
> -                mem_alloc_size -= crashk_end - memory;
> +                size = end - crashk_end;
>                   memory = crashk_end;
>               } else if (memory < crashk_base && end > crashk_base &&
> -                   end < crashk_end)
> +                   end < crashk_end) {
>                   /*
>                    * Overlap with the beginning of the region,
>                    * chop of end.
>                    */
> -                mem_alloc_size -= end - crashk_base;
> +                size = crashk_base - memory;
> +            }
>   #endif
> -            add_memory_region(memory, mem_alloc_size, BOOT_MEM_RAM);
> +            if (size)
> +                add_memory_region(memory, size, BOOT_MEM_RAM);
>               total += mem_alloc_size;
> -            /* Recovering mem_alloc_size */
> -            mem_alloc_size = 4 << 20;
>           } else {
>               break;
>           }
> @@ -994,7 +994,7 @@ void __init plat_mem_setup(void)
>
>       /* Adjust for physical offset. */
>       kernel_start &= ~0xffffffff80000000ULL;
> -    add_memory_region(kernel_start, kernel_size, BOOT_MEM_RAM);
> +    add_memory_region(kernel_start, kernel_size, BOOT_MEM_INIT_RAM);
>   #endif /* CONFIG_CRASH_DUMP */
>
>   #ifdef CONFIG_CAVIUM_RESERVE32
>
>