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
