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

[Petr Tesarik]

V Mon, 3 Dec 2012 15:28:15 +0900
Atsushi Kumagai <kumagai-atsushi at mxc.nes.nec.co.jp> napsáno:

> 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.

Hello Atsushi-san,

I'm sorry for my late reaction. Yes, this piece should be removed.
While there is a xen_phys_start variable in 32-bit Xen, it is always
zero, so we shouldn't complain if we can't get it.

Petr Tesarik