[PATCH v2 02/11] hw//sh4: Use qemu_log instead of fprintf to stderr

BALATON Zoltan posted 11 patches 4 years, 3 months ago
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Paolo Bonzini <pbonzini@redhat.com>, Magnus Damm <magnus.damm@gmail.com>
There is a newer version of this series
[PATCH v2 02/11] hw//sh4: Use qemu_log instead of fprintf to stderr
Posted by BALATON Zoltan 4 years, 3 months ago
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/char/sh_serial.c |  7 ++++---
 hw/sh4/sh7750.c     | 13 ++++++-------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
index 1b1e6a6a04..c4231975c7 100644
--- a/hw/char/sh_serial.c
+++ b/hw/char/sh_serial.c
@@ -30,6 +30,7 @@
 #include "hw/sh4/sh.h"
 #include "chardev/char-fe.h"
 #include "qapi/error.h"
+#include "qemu/log.h"
 #include "qemu/timer.h"
 
 //#define DEBUG_SERIAL
@@ -200,8 +201,8 @@ static void sh_serial_write(void *opaque, hwaddr offs,
         }
     }
 
-    fprintf(stderr, "sh_serial: unsupported write to 0x%02"
-            HWADDR_PRIx "\n", offs);
+    qemu_log_mask(LOG_UNIMP, "sh_serial: unsupported write to 0x%02"
+                  HWADDR_PRIx "\n", offs);
     abort();
 }
 
@@ -307,7 +308,7 @@ static uint64_t sh_serial_read(void *opaque, hwaddr offs,
 #endif
 
     if (ret & ~((1 << 16) - 1)) {
-        fprintf(stderr, "sh_serial: unsupported read from 0x%02"
+        qemu_log_mask(LOG_UNIMP, "sh_serial: unsupported read from 0x%02"
                 HWADDR_PRIx "\n", offs);
         abort();
     }
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index ca7e261aba..f2f251f165 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "hw/irq.h"
 #include "hw/sh4/sh.h"
 #include "sysemu/sysemu.h"
@@ -205,13 +206,13 @@ static void portb_changed(SH7750State *s, uint16_t prev)
 
 static void error_access(const char *kind, hwaddr addr)
 {
-    fprintf(stderr, "%s to %s (0x" TARGET_FMT_plx ") not supported\n",
-            kind, regname(addr), addr);
+    qemu_log_mask(LOG_GUEST_ERROR, "%s to %s (0x" TARGET_FMT_plx
+                  ") not supported\n", kind, regname(addr), addr);
 }
 
 static void ignore_access(const char *kind, hwaddr addr)
 {
-    fprintf(stderr, "%s to %s (0x" TARGET_FMT_plx ") ignored\n",
+    qemu_log_mask(LOG_UNIMP, "%s to %s (0x" TARGET_FMT_plx ") ignored\n",
             kind, regname(addr), addr);
 }
 
@@ -241,8 +242,7 @@ static uint32_t sh7750_mem_readw(void *opaque, hwaddr addr)
     case SH7750_PCR_A7:
         return s->pcr;
     case SH7750_RFCR_A7:
-        fprintf(stderr,
-                "Read access to refresh count register, incrementing\n");
+        /* Read access to refresh count register, incrementing */
         return s->rfcr++;
     case SH7750_PDTRA_A7:
         return porta_lines(s);
@@ -363,13 +363,12 @@ static void sh7750_mem_writew(void *opaque, hwaddr addr,
         portb_changed(s, temp);
         return;
     case SH7750_RFCR_A7:
-        fprintf(stderr, "Write access to refresh count register\n");
         s->rfcr = mem_value;
         return;
     case SH7750_GPIOIC_A7:
         s->gpioic = mem_value;
         if (mem_value != 0) {
-            fprintf(stderr, "I/O interrupts not implemented\n");
+            qemu_log_mask(LOG_UNIMP, "I/O interrupts not implemented\n");
             abort();
         }
         return;
-- 
2.21.4


Re: [PATCH v2 02/11] hw//sh4: Use qemu_log instead of fprintf to stderr
Posted by Philippe Mathieu-Daudé 4 years, 3 months ago
On 10/27/21 15:46, BALATON Zoltan wrote:
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  hw/char/sh_serial.c |  7 ++++---
>  hw/sh4/sh7750.c     | 13 ++++++-------
>  2 files changed, 10 insertions(+), 10 deletions(-)

>      case SH7750_GPIOIC_A7:
>          s->gpioic = mem_value;
>          if (mem_value != 0) {
> -            fprintf(stderr, "I/O interrupts not implemented\n");
> +            qemu_log_mask(LOG_UNIMP, "I/O interrupts not implemented\n");
>              abort();

This change is annoying. Before you'd get an error message and the abort
signal. Now if you don't use "-d unimp" you get an abort without
explanation. Can we use error_report() instead?

>          }
>          return;
> 


Re: [PATCH v2 02/11] hw//sh4: Use qemu_log instead of fprintf to stderr
Posted by BALATON Zoltan 4 years, 3 months ago
On Wed, 27 Oct 2021, Philippe Mathieu-Daudé wrote:
> On 10/27/21 15:46, BALATON Zoltan wrote:
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  hw/char/sh_serial.c |  7 ++++---
>>  hw/sh4/sh7750.c     | 13 ++++++-------
>>  2 files changed, 10 insertions(+), 10 deletions(-)
>
>>      case SH7750_GPIOIC_A7:
>>          s->gpioic = mem_value;
>>          if (mem_value != 0) {
>> -            fprintf(stderr, "I/O interrupts not implemented\n");
>> +            qemu_log_mask(LOG_UNIMP, "I/O interrupts not implemented\n");
>>              abort();
>
> This change is annoying. Before you'd get an error message and the abort
> signal. Now if you don't use "-d unimp" you get an abort without
> explanation. Can we use error_report() instead?

I plan to remove the aborts later. What's annoying is that the guest can 
crash QEMU by doing something invalid. Other devices don't do this just 
report the error with LOG_UNIMP and continue even if it will not work so I 
follow that convention but did not yet cleaned up the aborts.

Regards,
BALATON Zoltan