Re: [PATCH v2] kdump, vmcoreinfo: report actual value of phys_base

[Baoquan He <bhe@redhat.com>]

On 11/13/14 at 11:30am, HATAYAMA Daisuke wrote:
> Currently, VMCOREINFO note information reports the virtual address of
> phys_base that is assigned to symbol phys_base. But this doesn't make
> sense because to refer to phys_base, it's necessary to get the value
> of phys_base itself we are now about to refer to.
> 
> Userland tools related to kdump such as makedumpfile and crash utility
> so far have made some efforts to calculate phys_base on crash dump
> formats generated by mechanisms running outside Linux kernel, such as
> virtual machine hypervisor such as qemu dump, which ordinary users use
> via virsh dump, or ones implemented on vendor specific firmware.
> 
> That is, find a kernel data whose virtual and physical addresses are
> available via its note information and calculate phys_base from
> it. However, such data structure is not the one prepared for phys_base
> purpose. There's no guarantee that other crash dump mechanisms include
> such information that can be used to calculate phys_base similarly.
> 
> To get VMCOREINFO in vmcore, it's easy to use strings and grep
> commands like this; VMCOREINFO consists of simple string:
> 
> $ strings vmcore-3.10.0-121.el7.x86_64 | grep -E ".*VMCOREINFO.*" -A 100
> VMCOREINFO
> OSRELEASE=3.10.0-121.el7.x86_64
> PAGESIZE=4096
> ...
> 
> This is also useful to get value of phys_base in kdump 2nd kernel
> contained in vmcore using the above-mentioned external crash dump
> mechanism; kdump 2nd kernel is an inherently relocated kernel.
> 
> This commit doesn't remove VMCOREINFO_SYMBOL(phys_base) line because
> makedumpfile refers to it and if removing it, old versions
> makedumpfile doesn't work well.
> 
> ChangeLog:
> v2:
> - Introduce VMCOREINFO_PHYS_BASE().
> - Correct patch description: this work is primarily for mechanisms
>   running outside (kdump 1st) Linux kernel.
> 
> Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> ---
>  arch/x86/kernel/machine_kexec_64.c | 1 +
>  include/linux/kexec.h              | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 4859810..47cc835 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -334,6 +334,7 @@ void arch_crash_save_vmcoreinfo(void)
>  #endif
>  	vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
>  			      (unsigned long)&_text - __START_KERNEL);
> +	VMCOREINFO_PHYS_BASE(phys_base);
>  }
>  
>  /* arch-dependent functionality related to kexec file-based syscall */
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 9d957b7..bee3c5b 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -258,6 +258,8 @@ unsigned long paddr_vmcoreinfo_note(void);
>  	vmcoreinfo_append_str("NUMBER(%s)=%ld\n", #name, (long)name)
>  #define VMCOREINFO_CONFIG(name) \
>  	vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
> +#define VMCOREINFO_PHYS_BASE(value) \
> +	vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)

I don't like the idea that add a new MACRO for a specific case. I prefer
it to be a generic MACRO which can be used by later adding if they want
to add a unsigned long value. 

>  
>  extern struct kimage *kexec_image;
>  extern struct kimage *kexec_crash_image;
> -- 
> 1.9.3
> 
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec