Re: [PATCH 01/10] util_lib: add paddr_to_offset function and expose raw vmcoreinfo data

[Simon Horman <horms@kernel.org>]

On Mon, Jun 22, 2026 at 11:54:39PM +0300, Pnina Feder wrote:
> Add paddr_to_offset() to translate physical addresses to file offsets,
> complementing the existing vaddr_to_offset() for virtual addresses.
> 
> Store raw vmcoreinfo note data when parsing ELF notes and provide
> elf_get_raw_vmcoreinfo() accessor to retrieve it. This allows callers
> to access the unparsed vmcoreinfo content for external processing.
> and fix compilation warnings.
> 
> Change-Id: Icc75c22d6b2d95b6c951ccf647819d7e9cc4e7de
> Signed-off-by: Pnina Feder <pnina.feder@mobileye.com>

Overall this looks good to me.
I have provided some minor feedback inline.

> ---
>  util_lib/elf_info.c         | 53 ++++++++++++++++++++++++++++++++-----
>  util_lib/include/elf_info.h | 11 ++++----
>  2 files changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/util_lib/elf_info.c b/util_lib/elf_info.c

...

> @@ -776,7 +805,7 @@ static void dump_dmesg_legacy(int fd, void (*handler)(char*, unsigned int))
>  	 * To collect full dmesg including the part before `dmesg -c` is useful
>  	 * for later debugging. Use same logic as what crash utility is using.
>  	 */
> -	logged_chars = log_end < log_buf_len ? log_end : log_buf_len;
> +	logged_chars = log_end < (unsigned int)log_buf_len ? log_end : (unsigned int)log_buf_len;

Adding this cast seems to be a clean-up which is not strictly related to
this patch. I'm not a huge fan of adding casts, but that aside, it
seems like a cleanup that would be best put in a separate patch.

Likewise for similar updates in this patch.

>  
>  	if (handler)
>  		handler(buf + (log_buf_len - logged_chars), logged_chars);

...

> diff --git a/util_lib/include/elf_info.h b/util_lib/include/elf_info.h
> index fdf4c3d0..c2873365 100644
> --- a/util_lib/include/elf_info.h
> +++ b/util_lib/include/elf_info.h
> @@ -24,13 +24,14 @@
>  #include <ctype.h>
>  
>  int get_pt_load(int idx,
> -	unsigned long long *phys_start,
> -	unsigned long long *phys_end,
> -	unsigned long long *virt_start,
> -	unsigned long long *virt_end);
> +		unsigned long long *phys_start,
> +		unsigned long long *phys_end,
> +		unsigned long long *virt_start,
> +		unsigned long long *virt_end);

Similarly, this whitespace change also seems to be a clean-up and if so
doesn't belong in this patch.

>  int read_phys_offset_elf_kcore(int fd, long *phys_off);
>  int read_elf(int fd);
>  void dump_dmesg(int fd, void (*handler)(char*, unsigned int));
>  extern void (*arch_scan_vmcoreinfo)(char *pos);
> -
> +uint64_t paddr_to_offset(uint64_t paddr);
> +int elf_get_raw_vmcoreinfo(const char **data, size_t *size);
>  #endif /* ELF_INFO_H */
> -- 
> 2.43.0
>