Re: [Crash-utility] some updates of xencrash

Hi Dave, I thought it is better to be small value from the view point to save 'xen_phys_start' value in the crashinfo. From the view point of users you are right. It should be '--xen_phys_start=<address>'.

> Another theoretical question -- since the /proc/iomem exports the > xen_phys_start variable to user space, is there any way it could > be handled entirely in user space by kexec-tools? Theoretically yes. But it is necessary to add another ElfNote section. I thought it is the smallest modication necessary to add 'xen_phys_start' in the crashinfo. Do you think which is better ?

Thanks. Itsuro Oda On Fri, 18 Apr 2008 10:14:04 -0400 Dave Anderson <anderson(a)redhat.com&gt; wrote: > Itsuro ODA wrote: >> Hi, >> >> The attached is some fixes which are necessary for xencrash to use >> for the newer version of xen (after 3.1.0, ex. 3.1.2, 3.2.0). >> >> * x86 >> - add some symbols which are used to know the boundary >> condition at the tracing in 'bt'. >> - fix for the symbol 'idle_pg_table_l3' going away. >> >> * x86_64 >> - fix for the virtual address area of xen code/data is added. >> - fix for the symbol 'idle_pg_table_l4' going away. >> - add the option '--xen_phys_start_mfn=' >> newer version of xen (ex. RHEL5.2, 3.2.0) moves the >> physical(machine) address of xen code/data area after >> the system started up. >> The relocated physical(machine) address of xen code/data >> area is necessary to use xencrash. >> The value can be got by 'cat /proc/iomem' under dom0. >> ------------------------------------------------------- >> # cat /proc/iomem >> ... >> 7e600000-7f5fffff : Hypervisor code and data *** this line >> ... >> ------------------------------------------------------- >> ex. to use xencrash: >> # crash --xen_phys_start_mfn=0x7e600 xen-syms vmcore >> - extract 'xen_phys_start_mfn' from the XEN_ELFNOTE_CRASH_INFO >> ElfNote. >> This is experimental code because it is necessary to modify >> the xen to add the xen_phys_start_mfn value in the ElfNote >> at crash time. (the patch is also attached in this mail.) >> A sample dump is availavle in the following URL: >> http://people.valinux.co.jp/~oda/x86_64_dump_080417.tar.gz >> >> This patch is for crash-4.0-6.2. >> >> Thanks. >> Itsuro Oda > Hi Itsuro, > > Thanks very much for the quick response. > > I have one relatively minor issue. I understand that the previously existing > "--p2m_mfn" option was expressed as an mfn, and that made sense because its value > exists in the xen hypervisor code as an mfn value (actually a xen_pfn_t). > > But it seems unnecessary to bother going through the translation of the > hypservisors's "xen_phys_start" physical address into an mfn? And then > only to have the crash utility have to turn it back into a physical address > when it uses it? Why not just leave it as a physical address in both the > kernel patch, and when passing it to the crash utility? > > From a user's standpoint, when it's necessary to pre-determine the argument from > a hypervisor/dom0 vmcore that doesn't contain it, the xen_phys_start can be determined > by either reading /proc/iomem on the live system, where as you've shown in > the example above, it's 7e600000: > > > # cat /proc/iomem > > ... > > 7e600000-7f5fffff : Hypervisor code and data *** this line > > ... > > Alternatively, the same dom0/HV kdump vmcore can be run in a crash session using > the dom0 vmlinux, where in this example vmcore, it's 0x3f000000: > > crash> px xen_hypervisor_res > xen_hypervisor_res = $3 = { > start = 0x3f000000, > end = 0x3fffffff, > name = 0xffffffff8049ab72 "Hypervisor code and data", > flags = 0x80000200, > parent = 0xffff880000001180, > sibling = 0x0, > child = 0xffff8800000000a8 > } > > For convenience sake, I'd prefer to just leave it as that, without > needlessly forcing the user to translate the above into an mfn, so > that they can simply enter: "crash --xen_phys_start=0x3f000000 ..." > > The same thing applies to the kernel patch -- just leave it in its > natural state as a physical (machine) address. > > Unfortunately it's already going to be an annoyance to have to pre-determine > the xen_phys_start value -- so why bother making it even more cryptic by > forcing the user to turn it into an mfn? > > Another theoretical question -- since the /proc/iomem exports the > xen_phys_start variable to user space, is there any way it could > be handled entirely in user space by kexec-tools? > > Dave > > >> === for x86 === >> --- lkcd_x86_trace.c.org 2008-04-17 10:13:05.000000000 +0900 >> +++ lkcd_x86_trace.c 2008-04-17 10:13:17.000000000 +0900 >> @@ -1423,6 +1423,7 @@ >> if (XEN_HYPER_MODE()) { >> func_name = kl_funcname(pc); >> if (STREQ(func_name, "idle_loop") || STREQ(func_name, "hypercall") >> + || STREQ(func_name, "tracing_off") >> || STREQ(func_name, "handle_exception")) { >> UPDATE_FRAME(func_name, pc, 0, sp, bp, asp, 0, 0, bp - sp, 0); >> return(trace->nframes); >> @@ -1682,6 +1683,7 @@ >> if (func_name && XEN_HYPER_MODE()) { >> if (STREQ(func_name, "continue_nmi") || >> STREQ(func_name, "vmx_asm_vmexit_handler") || >> + STREQ(func_name, "handle_nmi_mce") || >> STREQ(func_name, "deferred_nmi")) { >> /* Interrupt frame */ >> sp = curframe->fp + 4; >> --- x86.c.org 2008-04-17 10:29:21.000000000 +0900 >> +++ x86.c 2008-04-17 10:31:51.000000000 +0900 >> @@ -2846,7 +2846,10 @@ >> *paddr = kvaddr - DIRECTMAP_VIRT_START; >> return TRUE; >> } >> - pgd = (ulonglong *)symbol_value("idle_pg_table_l3"); >> + if (symbol_exists("idle_pg_table_l3")) >> + pgd = (ulonglong *)symbol_value("idle_pg_table_l3"); >> + else >> + pgd = (ulonglong *)symbol_value("idle_pg_table"); >> } else { >> if (!vt->vmalloc_start) { >> *paddr = VTOP(kvaddr); >> @@ -4965,7 +4968,8 @@ >> break; >> >> case PRE_GDB: >> - if (symbol_exists("idle_pg_table_l3")) { >> + if (symbol_exists("create_pae_xen_mappings") || >> + symbol_exists("idle_pg_table_l3")) { >> machdep->flags |= PAE; >> PGDIR_SHIFT = PGDIR_SHIFT_3LEVEL; >> PTRS_PER_PTE = PTRS_PER_PTE_3LEVEL; >> >> === for x86_64 === >> --- defs.h.org 2008-04-17 15:03:14.000000000 +0900 >> +++ defs.h 2008-04-17 15:41:10.000000000 +0900 >> @@ -2162,10 +2162,13 @@ >> >> #define FILL_PML4_HYPER() { \ >> if (!machdep->machspec->last_pml4_read) { \ >> - readmem(symbol_value("idle_pg_table_4"), KVADDR, \ >> - machdep->machspec->pml4, PAGESIZE(), "idle_pg_table_4", \ >> + unsigned long idle_pg_table = \ >> + symbol_exists("idle_pg_table_4") ? symbol_value("idle_pg_table_4") : \ >> + symbol_value("idle_pg_table"); \ >> + readmem(idle_pg_table, KVADDR, \ >> + machdep->machspec->pml4, PAGESIZE(), "idle_pg_table", \ >> FAULT_ON_ERROR); \ >> - machdep->machspec->last_pml4_read = symbol_value("idle_pg_table_4"); \ >> + machdep->machspec->last_pml4_read = idle_pg_table; \ >> }\ >> } >> >> @@ -4081,6 +4084,8 @@ >> void get_kdump_regs(struct bt_info *, ulong *, ulong *); >> void xen_kdump_p2m_mfn(char *); >> int is_sadump_xen(void); >> +void set_xen_phys_start_mfn(char *); >> +ulong xen_phys_start_mfn(void); >> >> /* >> * diskdump.c >> --- main.c.org 2008-04-17 15:26:37.000000000 +0900 >> +++ main.c 2008-04-17 15:45:18.000000000 +0900 >> @@ -48,6 +48,7 @@ >> {"no_ikconfig", 0, 0, 0}, >> {"hyper", 0, 0, 0}, >> {"p2m_mfn", required_argument, 0, 0}, >> + {"xen_phys_start_mfn", required_argument, 0, 0}, >> {"zero_excluded", 0, 0, 0}, >> {"no_panic", 0, 0, 0}, >> {"more", 0, 0, 0}, >> @@ -155,6 +156,9 @@ >> else if (STREQ(long_options[option_index].name, "p2m_mfn")) >> xen_kdump_p2m_mfn(optarg); >> >> + else if (STREQ(long_options[option_index].name, "xen_phys_start_mfn")) >> + set_xen_phys_start_mfn(optarg); >> + >> else if (STREQ(long_options[option_index].name, "zero_excluded")) >> *diskdump_flags |= ZERO_EXCLUDED; >> >> --- netdump.c.org 2008-04-17 15:09:06.000000000 +0900 >> +++ netdump.c 2008-04-17 15:48:44.000000000 +0900 >> @@ -1521,6 +1521,8 @@ >> */ >> if (!nd->xen_kdump_data->p2m_mfn) >> nd->xen_kdump_data->p2m_mfn = *(uptr+(words-1)); >> + if (words > 9 && !nd->xen_kdump_data->xen_phys_start_mfn) >> + nd->xen_kdump_data->xen_phys_start_mfn = *(uptr+(words-2)); >> } >> } >> break; >> @@ -1724,6 +1726,8 @@ >> */ >> if (!nd->xen_kdump_data->p2m_mfn) >> nd->xen_kdump_data->p2m_mfn = *(up+(words-1)); >> + if (words > 9 && !nd->xen_kdump_data->xen_phys_start_mfn) >> + nd->xen_kdump_data->xen_phys_start_mfn = *(up+(words-2)); >> } >> } >> break; >> @@ -2312,3 +2316,22 @@ >> >> return FALSE; >> } >> + >> +void >> +set_xen_phys_start_mfn(char *arg) >> +{ >> + ulong value; >> + int errflag = 0; >> + >> + value = htol(arg, RETURN_ON_ERROR|QUIET, &errflag); >> + if (!errflag) >> + xen_kdump_data.xen_phys_start_mfn = value; >> + else >> + error(WARNING, "invalid xen_phys_start_mfn argument: %s\n", arg); >> +} >> + >> +ulong >> +xen_phys_start_mfn(void) >> +{ >> + return nd->xen_kdump_data->xen_phys_start_mfn; >> +} >> --- netdump.h.org 2008-04-17 15:38:59.000000000 +0900 >> +++ netdump.h 2008-04-17 15:39:38.000000000 +0900 >> @@ -109,6 +109,7 @@ >> ulong accesses; >> int p2m_frames; >> ulong *p2m_mfn_frame_list; >> + ulong xen_phys_start_mfn; >> }; >> >> #define KDUMP_P2M_INIT (0x1) >> --- x86_64.c.org 2008-04-17 15:34:32.000000000 +0900 >> +++ x86_64.c 2008-04-17 15:36:53.000000000 +0900 >> @@ -1401,6 +1401,10 @@ >> return FALSE; >> >> if (XEN_HYPER_MODE()) { >> + if (XENTEXT_VIRT_ADDR(kvaddr)) { >> + *paddr = kvaddr - __XEN_VIRT_START + (xen_phys_start_mfn() << PAGESHIFT()); >> + return TRUE; >> + } >> if (DIRECTMAP_VIRT_ADDR(kvaddr)) { >> *paddr = kvaddr - DIRECTMAP_VIRT_START; >> return TRUE; >> --- xen_hyper_defs.h.org 2008-04-17 15:31:23.000000000 +0900 >> +++ xen_hyper_defs.h 2008-04-17 15:34:15.000000000 +0900 >> @@ -64,6 +64,9 @@ >> #define DIRECTMAP_VIRT_START (0xffff830000000000) >> #define DIRECTMAP_VIRT_END (0xffff840000000000) >> #define PAGE_OFFSET_XEN_HYPER DIRECTMAP_VIRT_START >> +#define __XEN_VIRT_START (0xFFFF828C80000000) >> +#define XENTEXT_VIRT_ADDR(vaddr) \ >> + (((vaddr) >= __XEN_VIRT_START) && ((vaddr) < DIRECTMAP_VIRT_START)) >> #endif >> >> #ifdef IA64 >> >> === Xen (explanation purpose) === >> --- include/xen/elfcore.h.org 2008-04-17 14:11:41.000000000 +0900 >> +++ include/xen/elfcore.h 2008-04-17 14:11:57.000000000 +0900 >> @@ -66,6 +66,7 @@ >> unsigned long xen_compile_time; >> unsigned long tainted; >> #ifdef CONFIG_X86 >> + unsigned long xen_phys_start_mfn; >> unsigned long dom0_pfn_to_mfn_frame_list_list; >> #endif >> } crash_xen_info_t; >> --- arch/x86/crash.c.org 2008-04-17 14:12:51.000000000 +0900 >> +++ arch/x86/crash.c 2008-04-17 14:13:13.000000000 +0900 >> @@ -102,6 +102,7 @@ >> hvm_disable(); >> >> info = kexec_crash_save_info(); >> + info->xen_phys_start_mfn = xen_phys_start >> PAGE_SHIFT; >> info->dom0_pfn_to_mfn_frame_list_list = >> arch_get_pfn_to_mfn_frame_list_list(dom0); >> }