Re: some updates of xencrash

Hi Dave,

On Mon, 21 Apr 2008 09:09:59 -0400
Dave Anderson <andersonredhat.com> wrote:

> Itsuro ODA wrote:
> > 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>'.
> 
> Hi Itsuro,
> 
> But even from the kernel viewpoint, I'm not sure why it
would be better?
> The xen_phys_start symbol is an unsigned long, and its
container in the
> crash_xen_info_t is an unsigned long.  So I don't see
any benefit or reason
> to make it smaller.

Yes, I was too anxious.

> > 
> >> 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 ?
> 
> That's a good question.  My concern would be to have an
easier way for customers
> to deal with the problem without having to upgrade the
kernel package; it's always
> easier to upgrade a user-package.

I see.

> However, whatever gets fixed goes beyond the scope of
the crash utility.
> For now, the crash utility will need the
--xen-phys_start=xxxx workaround
> because the RHEL5.2 kernel will be released without a
fix.  And that
> decision will have to made by the kernel kexec/kdump
and kexec-tools
> maintainers.  Can you bring up this discussion on the
kexec list?

Yes, sure.

Thanks.
Itsuro Oda

> 
> Thanks,
>    Dave
> 
> > 
> > Thanks.
> > Itsuro Oda
> > 
> > On Fri, 18 Apr 2008 10:14:04 -0400
> > Dave Anderson <andersonredhat.com> 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.ta
r.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: %sn", 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);
> >>>  }
> > 
> 

-- 
Itsuro ODA <odavalinux.co.jp>

--
Crash-utility mailing list
Crash-utilityredhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Re: some updates of xencrash

Thread: Re: some updates of xencrash