[PATCHv3 5/9] Initialize phys_start during early Xen setup

[Atsushi Kumagai]

Hello Petr,

On Wed, 28 Nov 2012 14:56:58 +0900
Atsushi Kumagai <kumagai-atsushi at mxc.nes.nec.co.jp> wrote:

> Hello Petr,
> 
> On Fri, 24 Aug 2012 17:42:24 +0200
> Petr Tesarik <ptesarik at suse.cz> wrote:
> 
> > With early Xen setup, we can move the initialization of xen_phys_start
> > to the setup routine. This is cleaner, as we can get rid of ifdef'ed
> > arch-specific code in makedumpfile.c (which should IMO stay generic).
> > 
> > Signed-off-by: Petr Tesarik <ptesarik at suse.cz>
> > 
> > ---
> >  arch/x86.c     |    9 +++++++++
> >  arch/x86_64.c  |    9 +++++++++
> >  makedumpfile.c |   16 ----------------
> >  3 files changed, 18 insertions(+), 16 deletions(-)
> > 
> > --- a/arch/x86.c
> > +++ b/arch/x86.c
> > @@ -296,6 +296,15 @@ int get_xen_basic_info_x86(void)
> >  	unsigned long frame_table_vaddr;
> >  	unsigned long xen_end;
> >  
> > +	if (!info->xen_phys_start) {
> > +		if (info->xen_crash_info_v < 2) {
> > +			ERRMSG("Can't get Xen physical start address.\n"
> > +			       "Please use the --xen_phys_start option.");
> > +			return FALSE;
> > +		}
> > +		info->xen_phys_start = info->xen_crash_info.v2->xen_phys_start;
> > +	}
> > +
> 
> I tested for the vmcore which doesn't include xen_phys_start, then the 
> error message below showed:
> 
>   $ makedumpfile_v1.5.1 -E -X --xen-syms xen-syms-2.6.18-92.el5.debug vmcore dumpfile.EX
>   get_xen_basic_info_x86: Can't get Xen physical start address.
>   Please use the --xen_phys_start option.
>   makedumpfile Failed.
>   $
> 
> So, should I understand this result as reasonable error handling ?
> 
> The reason why I have this question is because the same vmcore can be
> filtered out with makedumpfile-1.5.0 or before like below:
> 
>   $ makedumpfile_v1.5.0 -E -X --xen-syms xen-syms-2.6.18-92.el5.debug vmcore dumpfile.EX
>   Switched running mode from cyclic to non-cyclic,
>   because the cyclic mode doesn't support Xen.
>   Copying data                       : [100 %]
> 
>   The dumpfile is saved to dumpfile.EX.
> 
>   makedumpfile Completed.
>   $
> 
> I have checked with grep, it seems that xen_phys_start is used for
> x86_64 only. So, is the valid check really necessary for x86 ?

I don't know much about Xen, but I think this is just regression.
And I want to release v1.5.1 in this week, so I will fix this regression
with the patch below:

diff --git a/arch/x86.c b/arch/x86.c
index 7de0495..ef29e3c 100644
--- a/arch/x86.c
+++ b/arch/x86.c
@@ -294,15 +294,6 @@ kvtop_xen_x86(unsigned long kvaddr)

 int get_xen_basic_info_x86(void)
 {
-       if (!info->xen_phys_start) {
-               if (info->xen_crash_info_v < 2) {
-                       ERRMSG("Can't get Xen physical start address.\n"
-                              "Please use the --xen_phys_start option.");
-                       return FALSE;
-               }
-               info->xen_phys_start = info->xen_crash_info.v2->xen_phys_start;
-       }
-
        if (SYMBOL(pgd_l2) == NOT_FOUND_SYMBOL &&
            SYMBOL(pgd_l3) == NOT_FOUND_SYMBOL) {
                ERRMSG("Can't get pgd.\n");


Please let me know if you have any objections.

Thanks
Atsushi Kumagai


> Thanks
> Atsushi Kumagai
> 
> >  	if (SYMBOL(pgd_l2) == NOT_FOUND_SYMBOL &&
> >  	    SYMBOL(pgd_l3) == NOT_FOUND_SYMBOL) {
> >  		ERRMSG("Can't get pgd.\n");
> > --- a/arch/x86_64.c
> > +++ b/arch/x86_64.c
> > @@ -361,6 +361,15 @@ int get_xen_basic_info_x86_64(void)
> >  	unsigned long frame_table_vaddr;
> >  	unsigned long xen_end;
> >  
> > + 	if (!info->xen_phys_start) {
> > +		if (info->xen_crash_info_v < 2) {
> > +			ERRMSG("Can't get Xen physical start address.\n"
> > +			       "Please use the --xen_phys_start option.");
> > +			return FALSE;
> > +		}
> > +		info->xen_phys_start = info->xen_crash_info.v2->xen_phys_start;
> > +	}
> > +
> >  	if (SYMBOL(pgd_l4) == NOT_FOUND_SYMBOL) {
> >  		ERRMSG("Can't get pml4.\n");
> >  		return FALSE;
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -5365,20 +5365,6 @@ init_xen_crash_info(void)
> >  }
> >  
> >  int
> > -get_xen_phys_start(void)
> > -{
> > -	if (info->xen_phys_start)
> > -		return TRUE;
> > -
> > -#if defined(__x86__) || defined(__x86_64__)
> > -	if (info->xen_crash_info_v >= 2)
> > -		info->xen_phys_start = info->xen_crash_info.v2->xen_phys_start;
> > -#endif
> > -
> > -	return TRUE;
> > -}
> > -
> > -int
> >  get_xen_info(void)
> >  {
> >  	unsigned long domain;
> > @@ -5877,8 +5863,6 @@ initial_xen(void)
> >  		if (!read_vmcoreinfo_from_vmcore(offset, size, TRUE))
> >  			return FALSE;
> >  	}
> > -	if (!get_xen_phys_start())
> > -		return FALSE;
> >  	if (!get_xen_info())
> >  		return FALSE;
> >  
> > 
> > 
> > 
> > _______________________________________________
> > kexec mailing list
> > kexec at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec