[Qemu-devel] [RFC 00/19] Debug output revamp

[Andreas Färber]

Hello,

Based on a suggestion from Igor Mitsyanko, this series aims to expose
debug output to compile-testing, so that it doesn't constantly break.
The basic requirement is to move format strings and arguments out of
#ifdef sections or no-op macros, so that the C compiler sees them.

For one thing this is to catch existing, e.g., %x -> HWADDR_PRIx breakages,
so that debug output still can be enabled when needed.

For another it is to assist in catching refactored, e.g., CPU_COMMON fields,
to not break things further in new patches.

When I passed on Igor's idea to Fred, Anthony rejected it due to inconsistency.
Thus the solution is not to put our heads in the sand but to do the legwork of
harmonizing things in one big effort if incremental improvements are undesired.
This RFC series therefore updates all targets (which happens to be where the
bulk of my CPUState refactorings happens), leaving devices for a v2.

Anthony brought up the issue that an if { ... } inside the macro might cause
unpredictable effects. In theory our Coding Style forbids affected uses such as
if (foo) DPRINTF(bar) else baz; but in practice not all code is converted,
so use G_STMT_START ... G_STMT_END to shield against this. That allows to
redefine them to no-op if so desired one day.
Note that checkpatch.pl does not seem to understand that this effectively is
do { ... } while (0).

In some cases I took the liberty of simplifying D(qemu_log(...)) to D_LOG(...)
or D(printf(...)) to DPRINTF(...), as D(x) might be used for variable
declarations (in my case CPUState, prompting this series), unsuited for an
if (foo) { ... } block.

target-mips was already using this pattern.
target-cris was partially using it.

target-{arm,xtensa} didn't seem to have any such debug output.

target-m68k/translate.c has a DEBUG_DISPATCH further down; it indirects the
function call and only logs the name, so leave as is.

target-microblaze/helper.c:do_interrupt() has some commented-out debug code,
but it accesses MB internals only, so leaving to maintainers.

target-ppc/mmu_helper.c has failing debug code for DUMP_PAGE_TABLES involving
non-existing sdr and mask variables that I couldn't find a quick fix for.
Alex, please check!

target-sh4/translate.c has a commented-out DEBUG_DISAS macro that conflicts
with the one defined in include/exec/exec-all.h, therefore untouched.
Aurélien, should we rename it or drop the commented-out definition?

target-sparc/int{32,64}_helper.c have a DEBUG_PCALL that seemed too heavyweight;
same for some statistics or dump options in other targets, unlikely to be
affected by tree-wide refactorings and thus the maintainer's problem. ;)

Patches 1-7 are bug fixes found by enabling today's debug output.
Patches 8-9 are preparatory Coding Style cleanups.
Patches 10-19 do the actual macro refactoring per target-*/.

Available from:
git://github.com/afaerber/qemu-cpu.git dprintf.v1
https://github.com/afaerber/qemu-cpu/commits/dprintf.v1

My immediate quest is finding a solution for target-cris/helper.c, where I ran
into an issue moving the interrupt_request field to CPUState, since there is a
lonely usage inside a D_LOG() (plus my changes causing checkpatch.pl warnings).
https://github.com/afaerber/qemu-cpu/commit/cb9d472dda6efcd578a86df9c1b43510a0837e0d#diff-30

Regards,
Andreas

>From individual patches:
* cris __VA__ARGS__ build fix was applied already.
* cris build fix and one cleanup sent out already are resent for testability.

Cc: Anthony Liguori <address@hidden>
Cc: Blue Swirl <address@hidden>
Cc: Igor Mitsyanko <address@hidden>
Cc: Fréderic Konrad <address@hidden>

Cc: Edgar E. Iglesias <address@hidden> (cris)
Cc: Alexander Graf <address@hidden> (ppc)
Cc: Aurélien Jarno <address@hidden> (sh4)

Cc: address@hidden
Cc: Richard Henderson <address@hidden> (i386, s390x)
Cc: Michael Walle <address@hidden> (lm32)
Cc: Peter Crosthwaite <address@hidden> (mblaze)
Cc: Jia Liu <address@hidden> (or32)
Cc: Guan Xuetao <address@hidden> (uc32)

Andreas Färber (19):
  target-cris: Build fix for debug output
  target-ppc: Fix target_ulong vs. hwaddr format mismatches
  target-ppc: Fix unused variable warning for FLUSH_ALL_TLBS
  target-ppc: Fix build for PPC_DEBUG_DISAS
  target-s390x: Fix debug output
  target-s390x: Fix debug output (continued)
  target-sparc: Fix debug output for DEBUG_MMU
  target-cris/helper.c: Update Coding Style
  target-cris/mmu.c: Update Coding Style of cris_mmu_translate_page()
  target-alpha: Refactor debug output macros
  target-cris: Refactor debug output macros
  target-i386: Refactor debug output macros
  target-lm32: Refactor debug output macros
  target-microblaze: Refactor debug output macros
  target-openrisc: Refactor debug output macros
  target-ppc: Refactor debug output macros
  target-s390x: Refactor debug output macros
  target-sparc: Refactor debug output macros
  target-unicore32: Refactor debug output macros

 target-alpha/translate.c      |   12 +-
 target-cris/helper.c          |  410 ++++++++++++++++++++---------------------
 target-cris/mmu.c             |  334 +++++++++++++++++----------------
 target-cris/op_helper.c       |   17 +-
 target-cris/translate.c       |   18 +-
 target-i386/helper.c          |   33 ++--
 target-i386/kvm.c             |   16 +-
 target-i386/seg_helper.c      |   20 +-
 target-lm32/translate.c       |   10 +-
 target-microblaze/helper.c    |   25 ++-
 target-microblaze/mmu.c       |   28 +--
 target-microblaze/op_helper.c |    2 -
 target-microblaze/translate.c |   11 +-
 target-openrisc/translate.c   |   20 +-
 target-ppc/excp_helper.c      |   22 +--
 target-ppc/kvm.c              |    9 +-
 target-ppc/mem_helper.c       |    2 -
 target-ppc/mmu_helper.c       |   74 ++++----
 target-ppc/translate.c        |   14 +-
 target-ppc/translate_init.c   |   10 +-
 target-s390x/cc_helper.c      |   13 +-
 target-s390x/fpu_helper.c     |   13 +-
 target-s390x/helper.c         |   48 +++--
 target-s390x/int_helper.c     |   13 +-
 target-s390x/kvm.c            |    9 +-
 target-s390x/mem_helper.c     |   13 +-
 target-s390x/misc_helper.c    |   13 +-
 target-s390x/translate.c      |   18 +-
 target-sparc/cpu.c            |   30 ++-
 target-sparc/ldst_helper.c    |  173 ++++++++---------
 target-unicore32/helper.c     |   12 +-
 target-unicore32/softmmu.c    |   12 +-
 32 Dateien geändert, 719 Zeilen hinzugefügt(+), 735 Zeilen entfernt(-)

-- 
1.7.10.4