At 02/15/2012 06:21 PM, Jan Kiszka Wrote: > On 2012-02-15 10:44, Wen Congyang wrote: >> At 02/15/2012 05:21 PM, Jan Kiszka Wrote: >>> On 2012-02-15 06:19, Wen Congyang wrote: >>>> At 02/15/2012 01:35 AM, Jan Kiszka Wrote: >>>>> On 2012-02-09 04:24, Wen Congyang wrote: >>>>>> Crash needs extra memory mapping to determine phys_base. >>>>>> >>>>>> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com> >>>>>> --- >>>>>> cpu-all.h | 2 ++ >>>>>> target-i386/arch-dump.c | 43 >>>>>> +++++++++++++++++++++++++++++++++++++++++++ >>>>>> 2 files changed, 45 insertions(+), 0 deletions(-) >>>>>> >>>>>> diff --git a/cpu-all.h b/cpu-all.h >>>>>> index efb5ba3..290c43a 100644 >>>>>> --- a/cpu-all.h >>>>>> +++ b/cpu-all.h >>>>>> @@ -530,10 +530,12 @@ int cpu_write_elf64_note(int fd, CPUState *env, >>>>>> int cpuid, >>>>>> target_phys_addr_t *offset); >>>>>> int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, >>>>>> target_phys_addr_t *offset); >>>>>> +int cpu_add_extra_memory_mapping(MemoryMappingList *list); >>>>>> #else >>>>>> #define cpu_get_memory_mapping(list, env) >>>>>> #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; }) >>>>>> #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; }) >>>>>> +#define cpu_add_extra_memory_mapping(list) ({ 0; }) >>>>>> #endif >>>>>> >>>>>> #endif /* CPU_ALL_H */ >>>>>> diff --git a/target-i386/arch-dump.c b/target-i386/arch-dump.c >>>>>> index 4c0ff77..d96f6ae 100644 >>>>>> --- a/target-i386/arch-dump.c >>>>>> +++ b/target-i386/arch-dump.c >>>>>> @@ -495,3 +495,46 @@ int cpu_write_elf32_note(int fd, CPUState *env, int >>>>>> cpuid, >>>>>> { >>>>>> return x86_write_elf32_note(fd, env, cpuid, offset); >>>>>> } >>>>>> + >>>>>> +/* This function is copied from crash */ >>>>> >>>>> And what does it do there and here? I suppose it is Linux-specific - any >>>>> version? This should be documented and encoded in the function name. >>>>> >>>>>> +static target_ulong get_phys_base_addr(CPUState *env, target_ulong >>>>>> *base_vaddr) >>>>>> +{ >>>>>> + int i; >>>>>> + target_ulong kernel_base = -1; >>>>>> + target_ulong last, mask; >>>>>> + >>>>>> + for (i = 30, last = -1; (kernel_base == -1) && (i >= 20); i--) { >>>>>> + mask = ~((1LL << i) - 1); >>>>>> + *base_vaddr = env->idt.base & mask; >>>>>> + if (*base_vaddr == last) { >>>>>> + continue; >>>>>> + } >>>>>> + >>>>>> + kernel_base = cpu_get_phys_page_debug(env, *base_vaddr); >>>>>> + last = *base_vaddr; >>>>>> + } >>>>>> + >>>>>> + return kernel_base; >>>>>> +} >>>>>> + >>>>>> +int cpu_add_extra_memory_mapping(MemoryMappingList *list) >>>>> >>>>> Again, what does "extra" mean? Probably guest-specific, no? >>>> >>>> crash will calculate the phys_base according to the virtual address and >>>> physical >>>> address stored in the PT_LOAD. >>> >>> Crash is a Linux-only tool, dump must not be restricted to that guest - >>> but could contain transparent extensions of the file format if needed. >>> >>>> >>>> If the vmcore is generated by 'virsh dump'(use migration to implement >>>> dumping), >>>> crash calculates the phys_base according to idt.base. The function >>>> get_phys_base_addr() >>>> uses the same way to calculates the phys_base. >>> >>> Hmm, where are those special registers (idt, gdt, tr etc.) stored in the >>> vmcore file, BTW? >> >> 'virsh dump' uses mirgation to implement dumping now. So the vmcore has all >> registers. > > This is about the new format. And there we are lacking those special
Yes, this file can be processed with crash. gdb cannot process such file. > registers. At some point, gdb will understand and need them to do proper > system-level debugging. I don't know the format structure here: can we > add sections to the core file in a way that consumers that don't know > them simply ignore them? I donot find such section now. If there is such section, I think it is better to store all cpu's register in the core file. I try to let the core file can be processed with crash and gdb. But crash still does not work well sometimes. I think we can add some option to let user choose whether to store memory mapping in the core file. Because crash does not need such mapping. If the p_vaddr in all PT_LOAD segments is 0, crash know the file is generated by qemu dump, and use another way to calculate phys_base. If you agree with it, please ignore this patch. Thanks Wen Congyang > > Jan >