xc_get_pfn_list() creates broken core files

xc_get_pfn_list() creates broken core files

[John Levon]

I noticed that the p2m list in domain core files was incorrect. The problem lies
in XEN_DOMCTL_getmemlist, here:

227             list_ent = d->page_list.next;
228             for ( i = 0; (i < max_pfns) && (list_ent != &d->page_list); i++ )
229             {
230                 mfn = page_to_mfn(list_entry(
231                     list_ent, struct page_info, list));
...
238                 list_ent = mfn_to_page(mfn)->list.next;
239             }

This does not account correctly for 'holes' where the pfn no longer maps an
mfn, and thus doesn't appear on page_list. As a result the whole array gets out
of sync. What's the right fix here? Below is what I threw together to verify
the problem.

thanks,
john

diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c
+++ b/tools/libxc/xc_core.c
@@ -103,7 +103,12 @@ xc_domain_dumpcore_via_callback(int xc_h
 
     for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
     {
-        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+	fprintf(stderr, "page_array %d is %lx\n", i, page_array[i]);
+	if (page_array[i] == -1) {
+		memset(dump_mem, 0, PAGE_SIZE);
+	} else {
+        	copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+	}
         dump_mem += PAGE_SIZE;
         if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
         {
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -215,6 +215,7 @@ long arch_do_domctl(
         struct domain *d = find_domain_by_id(domctl->domain);
         unsigned long max_pfns = domctl->u.getmemlist.max_pfns;
         unsigned long mfn, gmfn;
+	unsigned long gpfn;
         struct list_head *list_ent;
 
         ret = -EINVAL;
@@ -255,6 +256,26 @@ long arch_do_domctl(
                 {
                     mfn = page_to_mfn(list_entry(
                         list_ent, struct page_info, list));
+                    gpfn = get_gpfn_from_mfn(mfn);
+#ifdef __x86_64__
+                    if (gpfn != 0x5555555555555555L)
+#else
+                    if (gpfn != 0x55555555L)
+#endif
+                    {
+                        /* Account for unmapped gpfn's. */
+                        while (i < gpfn)
+                        {
+                            unsigned long tmp = -1;
+                            if ( copy_to_guest_offset(domctl->u.getmemlist.buffer,
+                                                      i, &tmp, 1) )
+                            {
+                                 ret = -EFAULT;
+                                 break;
+                            }
+                            i++;
+                        }
+                    }
                     if ( copy_to_guest_offset(domctl->u.getmemlist.buffer,
                                               i, &mfn, 1) )
                     {

Re: xc_get_pfn_list() creates broken core files

[Keir Fraser]

On 23/11/06 2:45 am, "John Levon" <levon@movementarian.org> wrote:

> This does not account correctly for 'holes' where the pfn no longer maps an
> mfn, and thus doesn't appear on page_list. As a result the whole array gets
> out
> of sync. What's the right fix here? Below is what I threw together to verify
> the problem.

Save out the guest's own p2m table. xc_linux_save has code to map it, and
from that code it's then easy to save it. get_pfn_list is deprecated.

 -- Keir

Re: xc_get_pfn_list() creates broken core files

[John Levon]

On Thu, Nov 23, 2006 at 08:52:36AM +0000, Keir Fraser wrote:

> > This does not account correctly for 'holes' where the pfn no longer maps an
> > mfn, and thus doesn't appear on page_list. As a result the whole array gets
> > out
> > of sync. What's the right fix here? Below is what I threw together to verify
> > the problem.
> 
> Save out the guest's own p2m table.

OK. What about HVM?

> get_pfn_list is deprecated.

Would be nice if that were mentioned somewhere :)

regards
john

Re: xc_get_pfn_list() creates broken core files

[Keir Fraser]

On 23/11/06 18:16, "John Levon" <levon@movementarian.org> wrote:

>> Save out the guest's own p2m table.
> 
> OK. What about HVM?

Pagetables etc. are all in 'p' address space, so there should be no need for
p2m or m2p in that case I think

In fact pagetables could be canonicalised into 'p' space for PV guests too
(xc_linux_save has the code for this) then there'd be no need to dump the
p2m table in the core dump at all. Save format and core dump format should
probably be one and the same!

 -- Keir

>> get_pfn_list is deprecated.
> 
> Would be nice if that were mentioned somewhere :)

Re: xc_get_pfn_list() creates broken core files

[John Levon]

On Thu, Nov 23, 2006 at 06:39:11PM +0000, Keir Fraser wrote:

> On 23/11/06 18:16, "John Levon" <levon@movementarian.org> wrote:
> 
> >> Save out the guest's own p2m table.
> > 
> > OK. What about HVM?
> 
> Pagetables etc. are all in 'p' address space, so there should be no need for
> p2m or m2p in that case I think

Sure, but I still need to map the domain pages, so I still need
xc_get_pfn_list() in this case, right? Will it work?

> In fact pagetables could be canonicalised into 'p' space for PV guests too
> (xc_linux_save has the code for this) then there'd be no need to dump the
> p2m table in the core dump at all. Save format and core dump format should
> probably be one and the same!

They should indeed be essentially the same format. But I'm not fond of
the idea of canonicalisation; it makes certain things impossible to
debug as you've lost all notion of what the pagetables /really/ were.
And there will be cases the domain is stuck in some HAT routine and I
really want to know what that local mfn variable refers to.

regards,
john

Re: xc_get_pfn_list() creates broken core files

[Keir Fraser]

On 23/11/06 7:17 pm, "John Levon" <levon@movementarian.org> wrote:

>> Pagetables etc. are all in 'p' address space, so there should be no need for
>> p2m or m2p in that case I think
> 
> Sure, but I still need to map the domain pages, so I still need
> xc_get_pfn_list() in this case, right? Will it work?

I'm imminently going to change the interface so that tools always map HVM
guest pages by pfn rather than mfn. That sidesteps this annoying issue.

 -- Keir

Re: xc_get_pfn_list() creates broken core files

[John Levon]

On Thu, Nov 23, 2006 at 07:20:29PM +0000, Keir Fraser wrote:

> I'm imminently going to change the interface so that tools always map HVM
> guest pages by pfn rather than mfn. That sidesteps this annoying issue.

How about the below. For now I've disabled HVM dumps until this
interface exists, but it should be ready to go.

# HG changeset patch
# User john.levon@sun.com
# Date 1164314513 28800
# Node ID c2cb4da91cd201f2645b1ead3964bcfecb5eef1a
# Parent  38037855268048b1326b8b86aad6736da88eea95
Use the guest's own p2m table instead of xc_get_pfn_list(), which cannot handle PFNs with no MFN.
Dump a zeroed page for PFNs with no MFN.
Clearly deprecate xc_get_pfn_list().
Do not include a P2M table with HVM domains.
Refuse to dump HVM until we can map its pages with PFNs.

Signed-off-by: John Levon <john.levon@sun.com>

diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c
+++ b/tools/libxc/xc_core.c
@@ -21,44 +21,139 @@ copy_from_domain_page(int xc_handle,
     return 0;
 }
 
+static int
+map_p2m(int xc_handle, xc_dominfo_t *info, xen_pfn_t **live_p2m,
+        unsigned long *pfnp)
+{
+    /* Double and single indirect references to the live P2M table */
+    xen_pfn_t *live_p2m_frame_list_list = NULL;
+    xen_pfn_t *live_p2m_frame_list = NULL;
+    shared_info_t *live_shinfo = NULL;
+    uint32_t dom = info->domid;
+    unsigned long max_pfn = 0;
+    int ret = -1;
+    int err;
+
+    /* Map the shared info frame */
+    live_shinfo = xc_map_foreign_range(xc_handle, dom, PAGE_SIZE,
+        PROT_READ, info->shared_info_frame);
+
+    if ( !live_shinfo )
+    {
+        PERROR("Couldn't map live_shinfo");
+        goto out;
+    }
+
+    max_pfn = live_shinfo->arch.max_pfn;
+
+    if ( max_pfn < info->nr_pages  )
+    {
+        ERROR("max_pfn < nr_pages -1 (%lx < %lx", max_pfn, info->nr_pages - 1);
+        goto out;
+    }
+
+    live_p2m_frame_list_list =
+        xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_READ,
+                             live_shinfo->arch.pfn_to_mfn_frame_list_list);
+
+    if ( !live_p2m_frame_list_list )
+    {
+        PERROR("Couldn't map p2m_frame_list_list (errno %d)", errno);
+        goto out;
+    }
+
+    live_p2m_frame_list =
+        xc_map_foreign_batch(xc_handle, dom, PROT_READ,
+                             live_p2m_frame_list_list,
+                             P2M_FLL_ENTRIES);
+
+    if ( !live_p2m_frame_list )
+    {
+        PERROR("Couldn't map p2m_frame_list");
+        goto out;
+    }
+
+    *live_p2m = xc_map_foreign_batch(xc_handle, dom, PROT_READ,
+                                    live_p2m_frame_list,
+                                    P2M_FL_ENTRIES);
+
+    if ( !live_p2m )
+    {
+        PERROR("Couldn't map p2m table");
+        goto out;
+    }
+
+    *pfnp = max_pfn;
+
+    ret = 0;
+
+out:
+    err = errno;
+
+    if ( live_shinfo )
+        munmap(live_shinfo, PAGE_SIZE);
+
+    if ( live_p2m_frame_list_list )
+        munmap(live_p2m_frame_list_list, PAGE_SIZE);
+
+    if ( live_p2m_frame_list )
+        munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
+
+    errno = err;
+    return ret;
+}
+
 int
 xc_domain_dumpcore_via_callback(int xc_handle,
                                 uint32_t domid,
                                 void *args,
                                 dumpcore_rtn_t dump_rtn)
 {
-    unsigned long nr_pages;
-    xen_pfn_t *page_array = NULL;
+    unsigned long nr_pages = 0;
+    unsigned long max_pfn = 0;
     xc_dominfo_t info;
     int i, nr_vcpus = 0;
     char *dump_mem, *dump_mem_start = NULL;
     struct xc_core_header header;
+    xen_pfn_t *p2m = NULL;
     vcpu_guest_context_t  ctxt[MAX_VIRT_CPUS];
     char dummy[PAGE_SIZE];
     int dummy_len;
-    int sts;
+    int sts = -1;
 
     if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
     {
         PERROR("Could not allocate dump_mem");
-        goto error_out;
+        goto out;
     }
 
     if ( xc_domain_getinfo(xc_handle, domid, 1, &info) != 1 )
     {
         PERROR("Could not get info for domain");
-        goto error_out;
+        goto out;
+    }
+
+    if ( info.hvm )
+    {
+        ERROR("Cannot dump HVM domains");
+        goto out;
     }
 
     if ( domid != info.domid )
     {
         PERROR("Domain %d does not exist", domid);
-        goto error_out;
+        goto out;
     }
 
     for ( i = 0; i <= info.max_vcpu_id; i++ )
         if ( xc_vcpu_getcontext(xc_handle, domid, i, &ctxt[nr_vcpus]) == 0)
             nr_vcpus++;
+
+    if ( nr_vcpus == 0 )
+    {
+        PERROR("No VCPU context could be grabbed");
+        goto out;
+    }
 
     nr_pages = info.nr_pages;
 
@@ -68,60 +163,66 @@ xc_domain_dumpcore_via_callback(int xc_h
     header.xch_ctxt_offset = sizeof(struct xc_core_header);
     header.xch_index_offset = sizeof(struct xc_core_header) +
         sizeof(vcpu_guest_context_t)*nr_vcpus;
-    dummy_len = (sizeof(struct xc_core_header) +
-                 (sizeof(vcpu_guest_context_t) * nr_vcpus) +
-                 (nr_pages * sizeof(xen_pfn_t)));
+    dummy_len = sizeof(struct xc_core_header) +
+                sizeof(vcpu_guest_context_t) * nr_vcpus;
+    if ( !info.hvm )
+        dummy_len += nr_pages * sizeof(xen_pfn_t);
     header.xch_pages_offset = round_pgup(dummy_len);
 
     sts = dump_rtn(args, (char *)&header, sizeof(struct xc_core_header));
     if ( sts != 0 )
-        goto error_out;
+        goto out;
 
     sts = dump_rtn(args, (char *)&ctxt, sizeof(ctxt[0]) * nr_vcpus);
     if ( sts != 0 )
-        goto error_out;
-
-    if ( (page_array = malloc(nr_pages * sizeof(xen_pfn_t))) == NULL )
-    {
-        IPRINTF("Could not allocate memory\n");
-        goto error_out;
-    }
-    if ( xc_get_pfn_list(xc_handle, domid, page_array, nr_pages) != nr_pages )
-    {
-        IPRINTF("Could not get the page frame list\n");
-        goto error_out;
-    }
-    sts = dump_rtn(args, (char *)page_array, nr_pages * sizeof(xen_pfn_t));
-    if ( sts != 0 )
-        goto error_out;
+        goto out;
+
+    if ( !info.hvm )
+    {
+        sts = map_p2m(xc_handle, &info, &p2m, &max_pfn);
+        if ( sts != 0 )
+            goto out;
+
+        sts = dump_rtn(args, (char *)p2m, sizeof (xen_pfn_t) * nr_pages);
+        if ( sts != 0 )
+            goto out;
+    }
 
     /* Pad the output data to page alignment. */
     memset(dummy, 0, PAGE_SIZE);
     sts = dump_rtn(args, dummy, header.xch_pages_offset - dummy_len);
     if ( sts != 0 )
-        goto error_out;
+        goto out;
 
     for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
     {
-        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+        if ( !info.hvm && p2m[i] == -1 )
+            memset(dump_mem, 0, PAGE_SIZE);
+        else
+            copy_from_domain_page(xc_handle, domid,
+                                  (info.hvm) ? i : p2m[i], dump_mem);
         dump_mem += PAGE_SIZE;
         if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
         {
             sts = dump_rtn(args, dump_mem_start, dump_mem - dump_mem_start);
             if ( sts != 0 )
-                goto error_out;
+                goto out;
             dump_mem = dump_mem_start;
         }
     }
 
+    sts = 0;
+
+out:
+    if ( p2m )
+    {
+        if ( info.hvm )
+            free ( p2m );
+        else
+            munmap(p2m, P2M_SIZE);
+    }
     free(dump_mem_start);
-    free(page_array);
-    return 0;
-
- error_out:
-    free(dump_mem_start);
-    free(page_array);
-    return -1;
+    return sts;
 }
 
 /* Callback args for writing to a local dump file. */
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -104,11 +104,17 @@ int xc_find_device_number(const char *na
  */
 
 typedef struct xc_core_header {
+    /* XC_CORE_MAGIC / XC_CORE_MAGIC_HVM */
     unsigned int xch_magic;
+    /* nr of vcpu_guest_contexts at xch_ctxt_offset */
     unsigned int xch_nr_vcpus;
+    /* nr of pages at xch_pages_offset, xch_index_offset */
     unsigned int xch_nr_pages;
+    /* offset of VCPU contexts */
     unsigned int xch_ctxt_offset;
+    /* offset of PFN->MFN array. Empty for HVM guests */
     unsigned int xch_index_offset;
+    /* offset of dump ages */
     unsigned int xch_pages_offset;
 } xc_core_header_t;
 
@@ -511,6 +517,10 @@ unsigned long xc_translate_foreign_addre
 unsigned long xc_translate_foreign_address(int xc_handle, uint32_t dom,
                                            int vcpu, unsigned long long virt);
 
+/**
+ * DEPRECATED.  Avoid using this, as it does not correctly account for PFNs
+ * without a backing MFN.
+ */
 int xc_get_pfn_list(int xc_handle, uint32_t domid, xen_pfn_t *pfn_buf,
                     unsigned long max_pfns);
 
diff --git a/tools/libxc/xg_private.h b/tools/libxc/xg_private.h
--- a/tools/libxc/xg_private.h
+++ b/tools/libxc/xg_private.h
@@ -119,6 +119,25 @@ typedef unsigned long l4_pgentry_t;
   (((_a) >> L4_PAGETABLE_SHIFT) & (L4_PAGETABLE_ENTRIES - 1))
 #endif
 
+#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
+
+/* Size in bytes of the P2M (rounded up to the nearest PAGE_SIZE bytes) */
+#define P2M_SIZE        ROUNDUP((max_pfn * sizeof(xen_pfn_t)), PAGE_SHIFT)
+
+/* Number of xen_pfn_t in a page */
+#define fpp             (PAGE_SIZE/sizeof(xen_pfn_t))
+
+/* Number of entries in the pfn_to_mfn_frame_list_list */
+#define P2M_FLL_ENTRIES (((max_pfn)+(fpp*fpp)-1)/(fpp*fpp))
+
+/* Number of entries in the pfn_to_mfn_frame_list */
+#define P2M_FL_ENTRIES  (((max_pfn)+fpp-1)/fpp)
+
+/* Size in bytes of the pfn_to_mfn_frame_list     */
+#define P2M_FL_SIZE     ((P2M_FL_ENTRIES)*sizeof(unsigned long))
+
+#define INVALID_P2M_ENTRY   (~0UL)
+
 struct domain_setup_info
 {
     uint64_t v_start;
diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h
--- a/tools/libxc/xg_save_restore.h
+++ b/tools/libxc/xg_save_restore.h
@@ -73,7 +73,6 @@ static int get_platform_info(int xc_hand
 */
 
 #define PFN_TO_KB(_pfn) ((_pfn) << (PAGE_SHIFT - 10))
-#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
 
 
 /*
@@ -86,21 +85,6 @@ static int get_platform_info(int xc_hand
 #define M2P_SIZE(_m)    ROUNDUP(((_m) * sizeof(xen_pfn_t)), M2P_SHIFT)
 #define M2P_CHUNKS(_m)  (M2P_SIZE((_m)) >> M2P_SHIFT)
 
-/* Size in bytes of the P2M (rounded up to the nearest PAGE_SIZE bytes) */
-#define P2M_SIZE        ROUNDUP((max_pfn * sizeof(xen_pfn_t)), PAGE_SHIFT)
-
-/* Number of xen_pfn_t in a page */
-#define fpp             (PAGE_SIZE/sizeof(xen_pfn_t))
-
-/* Number of entries in the pfn_to_mfn_frame_list */
-#define P2M_FL_ENTRIES  (((max_pfn)+fpp-1)/fpp)
-
-/* Size in bytes of the pfn_to_mfn_frame_list     */
-#define P2M_FL_SIZE     ((P2M_FL_ENTRIES)*sizeof(unsigned long))
-
-/* Number of entries in the pfn_to_mfn_frame_list_list */
-#define P2M_FLL_ENTRIES (((max_pfn)+(fpp*fpp)-1)/(fpp*fpp))
-
 /* Current guests allow 8MB 'slack' in their P2M */
 #define NR_SLACK_ENTRIES   ((8 * 1024 * 1024) / PAGE_SIZE)
 
@@ -109,8 +93,3 @@ static int get_platform_info(int xc_hand
 
 /* Returns TRUE if the PFN is currently mapped */
 #define is_mapped(pfn_type) (!((pfn_type) & 0x80000000UL))
-
-#define INVALID_P2M_ENTRY   (~0UL)
-
-
-

Re: xc_get_pfn_list() creates broken core files

[John Levon]

On Thu, Nov 23, 2006 at 08:52:57PM +0000, John Levon wrote:

> How about the below. For now I've disabled HVM dumps until this
> interface exists, but it should be ready to go.

Ugh, not that simple. We need to use max_pfn not nr_pages. Presumably
HVM domains have an accurate shared_info? And what do we do for
ia64/ppc?

john

Re: xc_get_pfn_list() creates broken core files

[Jimi Xenidis]

On Nov 23, 2006, at 5:05 PM, John Levon wrote:

> On Thu, Nov 23, 2006 at 08:52:57PM +0000, John Levon wrote:
>
>> How about the below. For now I've disabled HVM dumps until this
>> interface exists, but it should be ready to go.
>
> Ugh, not that simple. We need to use max_pfn not nr_pages. Presumably
> HVM domains have an accurate shared_info? And what do we do for
> ia64/ppc?

In PPC we are still using xc_get_pfn_list() and moving towards a p2m  
like data structure, but we are  hoping to have something better than  
an array of 4k entries, since we track memory at a larger granularity.
-JX

Re: xc_get_pfn_list() creates broken core files

[Keir Fraser]

On 23/11/06 8:52 pm, "John Levon" <levon@movementarian.org> wrote:

> Use the guest's own p2m table instead of xc_get_pfn_list(), which cannot
> handle PFNs with no MFN.
> Dump a zeroed page for PFNs with no MFN.
> Clearly deprecate xc_get_pfn_list().
> Do not include a P2M table with HVM domains.
> Refuse to dump HVM until we can map its pages with PFNs.
> 
> Signed-off-by: John Levon <john.levon@sun.com>

Rather than dump zero pages we could save a PFN-GMFN pair for each dumped
page. These can all go at the start of the core file in place of the p2m.
The dumped pages will then be in order of the PFN-GMFN pairs.

For a PV guest PFN=pseudophys, GMFN=machine (real). For an HVM guest
PFN=GMFN=pseudophys.

Finding max_pfn for a HVM guest is a bit tricky right now: we could add a
hypercall to read it out from Xen, or we could have Xen maintain the
shared_info max_pfn field.

 -- Keir

Re: xc_get_pfn_list() creates broken core files

[John Levon]

On Thu, Nov 23, 2006 at 11:33:59PM +0000, Keir Fraser wrote:

> Rather than dump zero pages we could save a PFN-GMFN pair for each dumped
> page. These can all go at the start of the core file in place of the p2m.
> The dumped pages will then be in order of the PFN-GMFN pairs.

I suppose we could do that; it would make reading things out a bit
harder though, since you couldn't just mmap() the table any more[1]. It
would be nice to be able to dump only up to the current ballooning
though.

Something to consider for the new format, though we're going to be using
something similar to the patch I sent for our 3.0.3-based stuff and
leave HVM dumps as something for later.

regards
john

[1] which is slightly annoying anyway, since it's not page aligned.

[PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)

[Isaku Yamahata]

[-- Attachment #1: Type: text/plain, Size: 19070 bytes --]

I added PFN-GMFN table to xen dump format and made it ELF format
based on John's patch. This patch isn't complete yet.

I choise ELF format because note section can be exteneded easily.
I suppose that anlysis tools (e.g. crash command) need more auxiliary 
infomation.

TODO
- Currently one program header per one page.
  It's possible to collapse program headers.
- HVM domain
- IA64 support


On Thu, Nov 23, 2006 at 11:48:39PM +0000, John Levon wrote:
> On Thu, Nov 23, 2006 at 11:33:59PM +0000, Keir Fraser wrote:
> 
> > Rather than dump zero pages we could save a PFN-GMFN pair for each dumped
> > page. These can all go at the start of the core file in place of the p2m.
> > The dumped pages will then be in order of the PFN-GMFN pairs.
> 
> I suppose we could do that; it would make reading things out a bit
> harder though, since you couldn't just mmap() the table any more[1]. It
> would be nice to be able to dump only up to the current ballooning
> though.
> 
> Something to consider for the new format, though we're going to be using
> something similar to the patch I sent for our 3.0.3-based stuff and
> leave HVM dumps as something for later.
> 
> regards
> john
> 
> [1] which is slightly annoying anyway, since it's not page aligned.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


# HG changeset patch
# User yamahata@valinux.co.jp
# Date 1168851172 -32400
# Node ID 9b0918c4332ef93b4352abf80a7c33a3b82b469f
# Parent  2b50acbdf01bfadbaab60a6d15a9f6a878d0224c
Use the guest's own p2m table instead of xc_get_pfn_list(), which cannot handle PFNs with no MFN.
Dump a zeroed page for PFNs with no MFN.
Clearly deprecate xc_get_pfn_list().
Do not include a P2M table with HVM domains.
Refuse to dump HVM until we can map its pages with PFNs.

Signed-off-by: John Levon <john.levon@sun.com>


PFN-GMFN table, ELF formatified.
TODO:
- Currently one program header per page.
  It's possible to collapse many program header.
- HVM domain
- IA64.
PATCHNAME: xen_dump_core_elf

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c	Sun Jan 14 17:22:24 2007 +0000
+++ b/tools/libxc/xc_core.c	Mon Jan 15 17:52:52 2007 +0900
@@ -1,10 +1,18 @@
+/*
+ * Elf format, (pfn, gmfn) table support.
+ * Copyright (c) 2006 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *
+ */
+
 #include "xg_private.h"
+#include "xc_elf.h"
+#include "xc_core.h"
 #include <stdlib.h>
 #include <unistd.h>
 
 /* number of pages to write at a time */
 #define DUMP_INCREMENT (4 * 1024)
-#define round_pgup(_p)    (((_p)+(PAGE_SIZE-1))&PAGE_MASK)
 
 static int
 copy_from_domain_page(int xc_handle,
@@ -21,107 +29,334 @@ copy_from_domain_page(int xc_handle,
     return 0;
 }
 
+static int
+map_p2m(int xc_handle, xc_dominfo_t *info, xen_pfn_t **live_p2m,
+        unsigned long *pfnp)
+{
+    /* Double and single indirect references to the live P2M table */
+    xen_pfn_t *live_p2m_frame_list_list = NULL;
+    xen_pfn_t *live_p2m_frame_list = NULL;
+    shared_info_t *live_shinfo = NULL;
+    uint32_t dom = info->domid;
+    unsigned long max_pfn = 0;
+    int ret = -1;
+    int err;
+
+    /* Map the shared info frame */
+    live_shinfo = xc_map_foreign_range(xc_handle, dom, PAGE_SIZE,
+        PROT_READ, info->shared_info_frame);
+
+    if ( !live_shinfo )
+    {
+        PERROR("Couldn't map live_shinfo");
+        goto out;
+    }
+
+    max_pfn = live_shinfo->arch.max_pfn;
+
+    if ( max_pfn < info->nr_pages  )
+    {
+        ERROR("max_pfn < nr_pages -1 (%lx < %lx", max_pfn, info->nr_pages - 1);
+        goto out;
+    }
+
+    live_p2m_frame_list_list =
+        xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_READ,
+                             live_shinfo->arch.pfn_to_mfn_frame_list_list);
+
+    if ( !live_p2m_frame_list_list )
+    {
+        PERROR("Couldn't map p2m_frame_list_list (errno %d)", errno);
+        goto out;
+    }
+
+    live_p2m_frame_list =
+        xc_map_foreign_batch(xc_handle, dom, PROT_READ,
+                             live_p2m_frame_list_list,
+                             P2M_FLL_ENTRIES);
+
+    if ( !live_p2m_frame_list )
+    {
+        PERROR("Couldn't map p2m_frame_list");
+        goto out;
+    }
+
+    *live_p2m = xc_map_foreign_batch(xc_handle, dom, PROT_READ,
+                                    live_p2m_frame_list,
+                                    P2M_FL_ENTRIES);
+
+    if ( !live_p2m )
+    {
+        PERROR("Couldn't map p2m table");
+        goto out;
+    }
+
+    *pfnp = max_pfn;
+
+
+    ret = 0;
+
+out:
+    err = errno;
+
+    if ( live_shinfo )
+        munmap(live_shinfo, PAGE_SIZE);
+
+    if ( live_p2m_frame_list_list )
+        munmap(live_p2m_frame_list_list, PAGE_SIZE);
+
+    if ( live_p2m_frame_list )
+        munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
+
+    errno = err;
+    return ret;
+}
+
 int
 xc_domain_dumpcore_via_callback(int xc_handle,
                                 uint32_t domid,
                                 void *args,
                                 dumpcore_rtn_t dump_rtn)
 {
-    unsigned long nr_pages;
-    xen_pfn_t *page_array = NULL;
     xc_dominfo_t info;
-    int i, nr_vcpus = 0;
+    int nr_vcpus = 0;
     char *dump_mem, *dump_mem_start = NULL;
-    struct xc_core_header header;
     vcpu_guest_context_t  ctxt[MAX_VIRT_CPUS];
     char dummy[PAGE_SIZE];
     int dummy_len;
-    int sts;
+    int sts = -1;
+
+    unsigned long filesz;
+    unsigned long i;
+    unsigned long j;
+    unsigned long nr_pages;
+    xen_pfn_t *p2m;
+    unsigned long max_pfn;
+    struct p2m *p2m_array = NULL;
+    unsigned long offset;
+
+    Elf_Ehdr ehdr;
+    Elf_Phdr phdr;
+    struct xen_note note;
+    struct xen_core_header_desc core_header;
 
     if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
     {
         PERROR("Could not allocate dump_mem");
-        goto error_out;
+        goto out;
     }
 
     if ( xc_domain_getinfo(xc_handle, domid, 1, &info) != 1 )
     {
         PERROR("Could not get info for domain");
-        goto error_out;
+        goto out;
+    }
+
+    if ( info.hvm )
+    {
+        ERROR("Cannot dump HVM domains");
+        goto out;
     }
 
     if ( domid != info.domid )
     {
         PERROR("Domain %d does not exist", domid);
-        goto error_out;
+        goto out;
     }
 
     for ( i = 0; i <= info.max_vcpu_id; i++ )
         if ( xc_vcpu_getcontext(xc_handle, domid, i, &ctxt[nr_vcpus]) == 0)
             nr_vcpus++;
+    if ( nr_vcpus == 0 )
+    {
+        PERROR("No VCPU context could be grabbed");
+        goto out;
+    }
 
     nr_pages = info.nr_pages;
-
-    header.xch_magic = info.hvm ? XC_CORE_MAGIC_HVM : XC_CORE_MAGIC;
-    header.xch_nr_vcpus = nr_vcpus;
-    header.xch_nr_pages = nr_pages;
-    header.xch_ctxt_offset = sizeof(struct xc_core_header);
-    header.xch_index_offset = sizeof(struct xc_core_header) +
-        sizeof(vcpu_guest_context_t)*nr_vcpus;
-    dummy_len = (sizeof(struct xc_core_header) +
-                 (sizeof(vcpu_guest_context_t) * nr_vcpus) +
-                 (nr_pages * sizeof(xen_pfn_t)));
-    header.xch_pages_offset = round_pgup(dummy_len);
-
-    sts = dump_rtn(args, (char *)&header, sizeof(struct xc_core_header));
-    if ( sts != 0 )
-        goto error_out;
-
+    p2m_array = malloc(nr_pages * sizeof(struct p2m));
+    if ( p2m_array == NULL )
+    {
+        PERROR("Count not allocate p2m array");
+        goto out;
+    }
+
+    /* obtain p2m table */
+    if ( !info.hvm )
+    {
+        sts = map_p2m(xc_handle, &info, &p2m, &max_pfn);
+        if ( sts != 0 )
+            goto out;
+    }
+    
+    memset(&ehdr, 0, sizeof(ehdr));
+    ehdr.e_ident[EI_MAG0] = ELFMAG0;
+    ehdr.e_ident[EI_MAG1] = ELFMAG1;
+    ehdr.e_ident[EI_MAG2] = ELFMAG2;
+    ehdr.e_ident[EI_MAG3] = ELFMAG3;
+    ehdr.e_ident[EI_CLASS] = ELFCLASS;
+
+    ehdr.e_ident[EI_DATA] = ELFDATA2LSB; /* XXX */
+    //ehdr.e_ident[EI_DATA] = ELFDATA2MSB;
+
+    ehdr.e_ident[EI_VERSION] = EV_CURRENT;
+    ehdr.e_ident[EI_OSABI] = ELFOSABI_LINUX; 
+    ehdr.e_ident[EI_ABIVERSION] = EV_CURRENT;
+
+    ehdr.e_type = ET_CORE;
+    ehdr.e_machine = 
+#if defined(__i386__)
+        EM_386
+#elif defined(__x86_64__)
+        EM_X86_64
+#else
+# error "unsupported archtecture"        
+#endif
+        ;
+
+    ehdr.e_version = EV_CURRENT;
+    ehdr.e_entry = 0;
+    ehdr.e_phoff = sizeof(ehdr);
+    ehdr.e_shoff = 0;
+#ifndef ELF_CORE_EFLAGS
+#define ELF_CORE_EFLAGS 0
+#endif
+    ehdr.e_flags = ELF_CORE_EFLAGS;
+    ehdr.e_ehsize = sizeof(ehdr);
+    ehdr.e_phentsize = sizeof(Elf_Phdr);
+    ehdr.e_phnum = nr_pages + 1; /* notes */
+    ehdr.e_shentsize = 0;
+    ehdr.e_shnum = 0;
+    ehdr.e_shstrndx = 0;
+    sts = dump_rtn(args, (char*)&ehdr, sizeof(ehdr));
+    if ( sts != 0 )
+        goto out;
+
+    /* create program header */
+    offset = sizeof(ehdr);
+    
+    /* note section */
+    offset += (1 + nr_pages) * sizeof(phdr); /* note section + nr_pages */
+    filesz = sizeof(struct xen_core_header) + /* core header */
+        sizeof(struct xen_note) + sizeof(ctxt[0]) * nr_vcpus + /* vcpu context */
+        sizeof(struct xen_note_p2m) + sizeof(p2m_array[0]) * nr_pages; /* p2m table */
+
+    memset(&phdr, 0, sizeof(phdr));
+    phdr.p_type = PT_NOTE;
+    phdr.p_flags = 0;
+    phdr.p_offset = offset;
+    phdr.p_vaddr = 0;
+    phdr.p_paddr = 0;
+    phdr.p_filesz = filesz;
+    phdr.p_memsz = 0;
+    phdr.p_align = 0;
+
+    sts = dump_rtn(args, (char*)&phdr, sizeof(phdr));
+    if ( sts != 0)
+        goto out;
+
+    offset += filesz;
+    dummy_len = ROUNDUP(offset, PAGE_SHIFT) - offset; /* padding length */
+    offset = ROUNDUP(offset, PAGE_SHIFT);
+    j = 0;
+    for (i = 0; i < max_pfn && j < nr_pages; i++) 
+    {
+        if (p2m[i] == INVALID_P2M_ENTRY)
+            continue;
+        
+        memset(&phdr, 0, sizeof(phdr));
+        phdr.p_type = PT_LOAD;
+        phdr.p_flags = PF_X | PF_W | PF_R;
+        phdr.p_offset = offset;
+        phdr.p_vaddr = 0;
+        phdr.p_paddr = i * PAGE_SIZE;
+        phdr.p_filesz = PAGE_SIZE;
+        phdr.p_memsz = PAGE_SIZE;
+        phdr.p_align = 0;
+        sts = dump_rtn(args, (char*)&phdr, sizeof(phdr));
+        if ( sts != 0)
+            goto out;
+
+        offset += PAGE_SIZE;
+        p2m_array[j].pfn = i;
+        p2m_array[j].gmfn = p2m[i];
+        j++;
+    }
+    if ( j != nr_pages )
+        PERROR("j(%ld) != nr_pages (%ld)", j, nr_pages);
+
+    /* note section */
+    memset(&note, 0, sizeof(note));
+    note.namesz = strlen(XEN_NOTES) + 1;
+    strncpy(note.name, XEN_NOTES, sizeof(note.name));
+    
+    /* note section:xen core header */
+    note.descsz = sizeof(core_header);
+    note.type = NT_XEN_HEADER;
+    core_header.xch_magic = info.hvm ? XC_CORE_MAGIC_HVM : XC_CORE_MAGIC;
+    core_header.xch_nr_vcpus = nr_vcpus;
+    core_header.xch_nr_pages = nr_pages;
+    core_header.xch_page_size = PAGE_SIZE;
+    sts = dump_rtn(args, (char*)&note, sizeof(note));
+    if ( sts != 0)
+        goto out;
+    sts = dump_rtn(args, (char*)&core_header, sizeof(core_header));
+    if ( sts != 0)
+        goto out;
+
+    /* note section:xen vcpu prstatus */
+    note.descsz = sizeof(ctxt[0]) * nr_vcpus;
+    note.type = NT_XEN_PRSTATUS;
+    sts = dump_rtn(args, (char*)&note, sizeof(note));
+    if ( sts != 0)
+        goto out;
     sts = dump_rtn(args, (char *)&ctxt, sizeof(ctxt[0]) * nr_vcpus);
     if ( sts != 0 )
-        goto error_out;
-
-    if ( (page_array = malloc(nr_pages * sizeof(xen_pfn_t))) == NULL )
-    {
-        IPRINTF("Could not allocate memory\n");
-        goto error_out;
-    }
-    if ( xc_get_pfn_list(xc_handle, domid, page_array, nr_pages) != nr_pages )
-    {
-        IPRINTF("Could not get the page frame list\n");
-        goto error_out;
-    }
-    sts = dump_rtn(args, (char *)page_array, nr_pages * sizeof(xen_pfn_t));
-    if ( sts != 0 )
-        goto error_out;
-
+        goto out;
+    
+    /* note section:create p2m table */
+    note.descsz = sizeof(p2m_array[0]) * nr_pages;
+    note.type = NT_XEN_P2M;
+    sts = dump_rtn(args, (char*)&note, sizeof(note));
+    if ( sts != 0 )
+        goto out;
+    sts = dump_rtn(args, (char *)p2m_array, sizeof(p2m_array[0]) * nr_pages);
+    if ( sts != 0 )
+        goto out;
+    
     /* Pad the output data to page alignment. */
     memset(dummy, 0, PAGE_SIZE);
-    sts = dump_rtn(args, dummy, header.xch_pages_offset - dummy_len);
-    if ( sts != 0 )
-        goto error_out;
-
+    sts = dump_rtn(args, dummy, dummy_len);
+    if ( sts != 0 )
+        goto out;
+
+    /* dump pages */
     for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
     {
-        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+        copy_from_domain_page(xc_handle, domid, p2m_array[i].gmfn, dump_mem);
         dump_mem += PAGE_SIZE;
         if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
         {
             sts = dump_rtn(args, dump_mem_start, dump_mem - dump_mem_start);
             if ( sts != 0 )
-                goto error_out;
+                goto out;
             dump_mem = dump_mem_start;
         }
     }
 
+    sts = 0;
+    
+out:
+    if ( p2m )
+    {
+        if ( info.hvm )
+            free( p2m );
+        else
+            munmap(p2m, P2M_SIZE);
+    }
     free(dump_mem_start);
-    free(page_array);
-    return 0;
-
- error_out:
-    free(dump_mem_start);
-    free(page_array);
-    return -1;
+    free(p2m_array);
+    return sts;
 }
 
 /* Callback args for writing to a local dump file. */
diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xc_core.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxc/xc_core.h	Mon Jan 15 17:52:52 2007 +0900
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2006 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ */
+
+#ifndef XC_CORE_H
+#define XC_CORE_H
+
+#define XEN_NOTES       "XEN CORE"
+
+/* Notes used in xen core*/
+#define NT_XEN_HEADER   7
+#define NT_XEN_PRSTATUS 8
+#define NT_XEN_P2M      9
+
+
+struct xen_note {
+    uint32_t    namesz;
+    uint32_t    descsz;
+    uint32_t    type;
+    char        name[12]; /* to hold XEN_NOTES and 64bit aligned.
+                           * 8 <= sizeof(XEN_NOTES) < 12
+                           */
+};
+
+
+struct xen_core_header_desc {
+    uint64_t    xch_magic;
+    uint64_t    xch_nr_vcpus;
+    uint64_t    xch_nr_pages;
+    uint64_t    xch_page_size;
+};
+
+struct p2m {
+    xen_pfn_t pfn;
+    xen_pfn_t gmfn;
+};
+
+
+struct xen_core_header {
+    struct xen_note             note;
+    struct xen_core_header_desc core_header;
+};
+
+struct xen_note_prstatus {
+    struct xen_note             note;
+    vcpu_guest_context_t        ctxt[0];
+};
+
+struct xen_note_p2m {
+    struct xen_note     note;
+    struct p2m          p2m[0];
+};
+
+#endif /* XC_CORE_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Sun Jan 14 17:22:24 2007 +0000
+++ b/tools/libxc/xenctrl.h	Mon Jan 15 17:52:52 2007 +0900
@@ -513,6 +513,10 @@ unsigned long xc_translate_foreign_addre
 unsigned long xc_translate_foreign_address(int xc_handle, uint32_t dom,
                                            int vcpu, unsigned long long virt);
 
+/**
+ * DEPRECATED.  Avoid using this, as it does not correctly account for PFNs
+ * without a backing MFN.
+ */
 int xc_get_pfn_list(int xc_handle, uint32_t domid, xen_pfn_t *pfn_buf,
                     unsigned long max_pfns);
 
diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xg_private.h
--- a/tools/libxc/xg_private.h	Sun Jan 14 17:22:24 2007 +0000
+++ b/tools/libxc/xg_private.h	Mon Jan 15 17:52:52 2007 +0900
@@ -119,6 +119,25 @@ typedef unsigned long l4_pgentry_t;
   (((_a) >> L4_PAGETABLE_SHIFT) & (L4_PAGETABLE_ENTRIES - 1))
 #endif
 
+#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
+
+/* Size in bytes of the P2M (rounded up to the nearest PAGE_SIZE bytes) */
+#define P2M_SIZE        ROUNDUP((max_pfn * sizeof(xen_pfn_t)), PAGE_SHIFT)
+
+/* Number of xen_pfn_t in a page */
+#define fpp             (PAGE_SIZE/sizeof(xen_pfn_t))
+
+/* Number of entries in the pfn_to_mfn_frame_list_list */
+#define P2M_FLL_ENTRIES (((max_pfn)+(fpp*fpp)-1)/(fpp*fpp))
+
+/* Number of entries in the pfn_to_mfn_frame_list */
+#define P2M_FL_ENTRIES  (((max_pfn)+fpp-1)/fpp)
+
+/* Size in bytes of the pfn_to_mfn_frame_list     */
+#define P2M_FL_SIZE     ((P2M_FL_ENTRIES)*sizeof(unsigned long))
+
+#define INVALID_P2M_ENTRY   (~0UL)
+
 struct domain_setup_info
 {
     uint64_t v_start;
diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xg_save_restore.h
--- a/tools/libxc/xg_save_restore.h	Sun Jan 14 17:22:24 2007 +0000
+++ b/tools/libxc/xg_save_restore.h	Mon Jan 15 17:52:52 2007 +0900
@@ -82,7 +82,6 @@ static int get_platform_info(int xc_hand
 */
 
 #define PFN_TO_KB(_pfn) ((_pfn) << (PAGE_SHIFT - 10))
-#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
 
 
 /*
@@ -95,25 +94,5 @@ static int get_platform_info(int xc_hand
 #define M2P_SIZE(_m)    ROUNDUP(((_m) * sizeof(xen_pfn_t)), M2P_SHIFT)
 #define M2P_CHUNKS(_m)  (M2P_SIZE((_m)) >> M2P_SHIFT)
 
-/* Size in bytes of the P2M (rounded up to the nearest PAGE_SIZE bytes) */
-#define P2M_SIZE        ROUNDUP((max_pfn * sizeof(xen_pfn_t)), PAGE_SHIFT)
-
-/* Number of xen_pfn_t in a page */
-#define fpp             (PAGE_SIZE/sizeof(xen_pfn_t))
-
-/* Number of entries in the pfn_to_mfn_frame_list */
-#define P2M_FL_ENTRIES  (((max_pfn)+fpp-1)/fpp)
-
-/* Size in bytes of the pfn_to_mfn_frame_list     */
-#define P2M_FL_SIZE     ((P2M_FL_ENTRIES)*sizeof(unsigned long))
-
-/* Number of entries in the pfn_to_mfn_frame_list_list */
-#define P2M_FLL_ENTRIES (((max_pfn)+(fpp*fpp)-1)/(fpp*fpp))
-
 /* Returns TRUE if the PFN is currently mapped */
 #define is_mapped(pfn_type) (!((pfn_type) & 0x80000000UL))
-
-#define INVALID_P2M_ENTRY   (~0UL)
-
-
-

-- 
yamahata

[-- Attachment #2: 13378_9b0918c4332e_xen_dump_core_elf.patch --]
[-- Type: text/x-diff, Size: 17654 bytes --]

# HG changeset patch
# User yamahata@valinux.co.jp
# Date 1168851172 -32400
# Node ID 9b0918c4332ef93b4352abf80a7c33a3b82b469f
# Parent  2b50acbdf01bfadbaab60a6d15a9f6a878d0224c
Use the guest's own p2m table instead of xc_get_pfn_list(), which cannot handle PFNs with no MFN.
Dump a zeroed page for PFNs with no MFN.
Clearly deprecate xc_get_pfn_list().
Do not include a P2M table with HVM domains.
Refuse to dump HVM until we can map its pages with PFNs.

Signed-off-by: John Levon <john.levon@sun.com>


PFN-GMFN table, ELF formatified.
TODO:
- Currently one program header per page.
  It's possible to collapse many program header.
- HVM domain
- IA64.
PATCHNAME: xen_dump_core_elf

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c	Sun Jan 14 17:22:24 2007 +0000
+++ b/tools/libxc/xc_core.c	Mon Jan 15 17:52:52 2007 +0900
@@ -1,10 +1,18 @@
+/*
+ * Elf format, (pfn, gmfn) table support.
+ * Copyright (c) 2006 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *
+ */
+
 #include "xg_private.h"
+#include "xc_elf.h"
+#include "xc_core.h"
 #include <stdlib.h>
 #include <unistd.h>
 
 /* number of pages to write at a time */
 #define DUMP_INCREMENT (4 * 1024)
-#define round_pgup(_p)    (((_p)+(PAGE_SIZE-1))&PAGE_MASK)
 
 static int
 copy_from_domain_page(int xc_handle,
@@ -21,107 +29,334 @@ copy_from_domain_page(int xc_handle,
     return 0;
 }
 
+static int
+map_p2m(int xc_handle, xc_dominfo_t *info, xen_pfn_t **live_p2m,
+        unsigned long *pfnp)
+{
+    /* Double and single indirect references to the live P2M table */
+    xen_pfn_t *live_p2m_frame_list_list = NULL;
+    xen_pfn_t *live_p2m_frame_list = NULL;
+    shared_info_t *live_shinfo = NULL;
+    uint32_t dom = info->domid;
+    unsigned long max_pfn = 0;
+    int ret = -1;
+    int err;
+
+    /* Map the shared info frame */
+    live_shinfo = xc_map_foreign_range(xc_handle, dom, PAGE_SIZE,
+        PROT_READ, info->shared_info_frame);
+
+    if ( !live_shinfo )
+    {
+        PERROR("Couldn't map live_shinfo");
+        goto out;
+    }
+
+    max_pfn = live_shinfo->arch.max_pfn;
+
+    if ( max_pfn < info->nr_pages  )
+    {
+        ERROR("max_pfn < nr_pages -1 (%lx < %lx", max_pfn, info->nr_pages - 1);
+        goto out;
+    }
+
+    live_p2m_frame_list_list =
+        xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_READ,
+                             live_shinfo->arch.pfn_to_mfn_frame_list_list);
+
+    if ( !live_p2m_frame_list_list )
+    {
+        PERROR("Couldn't map p2m_frame_list_list (errno %d)", errno);
+        goto out;
+    }
+
+    live_p2m_frame_list =
+        xc_map_foreign_batch(xc_handle, dom, PROT_READ,
+                             live_p2m_frame_list_list,
+                             P2M_FLL_ENTRIES);
+
+    if ( !live_p2m_frame_list )
+    {
+        PERROR("Couldn't map p2m_frame_list");
+        goto out;
+    }
+
+    *live_p2m = xc_map_foreign_batch(xc_handle, dom, PROT_READ,
+                                    live_p2m_frame_list,
+                                    P2M_FL_ENTRIES);
+
+    if ( !live_p2m )
+    {
+        PERROR("Couldn't map p2m table");
+        goto out;
+    }
+
+    *pfnp = max_pfn;
+
+
+    ret = 0;
+
+out:
+    err = errno;
+
+    if ( live_shinfo )
+        munmap(live_shinfo, PAGE_SIZE);
+
+    if ( live_p2m_frame_list_list )
+        munmap(live_p2m_frame_list_list, PAGE_SIZE);
+
+    if ( live_p2m_frame_list )
+        munmap(live_p2m_frame_list, P2M_FLL_ENTRIES * PAGE_SIZE);
+
+    errno = err;
+    return ret;
+}
+
 int
 xc_domain_dumpcore_via_callback(int xc_handle,
                                 uint32_t domid,
                                 void *args,
                                 dumpcore_rtn_t dump_rtn)
 {
-    unsigned long nr_pages;
-    xen_pfn_t *page_array = NULL;
     xc_dominfo_t info;
-    int i, nr_vcpus = 0;
+    int nr_vcpus = 0;
     char *dump_mem, *dump_mem_start = NULL;
-    struct xc_core_header header;
     vcpu_guest_context_t  ctxt[MAX_VIRT_CPUS];
     char dummy[PAGE_SIZE];
     int dummy_len;
-    int sts;
+    int sts = -1;
+
+    unsigned long filesz;
+    unsigned long i;
+    unsigned long j;
+    unsigned long nr_pages;
+    xen_pfn_t *p2m;
+    unsigned long max_pfn;
+    struct p2m *p2m_array = NULL;
+    unsigned long offset;
+
+    Elf_Ehdr ehdr;
+    Elf_Phdr phdr;
+    struct xen_note note;
+    struct xen_core_header_desc core_header;
 
     if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
     {
         PERROR("Could not allocate dump_mem");
-        goto error_out;
+        goto out;
     }
 
     if ( xc_domain_getinfo(xc_handle, domid, 1, &info) != 1 )
     {
         PERROR("Could not get info for domain");
-        goto error_out;
+        goto out;
+    }
+
+    if ( info.hvm )
+    {
+        ERROR("Cannot dump HVM domains");
+        goto out;
     }
 
     if ( domid != info.domid )
     {
         PERROR("Domain %d does not exist", domid);
-        goto error_out;
+        goto out;
     }
 
     for ( i = 0; i <= info.max_vcpu_id; i++ )
         if ( xc_vcpu_getcontext(xc_handle, domid, i, &ctxt[nr_vcpus]) == 0)
             nr_vcpus++;
+    if ( nr_vcpus == 0 )
+    {
+        PERROR("No VCPU context could be grabbed");
+        goto out;
+    }
 
     nr_pages = info.nr_pages;
-
-    header.xch_magic = info.hvm ? XC_CORE_MAGIC_HVM : XC_CORE_MAGIC;
-    header.xch_nr_vcpus = nr_vcpus;
-    header.xch_nr_pages = nr_pages;
-    header.xch_ctxt_offset = sizeof(struct xc_core_header);
-    header.xch_index_offset = sizeof(struct xc_core_header) +
-        sizeof(vcpu_guest_context_t)*nr_vcpus;
-    dummy_len = (sizeof(struct xc_core_header) +
-                 (sizeof(vcpu_guest_context_t) * nr_vcpus) +
-                 (nr_pages * sizeof(xen_pfn_t)));
-    header.xch_pages_offset = round_pgup(dummy_len);
-
-    sts = dump_rtn(args, (char *)&header, sizeof(struct xc_core_header));
-    if ( sts != 0 )
-        goto error_out;
-
+    p2m_array = malloc(nr_pages * sizeof(struct p2m));
+    if ( p2m_array == NULL )
+    {
+        PERROR("Count not allocate p2m array");
+        goto out;
+    }
+
+    /* obtain p2m table */
+    if ( !info.hvm )
+    {
+        sts = map_p2m(xc_handle, &info, &p2m, &max_pfn);
+        if ( sts != 0 )
+            goto out;
+    }
+    
+    memset(&ehdr, 0, sizeof(ehdr));
+    ehdr.e_ident[EI_MAG0] = ELFMAG0;
+    ehdr.e_ident[EI_MAG1] = ELFMAG1;
+    ehdr.e_ident[EI_MAG2] = ELFMAG2;
+    ehdr.e_ident[EI_MAG3] = ELFMAG3;
+    ehdr.e_ident[EI_CLASS] = ELFCLASS;
+
+    ehdr.e_ident[EI_DATA] = ELFDATA2LSB; /* XXX */
+    //ehdr.e_ident[EI_DATA] = ELFDATA2MSB;
+
+    ehdr.e_ident[EI_VERSION] = EV_CURRENT;
+    ehdr.e_ident[EI_OSABI] = ELFOSABI_LINUX; 
+    ehdr.e_ident[EI_ABIVERSION] = EV_CURRENT;
+
+    ehdr.e_type = ET_CORE;
+    ehdr.e_machine = 
+#if defined(__i386__)
+        EM_386
+#elif defined(__x86_64__)
+        EM_X86_64
+#else
+# error "unsupported archtecture"        
+#endif
+        ;
+
+    ehdr.e_version = EV_CURRENT;
+    ehdr.e_entry = 0;
+    ehdr.e_phoff = sizeof(ehdr);
+    ehdr.e_shoff = 0;
+#ifndef ELF_CORE_EFLAGS
+#define ELF_CORE_EFLAGS 0
+#endif
+    ehdr.e_flags = ELF_CORE_EFLAGS;
+    ehdr.e_ehsize = sizeof(ehdr);
+    ehdr.e_phentsize = sizeof(Elf_Phdr);
+    ehdr.e_phnum = nr_pages + 1; /* notes */
+    ehdr.e_shentsize = 0;
+    ehdr.e_shnum = 0;
+    ehdr.e_shstrndx = 0;
+    sts = dump_rtn(args, (char*)&ehdr, sizeof(ehdr));
+    if ( sts != 0 )
+        goto out;
+
+    /* create program header */
+    offset = sizeof(ehdr);
+    
+    /* note section */
+    offset += (1 + nr_pages) * sizeof(phdr); /* note section + nr_pages */
+    filesz = sizeof(struct xen_core_header) + /* core header */
+        sizeof(struct xen_note) + sizeof(ctxt[0]) * nr_vcpus + /* vcpu context */
+        sizeof(struct xen_note_p2m) + sizeof(p2m_array[0]) * nr_pages; /* p2m table */
+
+    memset(&phdr, 0, sizeof(phdr));
+    phdr.p_type = PT_NOTE;
+    phdr.p_flags = 0;
+    phdr.p_offset = offset;
+    phdr.p_vaddr = 0;
+    phdr.p_paddr = 0;
+    phdr.p_filesz = filesz;
+    phdr.p_memsz = 0;
+    phdr.p_align = 0;
+
+    sts = dump_rtn(args, (char*)&phdr, sizeof(phdr));
+    if ( sts != 0)
+        goto out;
+
+    offset += filesz;
+    dummy_len = ROUNDUP(offset, PAGE_SHIFT) - offset; /* padding length */
+    offset = ROUNDUP(offset, PAGE_SHIFT);
+    j = 0;
+    for (i = 0; i < max_pfn && j < nr_pages; i++) 
+    {
+        if (p2m[i] == INVALID_P2M_ENTRY)
+            continue;
+        
+        memset(&phdr, 0, sizeof(phdr));
+        phdr.p_type = PT_LOAD;
+        phdr.p_flags = PF_X | PF_W | PF_R;
+        phdr.p_offset = offset;
+        phdr.p_vaddr = 0;
+        phdr.p_paddr = i * PAGE_SIZE;
+        phdr.p_filesz = PAGE_SIZE;
+        phdr.p_memsz = PAGE_SIZE;
+        phdr.p_align = 0;
+        sts = dump_rtn(args, (char*)&phdr, sizeof(phdr));
+        if ( sts != 0)
+            goto out;
+
+        offset += PAGE_SIZE;
+        p2m_array[j].pfn = i;
+        p2m_array[j].gmfn = p2m[i];
+        j++;
+    }
+    if ( j != nr_pages )
+        PERROR("j(%ld) != nr_pages (%ld)", j, nr_pages);
+
+    /* note section */
+    memset(&note, 0, sizeof(note));
+    note.namesz = strlen(XEN_NOTES) + 1;
+    strncpy(note.name, XEN_NOTES, sizeof(note.name));
+    
+    /* note section:xen core header */
+    note.descsz = sizeof(core_header);
+    note.type = NT_XEN_HEADER;
+    core_header.xch_magic = info.hvm ? XC_CORE_MAGIC_HVM : XC_CORE_MAGIC;
+    core_header.xch_nr_vcpus = nr_vcpus;
+    core_header.xch_nr_pages = nr_pages;
+    core_header.xch_page_size = PAGE_SIZE;
+    sts = dump_rtn(args, (char*)&note, sizeof(note));
+    if ( sts != 0)
+        goto out;
+    sts = dump_rtn(args, (char*)&core_header, sizeof(core_header));
+    if ( sts != 0)
+        goto out;
+
+    /* note section:xen vcpu prstatus */
+    note.descsz = sizeof(ctxt[0]) * nr_vcpus;
+    note.type = NT_XEN_PRSTATUS;
+    sts = dump_rtn(args, (char*)&note, sizeof(note));
+    if ( sts != 0)
+        goto out;
     sts = dump_rtn(args, (char *)&ctxt, sizeof(ctxt[0]) * nr_vcpus);
     if ( sts != 0 )
-        goto error_out;
-
-    if ( (page_array = malloc(nr_pages * sizeof(xen_pfn_t))) == NULL )
-    {
-        IPRINTF("Could not allocate memory\n");
-        goto error_out;
-    }
-    if ( xc_get_pfn_list(xc_handle, domid, page_array, nr_pages) != nr_pages )
-    {
-        IPRINTF("Could not get the page frame list\n");
-        goto error_out;
-    }
-    sts = dump_rtn(args, (char *)page_array, nr_pages * sizeof(xen_pfn_t));
-    if ( sts != 0 )
-        goto error_out;
-
+        goto out;
+    
+    /* note section:create p2m table */
+    note.descsz = sizeof(p2m_array[0]) * nr_pages;
+    note.type = NT_XEN_P2M;
+    sts = dump_rtn(args, (char*)&note, sizeof(note));
+    if ( sts != 0 )
+        goto out;
+    sts = dump_rtn(args, (char *)p2m_array, sizeof(p2m_array[0]) * nr_pages);
+    if ( sts != 0 )
+        goto out;
+    
     /* Pad the output data to page alignment. */
     memset(dummy, 0, PAGE_SIZE);
-    sts = dump_rtn(args, dummy, header.xch_pages_offset - dummy_len);
-    if ( sts != 0 )
-        goto error_out;
-
+    sts = dump_rtn(args, dummy, dummy_len);
+    if ( sts != 0 )
+        goto out;
+
+    /* dump pages */
     for ( dump_mem = dump_mem_start, i = 0; i < nr_pages; i++ )
     {
-        copy_from_domain_page(xc_handle, domid, page_array[i], dump_mem);
+        copy_from_domain_page(xc_handle, domid, p2m_array[i].gmfn, dump_mem);
         dump_mem += PAGE_SIZE;
         if ( ((i + 1) % DUMP_INCREMENT == 0) || ((i + 1) == nr_pages) )
         {
             sts = dump_rtn(args, dump_mem_start, dump_mem - dump_mem_start);
             if ( sts != 0 )
-                goto error_out;
+                goto out;
             dump_mem = dump_mem_start;
         }
     }
 
+    sts = 0;
+    
+out:
+    if ( p2m )
+    {
+        if ( info.hvm )
+            free( p2m );
+        else
+            munmap(p2m, P2M_SIZE);
+    }
     free(dump_mem_start);
-    free(page_array);
-    return 0;
-
- error_out:
-    free(dump_mem_start);
-    free(page_array);
-    return -1;
+    free(p2m_array);
+    return sts;
 }
 
 /* Callback args for writing to a local dump file. */
diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xc_core.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxc/xc_core.h	Mon Jan 15 17:52:52 2007 +0900
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2006 Isaku Yamahata <yamahata at valinux co jp>
+ *                    VA Linux Systems Japan K.K.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ */
+
+#ifndef XC_CORE_H
+#define XC_CORE_H
+
+#define XEN_NOTES       "XEN CORE"
+
+/* Notes used in xen core*/
+#define NT_XEN_HEADER   7
+#define NT_XEN_PRSTATUS 8
+#define NT_XEN_P2M      9
+
+
+struct xen_note {
+    uint32_t    namesz;
+    uint32_t    descsz;
+    uint32_t    type;
+    char        name[12]; /* to hold XEN_NOTES and 64bit aligned.
+                           * 8 <= sizeof(XEN_NOTES) < 12
+                           */
+};
+
+
+struct xen_core_header_desc {
+    uint64_t    xch_magic;
+    uint64_t    xch_nr_vcpus;
+    uint64_t    xch_nr_pages;
+    uint64_t    xch_page_size;
+};
+
+struct p2m {
+    xen_pfn_t pfn;
+    xen_pfn_t gmfn;
+};
+
+
+struct xen_core_header {
+    struct xen_note             note;
+    struct xen_core_header_desc core_header;
+};
+
+struct xen_note_prstatus {
+    struct xen_note             note;
+    vcpu_guest_context_t        ctxt[0];
+};
+
+struct xen_note_p2m {
+    struct xen_note     note;
+    struct p2m          p2m[0];
+};
+
+#endif /* XC_CORE_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Sun Jan 14 17:22:24 2007 +0000
+++ b/tools/libxc/xenctrl.h	Mon Jan 15 17:52:52 2007 +0900
@@ -513,6 +513,10 @@ unsigned long xc_translate_foreign_addre
 unsigned long xc_translate_foreign_address(int xc_handle, uint32_t dom,
                                            int vcpu, unsigned long long virt);
 
+/**
+ * DEPRECATED.  Avoid using this, as it does not correctly account for PFNs
+ * without a backing MFN.
+ */
 int xc_get_pfn_list(int xc_handle, uint32_t domid, xen_pfn_t *pfn_buf,
                     unsigned long max_pfns);
 
diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xg_private.h
--- a/tools/libxc/xg_private.h	Sun Jan 14 17:22:24 2007 +0000
+++ b/tools/libxc/xg_private.h	Mon Jan 15 17:52:52 2007 +0900
@@ -119,6 +119,25 @@ typedef unsigned long l4_pgentry_t;
   (((_a) >> L4_PAGETABLE_SHIFT) & (L4_PAGETABLE_ENTRIES - 1))
 #endif
 
+#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
+
+/* Size in bytes of the P2M (rounded up to the nearest PAGE_SIZE bytes) */
+#define P2M_SIZE        ROUNDUP((max_pfn * sizeof(xen_pfn_t)), PAGE_SHIFT)
+
+/* Number of xen_pfn_t in a page */
+#define fpp             (PAGE_SIZE/sizeof(xen_pfn_t))
+
+/* Number of entries in the pfn_to_mfn_frame_list_list */
+#define P2M_FLL_ENTRIES (((max_pfn)+(fpp*fpp)-1)/(fpp*fpp))
+
+/* Number of entries in the pfn_to_mfn_frame_list */
+#define P2M_FL_ENTRIES  (((max_pfn)+fpp-1)/fpp)
+
+/* Size in bytes of the pfn_to_mfn_frame_list     */
+#define P2M_FL_SIZE     ((P2M_FL_ENTRIES)*sizeof(unsigned long))
+
+#define INVALID_P2M_ENTRY   (~0UL)
+
 struct domain_setup_info
 {
     uint64_t v_start;
diff -r 2b50acbdf01b -r 9b0918c4332e tools/libxc/xg_save_restore.h
--- a/tools/libxc/xg_save_restore.h	Sun Jan 14 17:22:24 2007 +0000
+++ b/tools/libxc/xg_save_restore.h	Mon Jan 15 17:52:52 2007 +0900
@@ -82,7 +82,6 @@ static int get_platform_info(int xc_hand
 */
 
 #define PFN_TO_KB(_pfn) ((_pfn) << (PAGE_SHIFT - 10))
-#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
 
 
 /*
@@ -95,25 +94,5 @@ static int get_platform_info(int xc_hand
 #define M2P_SIZE(_m)    ROUNDUP(((_m) * sizeof(xen_pfn_t)), M2P_SHIFT)
 #define M2P_CHUNKS(_m)  (M2P_SIZE((_m)) >> M2P_SHIFT)
 
-/* Size in bytes of the P2M (rounded up to the nearest PAGE_SIZE bytes) */
-#define P2M_SIZE        ROUNDUP((max_pfn * sizeof(xen_pfn_t)), PAGE_SHIFT)
-
-/* Number of xen_pfn_t in a page */
-#define fpp             (PAGE_SIZE/sizeof(xen_pfn_t))
-
-/* Number of entries in the pfn_to_mfn_frame_list */
-#define P2M_FL_ENTRIES  (((max_pfn)+fpp-1)/fpp)
-
-/* Size in bytes of the pfn_to_mfn_frame_list     */
-#define P2M_FL_SIZE     ((P2M_FL_ENTRIES)*sizeof(unsigned long))
-
-/* Number of entries in the pfn_to_mfn_frame_list_list */
-#define P2M_FLL_ENTRIES (((max_pfn)+(fpp*fpp)-1)/(fpp*fpp))
-
 /* Returns TRUE if the PFN is currently mapped */
 #define is_mapped(pfn_type) (!((pfn_type) & 0x80000000UL))
-
-#define INVALID_P2M_ENTRY   (~0UL)
-
-
-

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)

[Keir Fraser]

On 15/1/07 09:16, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:

> I added PFN-GMFN table to xen dump format and made it ELF format
> based on John's patch. This patch isn't complete yet.
> 
> I choise ELF format because note section can be exteneded easily.
> I suppose that anlysis tools (e.g. crash command) need more auxiliary
> infomation.

Thanks. This is definitely the direction we want to go!

I wonder whether our new save format should be Elf too. I'm not sure it will
buy us anything much, but perhaps the dump format will be extensible enough
if we define suitable extra Elf notes for the extra non-cpu state that we
need. Something to think about.

 -- Keir

Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)

[John Levon]

On Mon, Jan 15, 2007 at 10:13:56AM +0000, Keir Fraser wrote:

> I wonder whether our new save format should be Elf too. I'm not sure it will

Yes, yes, yes... it would be very handy to able to debug saved files
too, and the contents of both are nearly identical. We'd have to deviate
slightly from spec for migration across the wire (for example we could
still fill in section headers and leave the unknown offsets+size as a
special sentinel value or something)

regards
john

Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: xc_get_pfn_list() creates broken core files)

[John Levon]

On Mon, Jan 15, 2007 at 06:16:47PM +0900, Isaku Yamahata wrote:

> I added PFN-GMFN table to xen dump format and made it ELF format
> based on John's patch. This patch isn't complete yet.

You use LINUX for the OSABI, which doesn't seem right.

I'm not clear why these are all program headers instead of sections.
Given the slightly odd nature of this file, I would expect it to have no
phdrs, or at most one phdr for the page array (and plus a section
header). Same goes for notes - those would seem more useful as a
section per type of thing (e.g. a .XEN_vcpu_context section, etc.). And
that way you get the "what is this section's size" and "how big are the
entries" for free.

But before all this, most importantly, I'd like all interested parties
to sit down and design (and document!) the exact contents of the ELF
format first, especially considering the needs of HVM and save/migration
format. It doesn't make sense to me to start coding until we at least
have a good idea of what we need in the new format.

regards
john

Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)

[Isaku Yamahata]

On Wed, Jan 17, 2007 at 03:59:28AM +0000, John Levon wrote:
> On Mon, Jan 15, 2007 at 06:16:47PM +0900, Isaku Yamahata wrote:
> 
> > I added PFN-GMFN table to xen dump format and made it ELF format
> > based on John's patch. This patch isn't complete yet.
> 
> You use LINUX for the OSABI, which doesn't seem right.

Do you have any other correct value?
e.g. ELFOSABI_NONE (= ELFOSABI_SYSV = 0)
Or should it be compile-time configurable by defining something
like ELF_OSABI?


> I'm not clear why these are all program headers instead of sections.
> Given the slightly odd nature of this file, I would expect it to have no
> phdrs, or at most one phdr for the page array (and plus a section
> header). Same goes for notes - those would seem more useful as a
> section per type of thing (e.g. a .XEN_vcpu_context section, etc.). And
> that way you get the "what is this section's size" and "how big are the
> entries" for free.

I followed the way creating process core file and linux kernel dump file.
Process core file and linux kernel dump file use only program headers.
They use note section pointed by a program header to store register
infomations and etc.

Creating sections requires string table to store section name.
It makes dump-core code more compilated.
If using sections is appropreate for dump-core, such compilcations
might be acceptable.
However I don't see why section is more appropreate than program
header because I don't expect to have so many kind of notes.
At least for dump-core.

-- 
yamahata

Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: xc_get_pfn_list() creates broken core files)

[John Levon]

On Wed, Jan 17, 2007 at 02:54:51PM +0900, Isaku Yamahata wrote:

> Do you have any other correct value?
> e.g. ELFOSABI_NONE

NONE seems righter.

> I followed the way creating process core file and linux kernel dump file.
> Process core file and linux kernel dump file use only program headers.

But in both of those cases we have access to meaningful virtual address
maps, which isn't true here (or certainly not trivially true!)

> They use note section pointed by a program header to store register
> infomations and etc.
> Creating sections requires string table to store section name.
> It makes dump-core code more compilated.
> If using sections is appropreate for dump-core, such compilcations
> might be acceptable.
> However I don't see why section is more appropreate than program
> header because I don't expect to have so many kind of notes.
> At least for dump-core.

We want the same format for everything. And e.g. objcopy is designed for
manipulating sections (copying out contents of a section etc). Building
a .shstrstab isn't particularly difficult...

regards
john

Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)

[Isaku Yamahata]

On Wed, Jan 17, 2007 at 01:06:33PM +0000, John Levon wrote:
> > I followed the way creating process core file and linux kernel dump file.
> > Process core file and linux kernel dump file use only program headers.
> But in both of those cases we have access to meaningful virtual address
> maps, which isn't true here (or certainly not trivially true!)

In xen dump-core file, (pseudo) physical address(phdr.p_paddr) is true.
This seems reasonable.


> And e.g. objcopy is designed for
> manipulating sections (copying out contents of a section etc). 

Sections and tools like objcopy are for linking.
Program headers are for describing memory layout in virtual/physical
address.
I don't think that we want linking manipulations for core dump file.

-- 
yamahata

Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: xc_get_pfn_list() creates broken core files)

[John Levon]

On Thu, Jan 18, 2007 at 10:41:10AM +0900, Isaku Yamahata wrote:

> > > I followed the way creating process core file and linux kernel dump file.
> > > Process core file and linux kernel dump file use only program headers.
> > But in both of those cases we have access to meaningful virtual address
> > maps, which isn't true here (or certainly not trivially true!)
> 
> In xen dump-core file, (pseudo) physical address(phdr.p_paddr) is true.
> This seems reasonable.

Sure, but it's strictly duplicated information, and to get less than one
phdr per page you've got to do a load of coalescing, which seems way
more complicated than using sections :)

regards
john