Xen devel - Feb 2007 - [PATCH 3/3] dump-core take 5: elf formatify and added PFN-GMFN table

3 / 3 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Feb 02, 2007 at 09:42:28PM +0900, Isaku Yamahata wrote:

+ *  |section headers                                         |
+ *  |    null section header                                 |
+ *  |    .shstrtab                                           |
+ *  |    .note.Xen                                           |
+ *  |    .Xen.p2m or .Xen.pfn                                |
+ *  |    .Xen.pages                                          |

.xen_p2m as names would be more typical (e.g. Sun use .SUNW_ctf, etc.).

(This wasn''t really what I meant by documentation, I was talking about
a
document like the ELF standard that fully defines the content of each
section etc.)

+    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);

We should check for a zero value for this, and not even try, give some
warning like "domain hasn''t booted/resumed enough to be able to
dump
core".

+memory_map_get_old_hvm(int xc_handle, xc_dominfo_t *info, 

Can''t we have all the ia64 stuff in a separate xc_core_ia64?
It''s always
been painfully difficult to read the old xc_linux_build.c, for example,
because of this.

+    rc = xc_version(xc_handle, XENVER_pagesize, NULL);

Great, but are we also storing PAE vs. non-PAE? Currently there''s no
way
to even find out if a core is PAE or not (!)

+static int
+elfnote_fill_format_version(struct xen_elfnote_dumpcore_format_version_desc
+                            *format_version)
+{
+    format_version->major = XEN_DUMPCORE_FORMAT_MAJOR_VERSION;
+    format_version->minor = XEN_DUMPCORE_FORMAT_MINOR_VERSION;
+    format_version->extra = XEN_DUMPCORE_FORMAT_EXTRA_VERSION;

For what purpose do you see the "extra version" being used for? Why
isn''t this just one number? If we''re bumping a minor version,
it means
we must be /adding/ something - can''t the client code just check if
that
new thing exists?

+                /* try to map page to determin wheter it has underlying page */

''whether''

+        sizeof(struct xen_elfnote_dumpcore_none) +           /* none */
+        sizeof(struct xen_elfnote_dumpcore_header) +         /* core header */
+        sizeof(struct xen_elfnote_dumpcore_xen_version) +    /* xen version */
+        sizeof(struct xen_elfnote_dumpcore_format_version) + /* format version
*/
+        sizeof(struct xen_elfnote_dumpcore_prstatus) + sizeof(ctxt[0]) *
nr_vcpus; /* vcpu context */

Wouldn''t prstatus make more sense as a section (entsize and all that)

regards
john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel