Sounds good. I think it might be convienent to have a pointer in the specific
header to point back to the generic header, and vice versa. Also, maybe an
#ifdeffed typedef in the arch-specific header to define the commonly-used
register size, i.e. uint32_t for Intel, uint64_t for Alpha, but the code could
just use the "standard" arch specific typesize, I dunno, "reg_size_t" or maybe
"word_size_t" (this may already be defined somewhere?). We could also define
some "standard" arch specific info (naming convention!), like dh_pc and dh_sp
for the PC & SP.
On 03-Jan-2000 Matt Robinson wrote:
> Well, what _really_ needs to happen, and I'll do this for 1.0.4,
> is to do the following (and I intended to do this from the beginning,
> but we were desperate to get something out the door):
>
> +-----------------------------------+
> | generic dump header |
> +-----------------------------------+
> | architecture specific dump header |
> +-----------------------------------+
> | |
> | rest of dump |
> | |
> +-----------------------------------+
>
> That way, we can differentiate the generic from the processor
> specific stuff. So what we do is stuff the dump header into the
> top of the dump_page_buf, then write the architecture specific
> header right after it.
>
> Then you can have whatever you want in there, and it doesn't
> matter as far as other architectures are concerned.
>
> We can also add an dh_arch_type flag to the base dump header
> so you know what architecture the dump is for.
>
> Your thoughts? If you agree, I'll do this and have it out by
> the end of this week.
>
> --Matt
>
> On Mon, 3 Jan 2000, Brian Hall wrote:
>|>Since Intel & Alpha will be sharing the dump_header_t strucure, I thought
>|>there
>|>might be a problem with using 64 bit values on the Intel side, so I just
>|>added
>|>fields for Alpha matching Alpha's dh_regs. If this isn't a problem, I can
>|>just
>|>remove dh_esp and dh_eip from this structure. However, then the naming
>|>convention will match Alpha's dh_regs and not Intel's...
>|>
>|>Is there a reason we couldn't just store the pc & sp for either
>|>platform into the appropriate slot in pt_regs here?
>|>
>|>IOW, remove dh_esp, dh_eip, dh_pc, dh_ps, and pick the variable name based
>|>on
>|>the platform, i.e. in arch-alpha/vmdump.c:
>|>
>|>pt_regs->ps = sptr;
>|>pt_regs->pc = &&dump_here;
>|>
>|>Of course the code called after __dump_execute would need to refer to the
>|>registers this way also. Or will this not work because pt_regs gets
>|>overwritten?
>|>
>|>
>|>
>|>OK, here is my proposed change to dump_header_t.:
>|>
>|>
>|>typedef struct _dump_header_s {
>|> /* the dump magic number -- unique to verify dump is valid */
>|> uint64_t dh_magic_number;
>|>
>|> /* the version number of this dump */
>|> uint32_t dh_version;
>|>
>|> /* the size of this header (in case we can't read it) */
>|> uint32_t dh_header_size;
>|>
>|> /* the level of this dump (just a header?) */
>|> uint32_t dh_dump_level;
>|>
>|> /* the size of a Linux memory page (4K, 8K, 16K, etc.) */
>|> uint32_t dh_page_size;
>|>
>|> /* the size of all physical memory */
>|> uint64_t dh_memory_size;
>|>
>|> /* the start of physical memory */
>|> uint64_t dh_memory_start;
>|>
>|> /* the end of physical memory */
>|> uint64_t dh_memory_end;
>|>
>|> /* the esp for i386 systems -- MOVE LATER */
>|> uint32_t dh_esp;
>|>
>|> /* the eip for i386 systems -- MOVE LATER */
>|> uint32_t dh_eip;
>|>
>|> /* the program counter for Alpha systems -- ADDED */
>|> uint64_t dh_pc;
>|>
>|> /* the stack pointer for Alpha systems -- ADDED */
>|> uint64_t dh_ps;
>|>
>|> /* the number of pages in this dump specifically */
>|> uint32_t dh_num_pages;
>|>
>|> /* the panic string, if available */
>|> char dh_panic_string[DUMP_PANIC_LEN];
>|>
>|> /* the time of the system crash */
>|> struct timeval dh_time;
>|>
>|> /* the utsname (uname) information */
>|> struct new_utsname dh_utsname;
>|>
>|> /* the dump registers */
>|> struct pt_regs dh_regs;
>|>
>|> /* the address of the current task */
>|> struct task_struct *dh_current_task;
>|>
>|>} dump_header_t;
>|>
>|>
>|>On 22-Dec-1999 Matt Robinson wrote:
>|>> On Fri, 24 Dec 1999, Brian Hall wrote:
>|>>|>That fixed the vmdump.h problem. Now it actually compiles, with warnings
>|>>|>(implied casts). I notice that dh_esp and dh_eip are 32 bits wide; they
>|>>|>will
>|>>|>need to be 64 bit for Alpha; and the other uint32_t(s) in vmdump.c
>|>>|>(__dump_execute) will also need to be uint64_t(s), and the changes
>|>>|>propagated, correct?
>|>>|>
>|>>|>Now, should I go ahead and change dump_header_t to (choose one):
>|>>|>1) make dh_esp,etc uint64_t(s) in the header;
>|>>
>|>> I'd make them whatever the dh_esp and dh_eip equivalents are in the
>|>> pt_regs structure for SP and PC.
>|>>
>|>>|>2) add fields specifically for alpha;
>|>>|>3) do #1 and change them to dh_sp and dh_pc while I'm at it?
>|>>
>|>> Do the name change while you're at it. I'll change things in our patch.
>|>
>|>--
>|>Brian Hall <brianw.hall@xxxxxxxxxx>
>|>http://www.bigfoot.com/~brihall
>|>Linux Consultant
>|>
--
Brian Hall <brianw.hall@xxxxxxxxxx>
http://www.bigfoot.com/~brihall
Linux Consultant
|