Itsuro ODA wrote:
Hi, Dave

> However, given the above, I wonder whether it would be possible
> to postpone the setting of the XEN_HYPER flag from the
> setup_environment() function until later on in main() when
> the command line arguments are parsed?  Wouldn't it be possible
> to recognize that the xen-syms object file is the hypervisor
> binary, and to set the XEN_HYPER flag at that point?

I think it is difficult to decide according to content of the
object. It is available to assume the file name of xen-syms.
For example I attached a new version. (main.c only)
Do you think which is better ?

I'm not sure, but I do prefer the checking the "xen-syms" name
vs. checking "crash" vs. "xencrash".  What I want to avoid is
using the "xencrash/crash" string as the differentiator, because
it confuses the fact that -- with respect to crash -- the use
of the term "xen" currently implies a dom0 or domU kernel
session.

One small change I might prefer is that the call to check_xen_hyper()
should be made right after is_elf_file() has successfully accepted
the "xen-syms*" command line argument.

However, what I was thinking was that once you have returned from
symtab_init(), you can then easily distinguish between a vmlinux
and xen-syms file by the existence of per-binary symbol names.
And since the xen-syms and vmlinux versions of machdep_init()
for PRE_SYMTAB are identical, that wouldn't seem to be a
problem, at least for x86.

Another possibility would be, again just after the is_elf_file()
call has returned, to check the ELF header for obvious clues
that the binary is *not* a vmlinux file.  Like for example,
on an x86_64, the xen-syms file has this PT_LOAD segment:

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x001000 0xff100000 0xff100000 0x92658 0x92658 RWE 0x1000

With those VirtAddr and PhysAddr (???) values, there's no
way it could be a vmlinux binary.

But -- perhaps the simplest way is the better way, and your check
for the first 8 bytes in the "xen-syms" namelist name is sufficient
for now.

However, we should also have a "--hyper" command line argument to
force a hypervisor session in case everything else fails.

 

> 2. Can you make it work with a live hypervisor system
>    in the same way the crash works with a live Linux
>    kernel?

No. There is no method to see hypervisor's memory.
 

Ah, OK -- that's too bad.  That would be nice...

A couple of other suggestions...

1. I understand that the x86_64 is under development,
   but the patch as it is now won't compile because the
   call to x86_64_init_hyper() is just hanging out there
   in the middle of nowhere.
2. I don't see why it's necessary to bother with the
   additional BT_XEN_HYPER_MODE flag?  It seems
   its usage could be replaced with XEN_HYPER_MODE(),
   which you also use in lkcd_x86_trace.c.

But on the whole, the patch looks pretty clean,
and as long as the "binary-file-type-determination" can
be resolved cleanly, it looking pretty good.

Thanks,
  Dave