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) )
{
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2006-Nov-23 08:52 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2006-Nov-23 18:16 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-Nov-23 18:39 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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 :)_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Pratt
2006-Nov-23 18:53 UTC
RE: [Xen-devel] xc_get_pfn_list() creates broken core files
> 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!Trouble is, the PV guest hasn''t done an orderly suspend, and may be holding machine address references in registers/memory and in pagetables that haven''t been pinned yet. The guest''s core dump will already contain the p2m table, and its root location has been registered with xen during boot, so this just needs to be saved too. Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-Nov-23 19:05 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
On 23/11/06 6:53 pm, "Ian Pratt" <m+Ian.Pratt@cl.cam.ac.uk> wrote:>> 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! > > Trouble is, the PV guest hasn''t done an orderly suspend, and may be > holding machine address references in registers/memory and in pagetables > that haven''t been pinned yet. > > The guest''s core dump will already contain the p2m table, and its root > location has been registered with xen during boot, so this just needs to > be saved too.It''s questionable how useful the ability to m2p-translate those marginally possible bits of state is though. Currently the p2m table is used *only* to construct an m2p table allowing pagetable walks in xc_ptrace_core(). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2006-Nov-23 19:17 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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 thinkSure, 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-Nov-23 19:20 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2006-Nov-23 20:52 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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) - - - _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2006-Nov-23 22:05 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-Nov-23 23:33 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2006-Nov-23 23:48 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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
Jimi Xenidis
2006-Nov-25 16:03 UTC
Re: [Xen-devel] xc_get_pfn_list() creates broken core files
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Isaku Yamahata
2007-Jan-15 09:16 UTC
[PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)
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(¬e, 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*)¬e, 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*)¬e, 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*)¬e, 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Jan-15 10:13 UTC
Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2007-Jan-15 12:38 UTC
Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)
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 willYes, 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2007-Jan-17 03:59 UTC
Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Isaku Yamahata
2007-Jan-17 05:54 UTC
Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2007-Jan-17 13:06 UTC
Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)
On Wed, Jan 17, 2007 at 02:54:51PM +0900, Isaku Yamahata wrote:> Do you have any other correct value? > e.g. ELFOSABI_NONENONE 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Isaku Yamahata
2007-Jan-18 01:41 UTC
Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2007-Jan-18 12:42 UTC
Re: [PATCH][RFC] dump-core: PFN-GMFN table and ELF formatify (was Re: [Xen-devel] xc_get_pfn_list() creates broken core files)
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel