Itsuro ODA wrote:

> Hi Dave,
>
> > - Introduced support for xendumps of para-virtualized ia64 kernels.
> >   It should be noted that currently the ia64 Xen kernel does not
> >   lay down a switch_stack for the panic task, so only raw "bt -t"
> >   backtraces can be done on the panic task.  (anderson@redhat.com)
>
> page_index may contain INVALID_MFN (0xffffffffffffffffUL).
> I think it is better to check it.
> Here is a patch for this.
>
> ---
> --- crash-4.0-3.15-org/xendump.h        2006-12-21 09:57:29.000000000 +0900
> +++ crash-4.0-3.15/xendump.h    2006-12-21 14:37:35.000000000 +0900
> @@ -113,3 +113,8 @@
>      uint32_t hypercall_imm; /* Break imm for Xen hypercalls.  */
>  /* #endif */
>  } xen_domctl_arch_setup_t;
> +
> +#ifdef IA64
> +#define INVALID_MFN (0xffffffffffffffffUL)
> +#endif
> +
> --- crash-4.0-3.15-org/xendump.c        2006-12-21 09:57:29.000000000 +0900
> +++ crash-4.0-3.15/xendump.c    2006-12-21 14:42:48.000000000 +0900
> @@ -31,6 +31,7 @@
>
>  static void xc_core_p2m_create(void);
>  static ulong xc_core_pfn_to_page_index(ulong);
> +static int check_mfn_is_valid(ulong);
>
>  /*
>   *  Determine whether a file is a xendump creation, and if TRUE,
> @@ -1362,7 +1363,7 @@
>         off_t offset;
>
>         if (xd->flags & XC_CORE_NO_P2MM)
> -               return pfn;
> +               return check_mfn_is_valid(pfn) ? pfn : PFN_NOT_FOUND;
>
>         idx = pfn/PFNS_PER_PAGE;
>
> @@ -1405,6 +1406,34 @@
>         return mfn_idx;
>  }
>
> +static int
> +check_mfn_is_valid(ulong idx)
> +{
> +        ulong mfn;
> +
> +       if (idx >= xd->xc_core.header.xch_nr_pages)
> +               return FALSE;
> +
> +        if (lseek(xd->xfd,
> +           (off_t)xd->xc_core.header.xch_index_offset + idx * sizeof(ulong),
> +            SEEK_SET) == -1) {
> +                error(INFO, "cannot lseek to page index\n");
> +                return FALSE;
> +        }
> +
> +       if (read(xd->xfd, &mfn, sizeof(ulong)) != sizeof(ulong)) {
> +               error(INFO, "cannot read index page %d\n", idx);
> +               return FALSE;
> +       }
> +
> +       if (mfn == INVALID_MFN) {
> +               error(INFO, "idx: %lx indicates INVALID_MFN\n", idx);
> +               return FALSE;
> +       } else {
> +               return TRUE;
> +       }
> +}
> +
>  /*
>   *  Store the panic task's stack hooks from where it was found
>   *  in get_active_set_panic_task().
> ---
>
> Thanks.
> --
> Itsuro ODA <oda@valinux.co.jp>

Oda-san,

Good catch.  I was never reading "bad addresses", so I didn't
see any issue there.  Now, after going back and checking it out,
I see that if you attempt to read an uninstantiated pfn, it returns
the page index, and so it seeks to and reads whatever's in the
xendump from that location.  Given the way the FV xendump is
laid out, there has to be *something* there in the dumpfile, which
I would have thought would have been sparse file space and
read as all zeroes?  But in some dumps, it looks like there's "junk"
data there?  Whatever -- it's harmless.

In any case, I agree with you -- it's much better to return a read
error as xc_core_read() expects.  I'll just adjust your patch slightly
to handle all architectures.

Thanks,
  Dave

Oda-san,

Thanks again for the "overnight-QA" and for introducing support
for xendumps of fully-virtualized ia64 kernels!  I've posted a new
crash version with both fixes.

I modified your check_mfn_is_valid() function so that it could
be used by all 3 architectures as well as for xendumps of
32-bit kernels running on a 64-bit host:
 

/*
 *  In xendumps containing INVALID_MFN markers in the page index,
 *  return the validity of the pfn.
 */
static int
xc_core_pfn_valid(ulong pfn)
{
        ulong mfn;
        off_t offset;

        if (pfn >= (ulong)xd->xc_core.header.xch_nr_pages)
                return FALSE;

        offset = (off_t)xd->xc_core.header.xch_index_offset;

        if (xd->flags & XC_CORE_64BIT_HOST)
                offset += (off_t)(pfn * sizeof(ulonglong));
        else
                offset += (off_t)(pfn * sizeof(ulong));

        /*
         *  The lseek and read should never fail, so report
         *  any errors unconditionally.
         */
        if (lseek(xd->xfd, offset, SEEK_SET) == -1) {
                error(INFO,
                    "xendump: cannot lseek to page index for pfn %lx\n",
                        pfn);
                return FALSE;
        }

        if (read(xd->xfd, &mfn, sizeof(ulong)) != sizeof(ulong)) {
                error(INFO,
                    "xendump: cannot read index page for pfn %lx\n",
                        pfn);
                return FALSE;
        }

        /*
         *  If it's an invalid mfn, let the caller decide whether
         *  to display an error message (unless debugging).
         */
        if (mfn == INVALID_MFN) {
                if (CRASHDEBUG(1))
                        error(INFO,
                            "xendump: pfn %lx contains INVALID_MFN\n",
                                pfn);
                return FALSE;
        }

        return TRUE;
}

Thanks again,
  Dave