[PATCH v2 0/6] crash utility - add ARM support

[PATCH v2 0/6] crash utility - add ARM support

[Dave Anderson]

----- "Mika Westerberg" <ext-mika.1.westerberg@nokia.com> wrote:

> Hi Dave,
> 
> This series brings ARM support for the crash utility. This is the result of
> collaboration work with Nokia and SonyEricsson. Basically we combined our
> versions of the code. Previous version of the patches can be found here:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-June/019188.html
> 
> We tried to keep any ARM specific changes isolated with #ifdefs and similar so
> that it should not cause any problems with other archs.
> 
> In this series:
> 	o crash can be build as native ARM binary or "cross" version running on
> 	  x86 host (make target=ARM to build "cross" version)
> 	o supports kdump, diskdump and /dev/mem (live system)
> 	o stack unwinding with both framepointers and ARM unwind tables
> 	o most of the arch specific code is implemented
> 
> The patches apply on top of crash 5.0.6 sources.

Let me first thank you guys for making the integration of this patch-set
so simple, and for making the changes so non-intrusive.

I did make a few minor changes/additions:

In the spirit of avoiding "#ifdef <arch>" usage if at all possible,
I renamed kdump_phys_base() to arm_kdump_phys_base() and removed
the #ifdef ARM around it.  I also removed the #ifdef ARM around the
new entries in the offset_table and size_table, as there are already
several arch-specific entries in there already.  And I added those
new offset_table and size_table entries to the dump_offset_table()
output so that their values can be seen with "help -o".

For minimal documentation, I added arm references to the README file.

For building with the src.rpm, I added "arm" to the ExclusiveArch list
in the crash.spec file.

I fixed these warnings generated by "make warn":

  arm.c: In function ?arm_dump_backtrace_entry?:
  arm.c:1160: warning: format ?%d? expects type ?int?, but argument 6 has type ?ulong?
  arm.c:1166: warning: format ?%d? expects type ?int?, but argument 7 has type ?ulong?
  arm.c: In function ?arm_dump_irq?:
  arm.c:1424: warning: suggest parentheses around comparison in operand of ?&?
  arm.c:1490: warning: too many arguments for format
  arm.c:1357: warning: unused variable ?tmp2?
  arm.c: In function ?arm_parse_cmdline_args?:
  arm.c:409: warning: ?value? may be used uninitialized in this function

I modified arm_init() to capture any attempt to run an x86 binary built
for ARM on a live x86 or x86_64 system to display a fatal error message
indicating: "crash: compiled for the ARM architecture".  As it was, it
would fail with a nebulous "cannot resolve _stext" error.

The only other suggestion I can make is to put something in either the
top-level Makefile or in configure.c to catch/prevent a subsequent "make" command
being entered after having first done the initial build with "make target=ARM".
I found myself doing that constantly.  Or vice-versa, for that matter.
And if you really want to make the "other type" of binary, then there
should be a message that kills the build attempt, and indicates that you'd
have to do a "make clean" as well as removing the gdb subdirectory tree
entirely.  But that all can wait until after this first patch-set is released.

So -- with the minor changes above -- consider it queued for the next release.

And thanks again for making it so easy...

Dave

> 
> Best regards,
> MW
> 
> Jan Karlsson (1):
>   crash: update IRQ flags
> 
> Mika Westerberg (5):
>   configure/Makefile: add support for ARM targets
>   crash: add support for ARM kernel image
>   crash/diskdump: add ARM support
>   crash/kdump: add ARM support
>   crash: add ARM crashdump support
> 
>  Makefile            |   30 +-
>  arm.c               | 1741
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  configure.c         |   32 +-
>  defs.h              |  245 +++++++-
>  diskdump.c          |   42 ++-
>  kernel.c            |    3 +-
>  lkcd_vmdump_v2_v3.h |    4 +-
>  netdump.c           |  115 ++++
>  symbols.c           |   15 +-
>  unwind_arm.c        |  697 +++++++++++++++++++++
>  10 files changed, 2902 insertions(+), 22 deletions(-)
>  create mode 100644 arm.c
>  create mode 100644 unwind_arm.c

