linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: anderson@redhat.com (Dave Anderson)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 0/6] crash utility - add ARM support
Date: Thu, 26 Aug 2010 16:49:31 -0400 (EDT)	[thread overview]
Message-ID: <1390998169.686181282855771133.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> (raw)
In-Reply-To: <2024587576.686081282855712906.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>


----- "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

       reply	other threads:[~2010-08-26 20:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <2024587576.686081282855712906.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>
2010-08-26 20:49 ` Dave Anderson [this message]
2010-08-27  5:59   ` [PATCH v2 0/6] crash utility - add ARM support Mika Westerberg
2010-08-26 12:02 Mika Westerberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1390998169.686181282855771133.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com \
    --to=anderson@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).