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