[PATCH v2 0/6] crash utility - add ARM support

[Mika Westerberg]

On Thu, Aug 26, 2010 at 10:49:31PM +0200, ext Dave Anderson wrote:
> 
> I fixed these warnings generated by "make warn":
> 
>   arm.c: In function ?arm_dump_backtrace_entry?:
>   arm.c:1160: warning: format ?%d? expects type ?int?, but argument 6 has type ?ulong?
>   arm.c:1166: warning: format ?%d? expects type ?int?, but argument 7 has type ?ulong?
>   arm.c: In function ?arm_dump_irq?:
>   arm.c:1424: warning: suggest parentheses around comparison in operand of ?&?
>   arm.c:1490: warning: too many arguments for format
>   arm.c:1357: warning: unused variable ?tmp2?
>   arm.c: In function ?arm_parse_cmdline_args?:
>   arm.c:409: warning: ?value? may be used uninitialized in this function

Argh!

Looks like I forgot to compile the latest patch set with warnings enabled :(
Thanks for fixing them and sorry for any inconvenience.

> I modified arm_init() to capture any attempt to run an x86 binary built
> for ARM on a live x86 or x86_64 system to display a fatal error message
> indicating: "crash: compiled for the ARM architecture".  As it was, it
> would fail with a nebulous "cannot resolve _stext" error.

Yeah, it is better to have some useful error message instead.

> The only other suggestion I can make is to put something in either the
> top-level Makefile or in configure.c to catch/prevent a subsequent "make" command
> being entered after having first done the initial build with "make target=ARM".
> I found myself doing that constantly.  Or vice-versa, for that matter.
> And if you really want to make the "other type" of binary, then there
> should be a message that kills the build attempt, and indicates that you'd
> have to do a "make clean" as well as removing the gdb subdirectory tree
> entirely.  But that all can wait until after this first patch-set is released.

OK.

> So -- with the minor changes above -- consider it queued for the next release.

Thanks!

Regards,
MW

[PATCH v2 0/6] crash utility - add ARM support

[Mika Westerberg]

Hi Dave,

This series brings ARM support for the crash utility. This is the result of
collaboration work with Nokia and SonyEricsson. Basically we combined our
versions of the code. Previous version of the patches can be found here:

	http://lists.infradead.org/pipermail/linux-arm-kernel/2010-June/019188.html

We tried to keep any ARM specific changes isolated with #ifdefs and similar so
that it should not cause any problems with other archs.

In this series:
	o crash can be build as native ARM binary or "cross" version running on
	  x86 host (make target=ARM to build "cross" version)
	o supports kdump, diskdump and /dev/mem (live system)
	o stack unwinding with both framepointers and ARM unwind tables
	o most of the arch specific code is implemented

The patches apply on top of crash 5.0.6 sources.

Best regards,
MW

Jan Karlsson (1):
  crash: update IRQ flags

Mika Westerberg (5):
  configure/Makefile: add support for ARM targets
  crash: add support for ARM kernel image
  crash/diskdump: add ARM support
  crash/kdump: add ARM support
  crash: add ARM crashdump support

 Makefile            |   30 +-
 arm.c               | 1741 +++++++++++++++++++++++++++++++++++++++++++++++++++
 configure.c         |   32 +-
 defs.h              |  245 +++++++-
 diskdump.c          |   42 ++-
 kernel.c            |    3 +-
 lkcd_vmdump_v2_v3.h |    4 +-
 netdump.c           |  115 ++++
 symbols.c           |   15 +-
 unwind_arm.c        |  697 +++++++++++++++++++++
 10 files changed, 2902 insertions(+), 22 deletions(-)
 create mode 100644 arm.c
 create mode 100644 unwind_arm.c