qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dave@treblig.org>
To: BALATON Zoltan <balaton@eik.bme.hu>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>,
	qemu-devel@nongnu.org, "Artyom Tarasenko" <atar4qemu@gmail.com>,
	"Chris Wulff" <crwulff@gmail.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	"Marek Vasut" <marex@denx.de>,
	"Max Filippov" <jcmvbkbc@gmail.com>,
	"Jiaxun Yang" <jiaxun.yang@flygoat.com>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel Henrique Barboza" <danielhb413@gmail.com>,
	"Yoshinori Sato" <ysato@users.sourceforge.jp>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	qemu-ppc@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>
Subject: Re: [PATCH-for-9.1 05/21] target/m68k: Replace qemu_printf() by monitor_printf() in monitor
Date: Thu, 28 Mar 2024 21:59:16 +0000	[thread overview]
Message-ID: <ZgXoNN43Izq4SQhd@gallifrey> (raw)
In-Reply-To: <2da27069-e767-2bb7-f108-473294dcbef6@eik.bme.hu>

* BALATON Zoltan (balaton@eik.bme.hu) wrote:
> On Sun, 24 Mar 2024, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> > > Replace qemu_printf() by monitor_printf() / monitor_puts() in monitor.
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > >  target/m68k/cpu.h     |   2 +-
> > >  target/m68k/helper.c  | 126 +++++++++++++++++++++---------------------
> > >  target/m68k/monitor.c |   4 +-
> > >  3 files changed, 67 insertions(+), 65 deletions(-)
> > > 
> > > diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
> > > index 346427e144..4e4307956d 100644
> > > --- a/target/m68k/cpu.h
> > > +++ b/target/m68k/cpu.h
> > > @@ -620,6 +620,6 @@ static inline void cpu_get_tb_cpu_state(CPUM68KState *env, vaddr *pc,
> > >      }
> > >  }
> > > 
> > > -void dump_mmu(CPUM68KState *env);
> > > +void dump_mmu(Monitor *mon, CPUM68KState *env);
> > > 
> > >  #endif
> > > diff --git a/target/m68k/helper.c b/target/m68k/helper.c
> > > index 1a475f082a..310e26dfa1 100644
> > > --- a/target/m68k/helper.c
> > > +++ b/target/m68k/helper.c
> > > @@ -25,7 +25,7 @@
> > >  #include "exec/helper-proto.h"
> > >  #include "gdbstub/helpers.h"
> > >  #include "fpu/softfloat.h"
> > > -#include "qemu/qemu-print.h"
> > > +#include "monitor/monitor.h"
> > > 
> > >  #define SIGNBIT (1u << 31)
> > > 
> > > @@ -455,28 +455,30 @@ void m68k_switch_sp(CPUM68KState *env)
> > >  #if !defined(CONFIG_USER_ONLY)
> > >  /* MMU: 68040 only */
> > > 
> > > -static void print_address_zone(uint32_t logical, uint32_t physical,
> > > +static void print_address_zone(Monitor *mon,
> > > +                               uint32_t logical, uint32_t physical,
> > >                                 uint32_t size, int attr)
> > >  {
> > > -    qemu_printf("%08x - %08x -> %08x - %08x %c ",
> > > -                logical, logical + size - 1,
> > > -                physical, physical + size - 1,
> > > -                attr & 4 ? 'W' : '-');
> > > +    monitor_printf(mon, "%08x - %08x -> %08x - %08x %c ",
> > > +                   logical, logical + size - 1,
> > > +                   physical, physical + size - 1,
> > > +                   attr & 4 ? 'W' : '-');
> > >      size >>= 10;
> > >      if (size < 1024) {
> > > -        qemu_printf("(%d KiB)\n", size);
> > > +        monitor_printf(mon, "(%d KiB)\n", size);
> > >      } else {
> > >          size >>= 10;
> > >          if (size < 1024) {
> > > -            qemu_printf("(%d MiB)\n", size);
> > > +            monitor_printf(mon, "(%d MiB)\n", size);
> > >          } else {
> > >              size >>= 10;
> > > -            qemu_printf("(%d GiB)\n", size);
> > > +            monitor_printf(mon, "(%d GiB)\n", size);
> > >          }
> > >      }
> > >  }
> > > 
> > > -static void dump_address_map(CPUM68KState *env, uint32_t root_pointer)
> > > +static void dump_address_map(Monitor *mon, CPUM68KState *env,
> > > +                             uint32_t root_pointer)
> > >  {
> > >      int i, j, k;
> > >      int tic_size, tic_shift;
> > > @@ -545,7 +547,7 @@ static void dump_address_map(CPUM68KState *env, uint32_t root_pointer)
> > >                      if (first_logical != 0xffffffff) {
> > >                          size = last_logical + (1 << tic_shift) -
> > >                                 first_logical;
> > > -                        print_address_zone(first_logical,
> > > +                        print_address_zone(mon, first_logical,
> > >                                             first_physical, size, last_attr);
> > >                      }
> > >                      first_logical = logical;
> > > @@ -556,125 +558,125 @@ static void dump_address_map(CPUM68KState *env, uint32_t root_pointer)
> > >      }
> > >      if (first_logical != logical || (attr & 4) != (last_attr & 4)) {
> > >          size = logical + (1 << tic_shift) - first_logical;
> > > -        print_address_zone(first_logical, first_physical, size, last_attr);
> > > +        print_address_zone(mon, first_logical, first_physical, size, last_attr);
> > >      }
> > >  }
> > > 
> > >  #define DUMP_CACHEFLAGS(a) \
> > >      switch (a & M68K_DESC_CACHEMODE) { \
> > >      case M68K_DESC_CM_WRTHRU: /* cacheable, write-through */ \
> > > -        qemu_printf("T"); \
> > > +        monitor_puts(mon, "T"); \
> > >          break; \
> > >      case M68K_DESC_CM_COPYBK: /* cacheable, copyback */ \
> > > -        qemu_printf("C"); \
> > > +        monitor_puts(mon, "C"); \
> > >          break; \
> > >      case M68K_DESC_CM_SERIAL: /* noncachable, serialized */ \
> > > -        qemu_printf("S"); \
> > > +        monitor_puts(mon, "S"); \
> > >          break; \
> > >      case M68K_DESC_CM_NCACHE: /* noncachable */ \
> > > -        qemu_printf("N"); \
> > > +        monitor_puts(mon, "N"); \
> > >          break; \
> > >      }
> > > 
> > > -static void dump_ttr(uint32_t ttr)
> > > +static void dump_ttr(Monitor *mon, uint32_t ttr)
> > >  {
> > >      if ((ttr & M68K_TTR_ENABLED) == 0) {
> > > -        qemu_printf("disabled\n");
> > > +        monitor_puts(mon, "disabled\n");
> > >          return;
> > >      }
> > > -    qemu_printf("Base: 0x%08x Mask: 0x%08x Control: ",
> > > -                ttr & M68K_TTR_ADDR_BASE,
> > > -                (ttr & M68K_TTR_ADDR_MASK) << M68K_TTR_ADDR_MASK_SHIFT);
> > > +    monitor_printf(mon, "Base: 0x%08x Mask: 0x%08x Control: ",
> > > +                   ttr & M68K_TTR_ADDR_BASE,
> > > +                   (ttr & M68K_TTR_ADDR_MASK) << M68K_TTR_ADDR_MASK_SHIFT);
> > >      switch (ttr & M68K_TTR_SFIELD) {
> > >      case M68K_TTR_SFIELD_USER:
> > > -        qemu_printf("U");
> > > +        monitor_puts(mon, "U");
> > >          break;
> > >      case M68K_TTR_SFIELD_SUPER:
> > > -        qemu_printf("S");
> > > +        monitor_puts(mon, "S");
> > >          break;
> > >      default:
> > > -        qemu_printf("*");
> > > +        monitor_puts(mon, "*");
> > >          break;
> > >      }
> > >      DUMP_CACHEFLAGS(ttr);
> > >      if (ttr & M68K_DESC_WRITEPROT) {
> > > -        qemu_printf("R");
> > > +        monitor_puts(mon, "R");
> > >      } else {
> > > -        qemu_printf("W");
> > > +        monitor_puts(mon, "W");
> > >      }
> > > -    qemu_printf(" U: %d\n", (ttr & M68K_DESC_USERATTR) >>
> > > +    monitor_printf(mon, " U: %d\n", (ttr & M68K_DESC_USERATTR) >>
> > >                                 M68K_DESC_USERATTR_SHIFT);
> > >  }
> > > 
> > > -void dump_mmu(CPUM68KState *env)
> > > +void dump_mmu(Monitor *mon, CPUM68KState *env)
> > >  {
> > >      if ((env->mmu.tcr & M68K_TCR_ENABLED) == 0) {
> > > -        qemu_printf("Translation disabled\n");
> > > +        monitor_puts(mon, "Translation disabled\n");
> > >          return;
> > >      }
> > > -    qemu_printf("Page Size: ");
> > > +    monitor_puts(mon, "Page Size: ");
> > >      if (env->mmu.tcr & M68K_TCR_PAGE_8K) {
> > > -        qemu_printf("8kB\n");
> > > +        monitor_puts(mon, "8kB\n");
> > >      } else {
> > > -        qemu_printf("4kB\n");
> > > +        monitor_puts(mon, "4kB\n");
> > >      }
> > > 
> > > -    qemu_printf("MMUSR: ");
> > > +    monitor_puts(mon, "MMUSR: ");
> > >      if (env->mmu.mmusr & M68K_MMU_B_040) {
> > > -        qemu_printf("BUS ERROR\n");
> > > +        monitor_puts(mon, "BUS ERROR\n");
> > >      } else {
> > > -        qemu_printf("Phy=%08x Flags: ", env->mmu.mmusr & 0xfffff000);
> > > +        monitor_printf(mon, "Phy=%08x Flags: ", env->mmu.mmusr & 0xfffff000);
> > >          /* flags found on the page descriptor */
> > >          if (env->mmu.mmusr & M68K_MMU_G_040) {
> > > -            qemu_printf("G"); /* Global */
> > > +            monitor_puts(mon, "G"); /* Global */
> > >          } else {
> > > -            qemu_printf(".");
> > > +            monitor_puts(mon, ".");
> > >          }
> > >          if (env->mmu.mmusr & M68K_MMU_S_040) {
> > > -            qemu_printf("S"); /* Supervisor */
> > > +            monitor_puts(mon, "S"); /* Supervisor */
> > >          } else {
> > > -            qemu_printf(".");
> > > +            monitor_puts(mon, ".");
> > >          }
> > >          if (env->mmu.mmusr & M68K_MMU_M_040) {
> > > -            qemu_printf("M"); /* Modified */
> > > +            monitor_puts(mon, "M"); /* Modified */
> > >          } else {
> > > -            qemu_printf(".");
> > > +            monitor_puts(mon, ".");
> > >          }
> > >          if (env->mmu.mmusr & M68K_MMU_WP_040) {
> > > -            qemu_printf("W"); /* Write protect */
> > > +            monitor_puts(mon, "W"); /* Write protect */
> > >          } else {
> > > -            qemu_printf(".");
> > > +            monitor_puts(mon, ".");
> > >          }
> > >          if (env->mmu.mmusr & M68K_MMU_T_040) {
> > > -            qemu_printf("T"); /* Transparent */
> > > +            monitor_puts(mon, "T"); /* Transparent */
> > >          } else {
> > > -            qemu_printf(".");
> > > +            monitor_puts(mon, ".");
> > >          }
> > >          if (env->mmu.mmusr & M68K_MMU_R_040) {
> > > -            qemu_printf("R"); /* Resident */
> > > +            monitor_puts(mon, "R"); /* Resident */
> > >          } else {
> > > -            qemu_printf(".");
> > > +            monitor_puts(mon, ".");
> > >          }
> > > -        qemu_printf(" Cache: ");
> > > +        monitor_puts(mon, " Cache: ");
> > >          DUMP_CACHEFLAGS(env->mmu.mmusr);
> > > -        qemu_printf(" U: %d\n", (env->mmu.mmusr >> 8) & 3);
> > > -        qemu_printf("\n");
> > > +        monitor_printf(mon, " U: %d\n", (env->mmu.mmusr >> 8) & 3);
> > > +        monitor_puts(mon, "\n");
> > 
> > That one is a little odd isn't it; still, generally
> 
> Doesn't puts append a newline? Then this would add an extra empty line.

As rth said, apparently not.
But what made me more curious in this case is why not just flatten it
down so that the printf has a second \n rather than needing the second
call to puts.

Dave

> Regards,
> BALATON Zoltan
> 
> > Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> > 
> > >      }
> > > 
> > > -    qemu_printf("ITTR0: ");
> > > -    dump_ttr(env->mmu.ttr[M68K_ITTR0]);
> > > -    qemu_printf("ITTR1: ");
> > > -    dump_ttr(env->mmu.ttr[M68K_ITTR1]);
> > > -    qemu_printf("DTTR0: ");
> > > -    dump_ttr(env->mmu.ttr[M68K_DTTR0]);
> > > -    qemu_printf("DTTR1: ");
> > > -    dump_ttr(env->mmu.ttr[M68K_DTTR1]);
> > > +    monitor_puts(mon, "ITTR0: ");
> > > +    dump_ttr(mon, env->mmu.ttr[M68K_ITTR0]);
> > > +    monitor_puts(mon, "ITTR1: ");
> > > +    dump_ttr(mon, env->mmu.ttr[M68K_ITTR1]);
> > > +    monitor_puts(mon, "DTTR0: ");
> > > +    dump_ttr(mon, env->mmu.ttr[M68K_DTTR0]);
> > > +    monitor_puts(mon, "DTTR1: ");
> > > +    dump_ttr(mon, env->mmu.ttr[M68K_DTTR1]);
> > > 
> > > -    qemu_printf("SRP: 0x%08x\n", env->mmu.srp);
> > > -    dump_address_map(env, env->mmu.srp);
> > > +    monitor_printf(mon, "SRP: 0x%08x\n", env->mmu.srp);
> > > +    dump_address_map(mon, env, env->mmu.srp);
> > > 
> > > -    qemu_printf("URP: 0x%08x\n", env->mmu.urp);
> > > -    dump_address_map(env, env->mmu.urp);
> > > +    monitor_printf(mon, "URP: 0x%08x\n", env->mmu.urp);
> > > +    dump_address_map(mon, env, env->mmu.urp);
> > >  }
> > > 
> > >  static int check_TTR(uint32_t ttr, int *prot, target_ulong addr,
> > > diff --git a/target/m68k/monitor.c b/target/m68k/monitor.c
> > > index 2bdf6acae0..623c6ab635 100644
> > > --- a/target/m68k/monitor.c
> > > +++ b/target/m68k/monitor.c
> > > @@ -15,11 +15,11 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
> > >      CPUArchState *env1 = mon_get_cpu_env(mon);
> > > 
> > >      if (!env1) {
> > > -        monitor_printf(mon, "No CPU available\n");
> > > +        monitor_puts(mon, "No CPU available\n");
> > >          return;
> > >      }
> > > 
> > > -    dump_mmu(env1);
> > > +    dump_mmu(mon, env1);
> > >  }
> > > 
> > >  static const MonitorDef monitor_defs[] = {
> > > --
> > > 2.41.0
> > > 
> > 

-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/


  reply	other threads:[~2024-03-28 22:00 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-21 15:48 [PATCH-for-9.1 00/21] target/monitor: Cleanup around hmp_info_tlb() Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.0? 01/21] host/atomic128: Include missing 'qemu/atomic.h' header Philippe Mathieu-Daudé
2024-03-21 17:05   ` Richard Henderson
2024-04-23 13:54     ` Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 02/21] hw/core: Remove check on NEED_CPU_H in tcg-cpu-ops.h Philippe Mathieu-Daudé
2024-03-21 21:17   ` Richard Henderson
2024-03-21 15:48 ` [PATCH-for-9.1 03/21] target/i386: Move APIC related code to cpu-apic.c Philippe Mathieu-Daudé
2024-03-21 21:43   ` Richard Henderson
2024-04-24  8:26   ` Zhao Liu
2024-04-24 12:05     ` Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 04/21] target/i386: Extract x86_dump_mmu() from hmp_info_tlb() Philippe Mathieu-Daudé
2024-03-21 21:46   ` Richard Henderson
2024-03-21 15:48 ` [PATCH-for-9.1 05/21] target/m68k: Replace qemu_printf() by monitor_printf() in monitor Philippe Mathieu-Daudé
2024-03-21 21:49   ` Richard Henderson
2024-03-24 23:43   ` Dr. David Alan Gilbert
2024-03-25  0:38     ` BALATON Zoltan
2024-03-28 21:59       ` Dr. David Alan Gilbert [this message]
2024-03-28 22:29         ` BALATON Zoltan
2024-04-24  7:35   ` Markus Armbruster
2024-04-24  9:19     ` BALATON Zoltan
2024-04-24  9:22       ` BALATON Zoltan
2024-03-21 15:48 ` [PATCH-for-9.1 06/21] target/m68k: Have dump_ttr() take a @description argument Philippe Mathieu-Daudé
2024-03-21 21:49   ` Richard Henderson
2024-03-21 15:48 ` [PATCH-for-9.1 07/21] target/m68k: Move MMU monitor commands from helper.c to monitor.c Philippe Mathieu-Daudé
2024-03-21 21:50   ` Richard Henderson
2024-03-21 15:48 ` [PATCH-for-9.1 08/21] target/microblaze: Prefix MMU API with 'mb_' Philippe Mathieu-Daudé
2024-03-23 13:13   ` Edgar E. Iglesias
2024-03-21 15:48 ` [PATCH-for-9.1 09/21] target/mips: Prefix MMU API with 'mips_' Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 10/21] target/nios2: Prefix MMU API with 'nios2_' Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 11/21] target/nios2: Move monitor commands to monitor.c Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 12/21] target/nios2: Replace qemu_printf() by monitor_printf() in monitor Philippe Mathieu-Daudé
2024-04-24  7:38   ` Markus Armbruster
2024-03-21 15:48 ` [PATCH-for-9.1 13/21] target/ppc: " Philippe Mathieu-Daudé
2024-04-24  7:39   ` Markus Armbruster
2024-03-21 15:48 ` [PATCH-for-9.1 14/21] target/sh4: Extract sh4_dump_mmu() from hmp_info_tlb() Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.0? 15/21] target/sparc: Fix string format errors when DEBUG_MMU is defined Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 16/21] target/sparc: Replace qemu_printf() by monitor_printf() in monitor Philippe Mathieu-Daudé
2024-04-24  7:44   ` Markus Armbruster
2024-04-24 12:08     ` Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 17/21] target/xtensa: Prefix MMU API with 'xtensa_' Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 18/21] target/xtensa: Extract MMU API to new mmu.c/mmu.h files Philippe Mathieu-Daudé
2024-03-23 19:23   ` Max Filippov
2024-03-21 15:48 ` [PATCH-for-9.1 19/21] target/xtensa: Simplify dump_mpu() and dump_tlb() Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 20/21] target/xtensa: Move monitor commands to monitor.c Philippe Mathieu-Daudé
2024-03-21 15:48 ` [PATCH-for-9.1 21/21] target/xtensa: Replace qemu_printf() by monitor_printf() in monitor Philippe Mathieu-Daudé
2024-04-24  7:45   ` Markus Armbruster

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=ZgXoNN43Izq4SQhd@gallifrey \
    --to=dave@treblig.org \
    --cc=armbru@redhat.com \
    --cc=atar4qemu@gmail.com \
    --cc=balaton@eik.bme.hu \
    --cc=crwulff@gmail.com \
    --cc=danielhb413@gmail.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=jcmvbkbc@gmail.com \
    --cc=jiaxun.yang@flygoat.com \
    --cc=laurent@vivier.eu \
    --cc=marex@denx.de \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=npiggin@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=ysato@users.sourceforge.jp \
    /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).