[PATCH] makedumpfile: ppc64: get vmalloc start address from vmcoreinfo

[HAGIO KAZUHITO(萩尾 一仁)]

Hi Aditya,

thanks for the patch.

On 2024/02/24 4:03, Aditya Gupta wrote:
> Below error was noticed when running makedumpfile on linux-next kernel
> crash (linux-next tag next-20240121):
> 
>      ...
>      Checking for memory holes : [100.0 %] | readpage_elf: Attempt to read non-existent page at 0xc000000000000.
>      [ 17.551718] kdump.sh[404]: readmem: type_addr: 0, addr:c00c000000000000, size:16384
>      [ 17.551793] kdump.sh[404]: __exclude_unnecessary_pages: Can't read the buffer of struct page.
>      [ 17.551864] kdump.sh[404]: create_2nd_bitmap: Can't exclude unnecessary pages.
>      [ 17.562632] kdump.sh[404]: The kernel version is not supported.
>      [ 17.562708] kdump.sh[404]: The makedumpfile operation may be incomplete.
>      [ 17.562773] kdump.sh[404]: makedumpfile Failed.
>      [ 17.564335] kdump[406]: saving vmcore failed, _exitcode:1
> 
> Above error was due to 'vmap_area_list' and 'vmlist' symbols missing
> from the vmcore.
> 
> 'vmap_area_list' was removed in the linux kernel with below commit:
> 
>      commit 378eb24a0658dd922b29524e0ce35c6c43f56cba
>           mm/vmalloc: remove vmap_area_list
> 
> Subsequently the commit also introduced 'VMALLOC_START' in vmcoreinfo to
> get base address of vmalloc area, instead of depending on 'vmap_area_list'
> 
> Hence if 'VMALLOC_START' symbol is there in vmcoreinfo:
>    1. Set vmalloc_start based on 'VMALLOC_START'
>    2. Don't error if vmap_area_list/vmlist are not defined
> 
> Reported-by: Sachin Sant <sachinp at linux.ibm.com>
> Signed-off-by: Aditya Gupta <adityag at linux.ibm.com>
> ---
>   arch/ppc64.c   | 19 +++++++++++++------
>   makedumpfile.c |  3 ++-
>   makedumpfile.h |  6 +++---
>   3 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/ppc64.c b/arch/ppc64.c
> index 96c357cb0335..bb62e2cd199a 100644
> --- a/arch/ppc64.c
> +++ b/arch/ppc64.c
> @@ -568,7 +568,9 @@ get_machdep_info_ppc64(void)
>   	/*
>   	 * Get vmalloc_start value from either vmap_area_list or vmlist.
>   	 */
> -	if ((SYMBOL(vmap_area_list) != NOT_FOUND_SYMBOL)
> +	if (NUMBER(vmalloc_start) != NOT_FOUND_SYMBOL) {

I will fix this NOT_FOUND_SYMBOL to NOT_FOUND_NUMBER when applying, 
otherwise makedumpfile will fail for a dumpfile without the 
corresponding kernel patch, correct?

The patch looks good to me except for it.  I will apply this with the 
kernel version in the commit log after the kernel patch gets merged.

Thanks,
Kazu


> +		vmalloc_start = NUMBER(vmalloc_start);
> +	} else if ((SYMBOL(vmap_area_list) != NOT_FOUND_SYMBOL)
>   	    && (OFFSET(vmap_area.va_start) != NOT_FOUND_STRUCTURE)
>   	    && (OFFSET(vmap_area.list) != NOT_FOUND_STRUCTURE)) {
>   		if (!readmem(VADDR, SYMBOL(vmap_area_list) + OFFSET(list_head.next),
> @@ -684,11 +686,16 @@ vaddr_to_paddr_ppc64(unsigned long vaddr)
>   	if ((SYMBOL(vmap_area_list) == NOT_FOUND_SYMBOL)
>   	    || (OFFSET(vmap_area.va_start) == NOT_FOUND_STRUCTURE)
>   	    || (OFFSET(vmap_area.list) == NOT_FOUND_STRUCTURE)) {
> -		if ((SYMBOL(vmlist) == NOT_FOUND_SYMBOL)
> -		    || (OFFSET(vm_struct.addr) == NOT_FOUND_STRUCTURE)) {
> -			ERRMSG("Can't get info for vmalloc translation.\n");
> -			return NOT_PADDR;
> -		}
> +		/*
> +		 * Don't depend on vmap_area_list/vmlist if vmalloc_start is set in
> +		 * vmcoreinfo, in that case proceed without error
> +		 */
> +		if (NUMBER(vmalloc_start) == NOT_FOUND_NUMBER)
> +			if ((SYMBOL(vmlist) == NOT_FOUND_SYMBOL)
> +				|| (OFFSET(vm_struct.addr) == NOT_FOUND_STRUCTURE)) {
> +				ERRMSG("Can't get info for vmalloc translation.\n");
> +				return NOT_PADDR;
> +			}
>   	}
>   
>   	return ppc64_vtop_level4(vaddr);
> diff --git a/makedumpfile.c b/makedumpfile.c
> index b004b93fecb7..b6c63fad15f3 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -2978,6 +2978,8 @@ read_vmcoreinfo(void)
>   	READ_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE", PAGE_OFFLINE_MAPCOUNT_VALUE);
>   	READ_NUMBER("phys_base", phys_base);
>   	READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
> +
> +	READ_NUMBER_UNSIGNED("VMALLOC_START", vmalloc_start);
>   #ifdef __aarch64__
>   	READ_NUMBER("VA_BITS", VA_BITS);
>   	READ_NUMBER("TCR_EL1_T1SZ", TCR_EL1_T1SZ);
> @@ -2989,7 +2991,6 @@ read_vmcoreinfo(void)
>   	READ_NUMBER("VA_BITS", va_bits);
>   	READ_NUMBER_UNSIGNED("phys_ram_base", phys_ram_base);
>   	READ_NUMBER_UNSIGNED("PAGE_OFFSET", page_offset);
> -	READ_NUMBER_UNSIGNED("VMALLOC_START", vmalloc_start);
>   	READ_NUMBER_UNSIGNED("VMALLOC_END", vmalloc_end);
>   	READ_NUMBER_UNSIGNED("VMEMMAP_START", vmemmap_start);
>   	READ_NUMBER_UNSIGNED("VMEMMAP_END", vmemmap_end);
> diff --git a/makedumpfile.h b/makedumpfile.h
> index 59c83e1d9df3..4021c5af2a34 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -541,8 +541,6 @@ do { \
>    * The value of dependence on machine
>    */
>   #define PAGE_OFFSET		(info->page_offset)
> -#define VMALLOC_START		(info->vmalloc_start)
> -#define VMALLOC_END		(info->vmalloc_end)
>   #define VMEMMAP_START		(info->vmemmap_start)
>   #define VMEMMAP_END		(info->vmemmap_end)
>   #define PMASK			(0x7ffffffffffff000UL)
> @@ -2262,6 +2260,9 @@ struct number_table {
>   	long    HUGETLB_PAGE_DTOR;
>   	long	phys_base;
>   	long	KERNEL_IMAGE_SIZE;
> +
> +	unsigned long vmalloc_start;
> +
>   #ifdef __aarch64__
>   	long 	VA_BITS;
>   	long	TCR_EL1_T1SZ;
> @@ -2272,7 +2273,6 @@ struct number_table {
>   	long va_bits;
>   	unsigned long phys_ram_base;
>   	unsigned long page_offset;
> -	unsigned long vmalloc_start;
>   	unsigned long vmalloc_end;
>   	unsigned long vmemmap_start;
>   	unsigned long vmemmap_end;