[Qemu-devel] [PATCH 00/11] hw/arm: use qemu_log_mask instead of fprintf

[Qemu-devel] [PATCH 00/11] hw/arm: use qemu_log_mask instead of fprintf

[Philippe Mathieu-Daudé]

Hi,

This series is another collection of trivial patches from
different stagnating branches, OMAP and Stellaris. Time to
share before loosing them.

Regards,

Phil.

Philippe Mathieu-Daudé (11):
  hw/input/pckbd: Use qemu_log_mask(GUEST_ERROR) instead of fprintf
  hw/input/tsc2005: Use qemu_log_mask(GUEST_ERROR) instead of fprintf
  hw/dma/omap_dma: Use qemu_log_mask(UNIMP) instead of printf
  hw/dma/omap_dma: Use qemu_log_mask(GUEST_ERROR) instead of fprintf
  hw/ssi/omap_spi: Use qemu_log_mask(GUEST_ERROR) instead of fprintf
  hw/sd/omap_mmc: Use qemu_log_mask(UNIMP) instead of printf
  hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf
  hw/arm/omap: Use qemu_log_mask(GUEST_ERROR) instead of fprintf
  hw/arm/stellaris: Use qemu_log_mask(UNIMP) instead of fprintf
  hw/net/stellaris_enet: Use qemu_log_mask(GUEST_ERROR) instead of hw_error
  hw/net/smc91c111: Use qemu_log_mask(GUEST_ERROR) instead of hw_error

 include/hw/arm/omap.h   | 20 +++++++-----
 hw/arm/stellaris.c      |  2 +-
 hw/dma/omap_dma.c       | 72 ++++++++++++++++++++++++++---------------
 hw/i2c/omap_i2c.c       | 23 ++++++++-----
 hw/input/pckbd.c        |  4 ++-
 hw/input/tsc2005.c      | 14 +++++---
 hw/net/smc91c111.c      |  9 ++++--
 hw/net/stellaris_enet.c |  9 ++++--
 hw/sd/omap_mmc.c        | 13 +++++---
 hw/ssi/omap_spi.c       | 15 ++++++---
 10 files changed, 119 insertions(+), 62 deletions(-)

-- 
2.18.0.rc2

[Qemu-devel] [PATCH 01/11] hw/input/pckbd: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/input/pckbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index f33e3fc63d..1102d2a31b 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "hw/hw.h"
 #include "hw/isa/isa.h"
 #include "hw/i386/pc.h"
@@ -308,7 +309,8 @@ static void kbd_write_command(void *opaque, hwaddr addr,
         /* ignore that */
         break;
     default:
-        fprintf(stderr, "qemu: unsupported keyboard cmd=0x%02x\n", (int)val);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "qemu: unsupported keyboard cmd=0x%02lx\n", val);
         break;
     }
 }
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 01/11] hw/input/pckbd: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/input/pckbd.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index f33e3fc63d..1102d2a31b 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -22,6 +22,7 @@
>   * THE SOFTWARE.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "hw/hw.h"
>  #include "hw/isa/isa.h"
>  #include "hw/i386/pc.h"
> @@ -308,7 +309,8 @@ static void kbd_write_command(void *opaque, hwaddr addr,
>          /* ignore that */
>          break;
>      default:
> -        fprintf(stderr, "qemu: unsupported keyboard cmd=0x%02x\n", (int)val);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "qemu: unsupported keyboard cmd=0x%02lx\n", val);

Please remove the "qemu:" prefix while you're at it.

 Thomas

Re: [Qemu-devel] [PATCH 01/11] hw/input/pckbd: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Philippe Mathieu-Daudé]

On 06/22/2018 05:04 AM, Thomas Huth wrote:
> On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/input/pckbd.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
>> index f33e3fc63d..1102d2a31b 100644
>> --- a/hw/input/pckbd.c
>> +++ b/hw/input/pckbd.c
>> @@ -22,6 +22,7 @@
>>   * THE SOFTWARE.
>>   */
>>  #include "qemu/osdep.h"
>> +#include "qemu/log.h"
>>  #include "hw/hw.h"
>>  #include "hw/isa/isa.h"
>>  #include "hw/i386/pc.h"
>> @@ -308,7 +309,8 @@ static void kbd_write_command(void *opaque, hwaddr addr,
>>          /* ignore that */
>>          break;
>>      default:
>> -        fprintf(stderr, "qemu: unsupported keyboard cmd=0x%02x\n", (int)val);
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "qemu: unsupported keyboard cmd=0x%02lx\n", val);
> 
> Please remove the "qemu:" prefix while you're at it.

Sure.

[Qemu-devel] [PATCH 03/11] hw/dma/omap_dma: Use qemu_log_mask(UNIMP) instead of printf

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/dma/omap_dma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c
index abd18c67ea..ab3a1b0451 100644
--- a/hw/dma/omap_dma.c
+++ b/hw/dma/omap_dma.c
@@ -18,6 +18,7 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "qemu-common.h"
 #include "qemu/timer.h"
 #include "hw/arm/omap.h"
@@ -1439,8 +1440,9 @@ static int omap_dma_sys_read(struct omap_dma_s *s, int offset,
     case 0x480:	/* DMA_PCh0_SR */
     case 0x482:	/* DMA_PCh1_SR */
     case 0x4c0:	/* DMA_PChD_SR_0 */
-        printf("%s: Physical Channel Status Registers not implemented.\n",
-               __func__);
+        qemu_log_mask(LOG_UNIMP,
+                      "%s: Physical Channel Status Registers not implemented\n",
+                      __func__);
         *ret = 0xff;
         break;
 
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 03/11] hw/dma/omap_dma: Use qemu_log_mask(UNIMP) instead of printf

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/dma/omap_dma.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c
> index abd18c67ea..ab3a1b0451 100644
> --- a/hw/dma/omap_dma.c
> +++ b/hw/dma/omap_dma.c
> @@ -18,6 +18,7 @@
>   * with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "qemu-common.h"
>  #include "qemu/timer.h"
>  #include "hw/arm/omap.h"
> @@ -1439,8 +1440,9 @@ static int omap_dma_sys_read(struct omap_dma_s *s, int offset,
>      case 0x480:	/* DMA_PCh0_SR */
>      case 0x482:	/* DMA_PCh1_SR */
>      case 0x4c0:	/* DMA_PChD_SR_0 */
> -        printf("%s: Physical Channel Status Registers not implemented.\n",
> -               __func__);
> +        qemu_log_mask(LOG_UNIMP,
> +                      "%s: Physical Channel Status Registers not implemented\n",
> +                      __func__);
>          *ret = 0xff;
>          break;

Reviewed-by: Thomas Huth <thuth@redhat.com>

[Qemu-devel] [PATCH 04/11] hw/dma/omap_dma: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/dma/omap_dma.c | 66 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c
index ab3a1b0451..a4a786f546 100644
--- a/hw/dma/omap_dma.c
+++ b/hw/dma/omap_dma.c
@@ -879,15 +879,18 @@ static int omap_dma_ch_reg_write(struct omap_dma_s *s,
         ch->burst[0] = (value & 0x0180) >> 7;
         ch->pack[0] = (value & 0x0040) >> 6;
         ch->port[0] = (enum omap_dma_port) ((value & 0x003c) >> 2);
-        if (ch->port[0] >= __omap_dma_port_last)
-            printf("%s: invalid DMA port %i\n", __func__,
-                            ch->port[0]);
-        if (ch->port[1] >= __omap_dma_port_last)
-            printf("%s: invalid DMA port %i\n", __func__,
-                            ch->port[1]);
+        if (ch->port[0] >= __omap_dma_port_last) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid DMA port %i\n",
+                          __func__, ch->port[0]);
+        }
+        if (ch->port[1] >= __omap_dma_port_last) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid DMA port %i\n",
+                          __func__, ch->port[1]);
+        }
         ch->data_type = 1 << (value & 3);
         if ((value & 3) == 3) {
-            printf("%s: bad data_type for DMA channel\n", __func__);
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: bad data_type for DMA channel\n", __func__);
             ch->data_type >>= 1;
         }
         break;
@@ -1899,14 +1902,20 @@ static void omap_dma4_write(void *opaque, hwaddr addr,
         if (value & 2)						/* SOFTRESET */
             omap_dma_reset(s->dma);
         s->ocp = value & 0x3321;
-        if (((s->ocp >> 12) & 3) == 3)				/* MIDLEMODE */
-            fprintf(stderr, "%s: invalid DMA power mode\n", __func__);
+        if (((s->ocp >> 12) & 3) == 3) {
+            /* MIDLEMODE */
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid DMA power mode\n",
+                          __func__);
+        }
         return;
 
     case 0x78:	/* DMA4_GCR */
         s->gcr = value & 0x00ff00ff;
-	if ((value & 0xff) == 0x00)		/* MAX_CHANNEL_FIFO_DEPTH */
-            fprintf(stderr, "%s: wrong FIFO depth in GCR\n", __func__);
+        if ((value & 0xff) == 0x00) {
+            /* MAX_CHANNEL_FIFO_DEPTH */
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: wrong FIFO depth in GCR\n",
+                          __func__);
+        }
         return;
 
     case 0x80 ... 0xfff:
@@ -1935,9 +1944,11 @@ static void omap_dma4_write(void *opaque, hwaddr addr,
     case 0x00:	/* DMA4_CCR */
         ch->buf_disable = (value >> 25) & 1;
         ch->src_sync = (value >> 24) & 1;	/* XXX For CamDMA must be 1 */
-        if (ch->buf_disable && !ch->src_sync)
-            fprintf(stderr, "%s: Buffering disable is not allowed in "
-                            "destination synchronised mode\n", __func__);
+        if (ch->buf_disable && !ch->src_sync) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: Buffering disable is not allowed in "
+                          "destination synchronised mode\n", __func__);
+        }
         ch->prefetch = (value >> 23) & 1;
         ch->bs = (value >> 18) & 1;
         ch->transparent_copy = (value >> 17) & 1;
@@ -1947,9 +1958,11 @@ static void omap_dma4_write(void *opaque, hwaddr addr,
         ch->suspend = (value & 0x0100) >> 8;
         ch->priority = (value & 0x0040) >> 6;
         ch->fs = (value & 0x0020) >> 5;
-        if (ch->fs && ch->bs && ch->mode[0] && ch->mode[1])
-            fprintf(stderr, "%s: For a packet transfer at least one port "
-                            "must be constant-addressed\n", __func__);
+        if (ch->fs && ch->bs && ch->mode[0] && ch->mode[1]) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: For a packet transfer at least one port "
+                          "must be constant-addressed\n", __func__);
+        }
         ch->sync = (value & 0x001f) | ((value >> 14) & 0x0060);
         /* XXX must be 0x01 for CamDMA */
 
@@ -1978,9 +1991,11 @@ static void omap_dma4_write(void *opaque, hwaddr addr,
         ch->endian_lock[0] =(value >> 20) & 1;
         ch->endian[1] =(value >> 19) & 1;
         ch->endian_lock[1] =(value >> 18) & 1;
-        if (ch->endian[0] != ch->endian[1])
-            fprintf(stderr, "%s: DMA endianness conversion enable attempt\n",
-                            __func__);
+        if (ch->endian[0] != ch->endian[1]) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: DMA endianness conversion enable attempt\n",
+                          __func__);
+        }
         ch->write_mode = (value >> 16) & 3;
         ch->burst[1] = (value & 0xc000) >> 14;
         ch->pack[1] = (value & 0x2000) >> 13;
@@ -1988,12 +2003,15 @@ static void omap_dma4_write(void *opaque, hwaddr addr,
         ch->burst[0] = (value & 0x0180) >> 7;
         ch->pack[0] = (value & 0x0040) >> 6;
         ch->translate[0] = (value & 0x003c) >> 2;
-        if (ch->translate[0] | ch->translate[1])
-            fprintf(stderr, "%s: bad MReqAddressTranslate sideband signal\n",
-                            __func__);
+        if (ch->translate[0] | ch->translate[1]) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: bad MReqAddressTranslate sideband signal\n",
+                          __func__);
+        }
         ch->data_type = 1 << (value & 3);
         if ((value & 3) == 3) {
-            printf("%s: bad data_type for DMA channel\n", __func__);
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: bad data_type for DMA channel\n", __func__);
             ch->data_type >>= 1;
         }
         break;
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 04/11] hw/dma/omap_dma: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/dma/omap_dma.c | 66 ++++++++++++++++++++++++++++++-----------------
>  1 file changed, 42 insertions(+), 24 deletions(-)
[...]
>      case 0x78:	/* DMA4_GCR */
>          s->gcr = value & 0x00ff00ff;
> -	if ((value & 0xff) == 0x00)		/* MAX_CHANNEL_FIFO_DEPTH */
> -            fprintf(stderr, "%s: wrong FIFO depth in GCR\n", __func__);
> +        if ((value & 0xff) == 0x00) {
> +            /* MAX_CHANNEL_FIFO_DEPTH */
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: wrong FIFO depth in GCR\n",
> +                          __func__);
> +        }
>          return;

Not sure, but doesn't that MAX_CHANNEL_FIFO_DEPTH comment rather belong
to the if-statement than to the print statement? If so, could you please
leave it at the end of the line?

 Thomas

Re: [Qemu-devel] [PATCH 04/11] hw/dma/omap_dma: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Philippe Mathieu-Daudé]

On 06/22/2018 05:24 AM, Thomas Huth wrote:
> On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/dma/omap_dma.c | 66 ++++++++++++++++++++++++++++++-----------------
>>  1 file changed, 42 insertions(+), 24 deletions(-)
> [...]
>>      case 0x78:	/* DMA4_GCR */
>>          s->gcr = value & 0x00ff00ff;
>> -	if ((value & 0xff) == 0x00)		/* MAX_CHANNEL_FIFO_DEPTH */
>> -            fprintf(stderr, "%s: wrong FIFO depth in GCR\n", __func__);
>> +        if ((value & 0xff) == 0x00) {
>> +            /* MAX_CHANNEL_FIFO_DEPTH */
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: wrong FIFO depth in GCR\n",
>> +                          __func__);
>> +        }
>>          return;
> 
> Not sure, but doesn't that MAX_CHANNEL_FIFO_DEPTH comment rather belong
> to the if-statement than to the print statement? If so, could you please
> leave it at the end of the line?

OK ;)

[Qemu-devel] [PATCH 05/11] hw/ssi/omap_spi: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ssi/omap_spi.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/ssi/omap_spi.c b/hw/ssi/omap_spi.c
index 34163e5646..f278a55160 100644
--- a/hw/ssi/omap_spi.c
+++ b/hw/ssi/omap_spi.c
@@ -20,6 +20,7 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "hw/hw.h"
 #include "hw/arm/omap.h"
 
@@ -294,11 +295,15 @@ static void omap_mcspi_write(void *opaque, hwaddr addr,
     case 0x2c:	/* MCSPI_CHCONF */
         if ((value ^ s->ch[ch].config) & (3 << 14))	/* DMAR | DMAW */
             omap_mcspi_dmarequest_update(s->ch + ch);
-        if (((value >> 12) & 3) == 3)			/* TRM */
-            fprintf(stderr, "%s: invalid TRM value (3)\n", __func__);
-        if (((value >> 7) & 0x1f) < 3)			/* WL */
-            fprintf(stderr, "%s: invalid WL value (%" PRIx64 ")\n",
-                            __func__, (value >> 7) & 0x1f);
+        if (((value >> 12) & 3) == 3) { /* TRM */
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid TRM value (3)\n",
+                          __func__);
+        }
+        if (((value >> 7) & 0x1f) < 3) { /* WL */
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: invalid WL value (%" PRIx64 ")\n",
+                          __func__, (value >> 7) & 0x1f);
+        }
         s->ch[ch].config = value & 0x7fffff;
         break;
 
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 05/11] hw/ssi/omap_spi: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Alistair Francis]

On Thu, Jun 21, 2018 at 11:02 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/ssi/omap_spi.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ssi/omap_spi.c b/hw/ssi/omap_spi.c
> index 34163e5646..f278a55160 100644
> --- a/hw/ssi/omap_spi.c
> +++ b/hw/ssi/omap_spi.c
> @@ -20,6 +20,7 @@
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "hw/hw.h"
>  #include "hw/arm/omap.h"
>
> @@ -294,11 +295,15 @@ static void omap_mcspi_write(void *opaque, hwaddr addr,
>      case 0x2c: /* MCSPI_CHCONF */
>          if ((value ^ s->ch[ch].config) & (3 << 14))    /* DMAR | DMAW */
>              omap_mcspi_dmarequest_update(s->ch + ch);
> -        if (((value >> 12) & 3) == 3)                  /* TRM */
> -            fprintf(stderr, "%s: invalid TRM value (3)\n", __func__);
> -        if (((value >> 7) & 0x1f) < 3)                 /* WL */
> -            fprintf(stderr, "%s: invalid WL value (%" PRIx64 ")\n",
> -                            __func__, (value >> 7) & 0x1f);
> +        if (((value >> 12) & 3) == 3) { /* TRM */
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid TRM value (3)\n",
> +                          __func__);
> +        }
> +        if (((value >> 7) & 0x1f) < 3) { /* WL */
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: invalid WL value (%" PRIx64 ")\n",
> +                          __func__, (value >> 7) & 0x1f);
> +        }
>          s->ch[ch].config = value & 0x7fffff;
>          break;
>
> --
> 2.18.0.rc2
>
>

[Qemu-devel] [PATCH 06/11] hw/sd/omap_mmc: Use qemu_log_mask(UNIMP) instead of printf

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/omap_mmc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index 5b47cadf11..aa2a816f76 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -17,6 +17,7 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "hw/hw.h"
 #include "hw/arm/omap.h"
 #include "hw/sd/sd.h"
@@ -449,10 +450,14 @@ static void omap_mmc_write(void *opaque, hwaddr offset,
         s->enable = (value >> 11) & 1;
         s->be = (value >> 10) & 1;
         s->clkdiv = (value >> 0) & (s->rev >= 2 ? 0x3ff : 0xff);
-        if (s->mode != 0)
-            printf("SD mode %i unimplemented!\n", s->mode);
-        if (s->be != 0)
-            printf("SD FIFO byte sex unimplemented!\n");
+        if (s->mode != 0) {
+            qemu_log_mask(LOG_UNIMP,
+                          "omap_mmc_wr: mode #%i unimplemented\n", s->mode);
+        }
+        if (s->be != 0) {
+            qemu_log_mask(LOG_UNIMP,
+                          "omap_mmc_wr: Big Endian not implemented\n");
+        }
         if (s->dw != 0 && s->lines < 4)
             printf("4-bit SD bus enabled\n");
         if (!s->enable)
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 06/11] hw/sd/omap_mmc: Use qemu_log_mask(UNIMP) instead of printf

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/omap_mmc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
> index 5b47cadf11..aa2a816f76 100644
> --- a/hw/sd/omap_mmc.c
> +++ b/hw/sd/omap_mmc.c
> @@ -17,6 +17,7 @@
>   * with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "hw/hw.h"
>  #include "hw/arm/omap.h"
>  #include "hw/sd/sd.h"
> @@ -449,10 +450,14 @@ static void omap_mmc_write(void *opaque, hwaddr offset,
>          s->enable = (value >> 11) & 1;
>          s->be = (value >> 10) & 1;
>          s->clkdiv = (value >> 0) & (s->rev >= 2 ? 0x3ff : 0xff);
> -        if (s->mode != 0)
> -            printf("SD mode %i unimplemented!\n", s->mode);
> -        if (s->be != 0)
> -            printf("SD FIFO byte sex unimplemented!\n");
> +        if (s->mode != 0) {
> +            qemu_log_mask(LOG_UNIMP,
> +                          "omap_mmc_wr: mode #%i unimplemented\n", s->mode);
> +        }
> +        if (s->be != 0) {
> +            qemu_log_mask(LOG_UNIMP,
> +                          "omap_mmc_wr: Big Endian not implemented\n");
> +        }
>          if (s->dw != 0 && s->lines < 4)
>              printf("4-bit SD bus enabled\n");
>          if (!s->enable)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[Qemu-devel] [PATCH 07/11] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/i2c/omap_i2c.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/i2c/omap_i2c.c b/hw/i2c/omap_i2c.c
index 26e3e5ebf6..e6f34fe31f 100644
--- a/hw/i2c/omap_i2c.c
+++ b/hw/i2c/omap_i2c.c
@@ -17,6 +17,7 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "hw/hw.h"
 #include "hw/i2c/i2c.h"
 #include "hw/arm/omap.h"
@@ -339,14 +340,17 @@ static void omap_i2c_write(void *opaque, hwaddr addr,
             }
             break;
         }
-        if ((value & (1 << 15)) && !(value & (1 << 10))) {	/* MST */
-            fprintf(stderr, "%s: I^2C slave mode not supported\n",
-                            __func__);
+        if ((value & (1 << 15)) && !(value & (1 << 10))) {
+            /* MST */
+            qemu_log_mask(LOG_UNIMP, "%s: I^2C slave mode not supported\n",
+                          __func__);
             break;
         }
-        if ((value & (1 << 15)) && value & (1 << 8)) {		/* XA */
-            fprintf(stderr, "%s: 10-bit addressing mode not supported\n",
-                            __func__);
+        if ((value & (1 << 15)) && value & (1 << 8)) {
+            /* XA */
+            qemu_log_mask(LOG_UNIMP,
+                          "%s: 10-bit addressing mode not supported\n",
+                          __func__);
             break;
         }
         if ((value & (1 << 15)) && value & (1 << 0)) {		/* STT */
@@ -392,8 +396,11 @@ static void omap_i2c_write(void *opaque, hwaddr addr,
                 s->stat |= 0x3f;
                 omap_i2c_interrupts_update(s);
             }
-        if (value & (1 << 15))					/* ST_EN */
-            fprintf(stderr, "%s: System Test not supported\n", __func__);
+        if (value & (1 << 15)) {
+            /* ST_EN */
+            qemu_log_mask(LOG_UNIMP,
+                          "%s: System Test not supported\n", __func__);
+        }
         break;
 
     default:
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 07/11] hw/i2c/omap_i2c: Use qemu_log_mask(UNIMP) instead of fprintf

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/i2c/omap_i2c.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i2c/omap_i2c.c b/hw/i2c/omap_i2c.c
> index 26e3e5ebf6..e6f34fe31f 100644
> --- a/hw/i2c/omap_i2c.c
> +++ b/hw/i2c/omap_i2c.c
> @@ -17,6 +17,7 @@
>   * with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "hw/hw.h"
>  #include "hw/i2c/i2c.h"
>  #include "hw/arm/omap.h"
> @@ -339,14 +340,17 @@ static void omap_i2c_write(void *opaque, hwaddr addr,
>              }
>              break;
>          }
> -        if ((value & (1 << 15)) && !(value & (1 << 10))) {	/* MST */
> -            fprintf(stderr, "%s: I^2C slave mode not supported\n",
> -                            __func__);
> +        if ((value & (1 << 15)) && !(value & (1 << 10))) {
> +            /* MST */
> +            qemu_log_mask(LOG_UNIMP, "%s: I^2C slave mode not supported\n",
> +                          __func__);
>              break;
>          }
> -        if ((value & (1 << 15)) && value & (1 << 8)) {		/* XA */
> -            fprintf(stderr, "%s: 10-bit addressing mode not supported\n",
> -                            __func__);
> +        if ((value & (1 << 15)) && value & (1 << 8)) {
> +            /* XA */
> +            qemu_log_mask(LOG_UNIMP,
> +                          "%s: 10-bit addressing mode not supported\n",
> +                          __func__);
>              break;
>          }
>          if ((value & (1 << 15)) && value & (1 << 0)) {		/* STT */
> @@ -392,8 +396,11 @@ static void omap_i2c_write(void *opaque, hwaddr addr,
>                  s->stat |= 0x3f;
>                  omap_i2c_interrupts_update(s);
>              }
> -        if (value & (1 << 15))					/* ST_EN */
> -            fprintf(stderr, "%s: System Test not supported\n", __func__);
> +        if (value & (1 << 15)) {
> +            /* ST_EN */
> +            qemu_log_mask(LOG_UNIMP,
> +                          "%s: System Test not supported\n", __func__);
> +        }
>          break;
>  
>      default:

Could you please leave the comments at the end of the if-statement lines?

 Thomas

[Qemu-devel] [PATCH 08/11] hw/arm/omap: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/arm/omap.h | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h
index b398607b06..3d805b6174 100644
--- a/include/hw/arm/omap.h
+++ b/include/hw/arm/omap.h
@@ -21,6 +21,7 @@
 # define hw_omap_h		"omap.h"
 #include "hw/irq.h"
 #include "target/arm/cpu-qom.h"
+#include "qemu/log.h"
 
 # define OMAP_EMIFS_BASE	0x00000000
 # define OMAP2_Q0_BASE		0x00000000
@@ -962,8 +963,8 @@ void omap_mpu_wakeup(void *opaque, int irq, int req);
         fprintf(stderr, "%s: Bad register " OMAP_FMT_plx "\n",	\
                         __func__, paddr)
 # define OMAP_RO_REG(paddr)		\
-        fprintf(stderr, "%s: Read-only register " OMAP_FMT_plx "\n",	\
-                        __func__, paddr)
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Read-only register " OMAP_FMT_plx \
+                      "\n", __func__, paddr)
 
 /* OMAP-specific Linux bootloader tags for the ATAG_BOARD area
    (Board-specifc tags are not here)  */
@@ -997,14 +998,17 @@ enum {
 
 # ifdef TCMI_VERBOSE
 #  define OMAP_8B_REG(paddr)		\
-        fprintf(stderr, "%s: 8-bit register " OMAP_FMT_plx "\n",	\
-                        __func__, paddr)
+        qemu_log_mask(LOG_GUEST_ERROR, \
+                      "%s: 8-bit register " OMAP_FMT_plx "\n", \
+                      __func__, paddr)
 #  define OMAP_16B_REG(paddr)		\
-        fprintf(stderr, "%s: 16-bit register " OMAP_FMT_plx "\n",	\
-                        __func__, paddr)
+        qemu_log_mask(LOG_GUEST_ERROR, \
+                      "%s: 16-bit register " OMAP_FMT_plx "\n", \
+                      __func__, paddr)
 #  define OMAP_32B_REG(paddr)		\
-        fprintf(stderr, "%s: 32-bit register " OMAP_FMT_plx "\n",	\
-                        __func__, paddr)
+        qemu_log_mask(LOG_GUEST_ERROR, \
+                      "%s: 32-bit register " OMAP_FMT_plx "\n", \
+                      __func__, paddr)
 # else
 #  define OMAP_8B_REG(paddr)
 #  define OMAP_16B_REG(paddr)
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 08/11] hw/arm/omap: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/arm/omap.h | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
[...]
> @@ -997,14 +998,17 @@ enum {
>  
>  # ifdef TCMI_VERBOSE
>  #  define OMAP_8B_REG(paddr)		\
> -        fprintf(stderr, "%s: 8-bit register " OMAP_FMT_plx "\n",	\
> -                        __func__, paddr)
> +        qemu_log_mask(LOG_GUEST_ERROR, \
> +                      "%s: 8-bit register " OMAP_FMT_plx "\n", \
> +                      __func__, paddr)
>  #  define OMAP_16B_REG(paddr)		\
> -        fprintf(stderr, "%s: 16-bit register " OMAP_FMT_plx "\n",	\
> -                        __func__, paddr)
> +        qemu_log_mask(LOG_GUEST_ERROR, \
> +                      "%s: 16-bit register " OMAP_FMT_plx "\n", \
> +                      __func__, paddr)
>  #  define OMAP_32B_REG(paddr)		\
> -        fprintf(stderr, "%s: 32-bit register " OMAP_FMT_plx "\n",	\
> -                        __func__, paddr)
> +        qemu_log_mask(LOG_GUEST_ERROR, \
> +                      "%s: 32-bit register " OMAP_FMT_plx "\n", \
> +                      __func__, paddr)
>  # else
>  #  define OMAP_8B_REG(paddr)
>  #  define OMAP_16B_REG(paddr)
> 

Since the qemu_log_mask(LOG_GUEST_ERROR, ...) calls are not shown up by
default anymore, I think it would rather be better to get rid of this
#ifdef TCMI_VERBOSE ... #endif block now and put the qemu_log_mask()
statements directly into the omap_badwidth_* functions instead.

 Thomas

Re: [Qemu-devel] [PATCH 08/11] hw/arm/omap: Use qemu_log_mask(GUEST_ERROR) instead of fprintf

[Philippe Mathieu-Daudé]

On 06/22/2018 05:34 AM, Thomas Huth wrote:
> On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/hw/arm/omap.h | 20 ++++++++++++--------
>>  1 file changed, 12 insertions(+), 8 deletions(-)
> [...]
>> @@ -997,14 +998,17 @@ enum {
>>  
>>  # ifdef TCMI_VERBOSE
>>  #  define OMAP_8B_REG(paddr)		\
>> -        fprintf(stderr, "%s: 8-bit register " OMAP_FMT_plx "\n",	\
>> -                        __func__, paddr)
>> +        qemu_log_mask(LOG_GUEST_ERROR, \
>> +                      "%s: 8-bit register " OMAP_FMT_plx "\n", \
>> +                      __func__, paddr)
>>  #  define OMAP_16B_REG(paddr)		\
>> -        fprintf(stderr, "%s: 16-bit register " OMAP_FMT_plx "\n",	\
>> -                        __func__, paddr)
>> +        qemu_log_mask(LOG_GUEST_ERROR, \
>> +                      "%s: 16-bit register " OMAP_FMT_plx "\n", \
>> +                      __func__, paddr)
>>  #  define OMAP_32B_REG(paddr)		\
>> -        fprintf(stderr, "%s: 32-bit register " OMAP_FMT_plx "\n",	\
>> -                        __func__, paddr)
>> +        qemu_log_mask(LOG_GUEST_ERROR, \
>> +                      "%s: 32-bit register " OMAP_FMT_plx "\n", \
>> +                      __func__, paddr)
>>  # else
>>  #  define OMAP_8B_REG(paddr)
>>  #  define OMAP_16B_REG(paddr)
>>
> 
> Since the qemu_log_mask(LOG_GUEST_ERROR, ...) calls are not shown up by
> default anymore, I think it would rather be better to get rid of this
> #ifdef TCMI_VERBOSE ... #endif block now and put the qemu_log_mask()
> statements directly into the omap_badwidth_* functions instead.

Good idea!

[Qemu-devel] [PATCH 09/11] hw/arm/stellaris: Use qemu_log_mask(UNIMP) instead of fprintf

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/stellaris.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index a8f1f6a912..d06e366402 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -560,7 +560,7 @@ static void ssys_write(void *opaque, hwaddr offset,
     case 0x040: /* SRCR0 */
     case 0x044: /* SRCR1 */
     case 0x048: /* SRCR2 */
-        fprintf(stderr, "Peripheral reset not implemented\n");
+        qemu_log_mask(LOG_UNIMP, "Peripheral reset not implemented\n");
         break;
     case 0x054: /* IMC */
         s->int_mask = value & 0x7f;
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 09/11] hw/arm/stellaris: Use qemu_log_mask(UNIMP) instead of fprintf

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/stellaris.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index a8f1f6a912..d06e366402 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -560,7 +560,7 @@ static void ssys_write(void *opaque, hwaddr offset,
>      case 0x040: /* SRCR0 */
>      case 0x044: /* SRCR1 */
>      case 0x048: /* SRCR2 */
> -        fprintf(stderr, "Peripheral reset not implemented\n");
> +        qemu_log_mask(LOG_UNIMP, "Peripheral reset not implemented\n");
>          break;
>      case 0x054: /* IMC */
>          s->int_mask = value & 0x7f;
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

[Qemu-devel] [PATCH 10/11] hw/net/stellaris_enet: Use qemu_log_mask(GUEST_ERROR) instead of hw_error

[Philippe Mathieu-Daudé]

hw_error() finally calls abort(), but there is no need to abort here.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/stellaris_enet.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index 04bd10ada3..188adcbd15 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -9,6 +9,7 @@
 #include "qemu/osdep.h"
 #include "hw/sysbus.h"
 #include "net/net.h"
+#include "qemu/log.h"
 #include <zlib.h>
 
 //#define DEBUG_STELLARIS_ENET 1
@@ -343,7 +344,9 @@ static uint64_t stellaris_enet_read(void *opaque, hwaddr offset,
     case 0x3c: /* Undocuented: Timestamp? */
         return 0;
     default:
-        hw_error("stellaris_enet_read: Bad offset %x\n", (int)offset);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "stellaris_enet_rd%d: 0x%" HWADDR_PRIx "\n",
+                      size, offset);
         return 0;
     }
 }
@@ -442,7 +445,9 @@ static void stellaris_enet_write(void *opaque, hwaddr offset,
         /* Ignored.  */
         break;
     default:
-        hw_error("stellaris_enet_write: Bad offset %x\n", (int)offset);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "stellaris_enet_wr%d: 0x%" HWADDR_PRIx " = 0x%lx\n",
+                      size, offset, value);
     }
 }
 
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 10/11] hw/net/stellaris_enet: Use qemu_log_mask(GUEST_ERROR) instead of hw_error

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> hw_error() finally calls abort(), but there is no need to abort here.

Additionally, hw_error() is also only meant for CPU errors (since it
dumps the CPU state). We should really rename that function to
"cpu_hw_error" one day to avoid that people use it in the wrong spots...

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/net/stellaris_enet.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
> index 04bd10ada3..188adcbd15 100644
> --- a/hw/net/stellaris_enet.c
> +++ b/hw/net/stellaris_enet.c
> @@ -9,6 +9,7 @@
>  #include "qemu/osdep.h"
>  #include "hw/sysbus.h"
>  #include "net/net.h"
> +#include "qemu/log.h"
>  #include <zlib.h>
>  
>  //#define DEBUG_STELLARIS_ENET 1
> @@ -343,7 +344,9 @@ static uint64_t stellaris_enet_read(void *opaque, hwaddr offset,
>      case 0x3c: /* Undocuented: Timestamp? */
>          return 0;
>      default:
> -        hw_error("stellaris_enet_read: Bad offset %x\n", (int)offset);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "stellaris_enet_rd%d: 0x%" HWADDR_PRIx "\n",
> +                      size, offset);
>          return 0;
>      }
>  }
> @@ -442,7 +445,9 @@ static void stellaris_enet_write(void *opaque, hwaddr offset,
>          /* Ignored.  */
>          break;
>      default:
> -        hw_error("stellaris_enet_write: Bad offset %x\n", (int)offset);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "stellaris_enet_wr%d: 0x%" HWADDR_PRIx " = 0x%lx\n",
> +                      size, offset, value);
>      }
>  }

Could you please keep the "bad offset" or a similar error message, so
that the user knows that there is really something wrong here?

 Thomas

Re: [Qemu-devel] [PATCH 10/11] hw/net/stellaris_enet: Use qemu_log_mask(GUEST_ERROR) instead of hw_error

[Philippe Mathieu-Daudé]

On 06/22/2018 05:43 AM, Thomas Huth wrote:
> On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
>> hw_error() finally calls abort(), but there is no need to abort here.
> 
> Additionally, hw_error() is also only meant for CPU errors (since it
> dumps the CPU state). We should really rename that function to
> "cpu_hw_error" one day to avoid that people use it in the wrong spots...

Another painful cleanup series...

> 
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/net/stellaris_enet.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
>> index 04bd10ada3..188adcbd15 100644
>> --- a/hw/net/stellaris_enet.c
>> +++ b/hw/net/stellaris_enet.c
>> @@ -9,6 +9,7 @@
>>  #include "qemu/osdep.h"
>>  #include "hw/sysbus.h"
>>  #include "net/net.h"
>> +#include "qemu/log.h"
>>  #include <zlib.h>
>>  
>>  //#define DEBUG_STELLARIS_ENET 1
>> @@ -343,7 +344,9 @@ static uint64_t stellaris_enet_read(void *opaque, hwaddr offset,
>>      case 0x3c: /* Undocuented: Timestamp? */
>>          return 0;
>>      default:
>> -        hw_error("stellaris_enet_read: Bad offset %x\n", (int)offset);
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "stellaris_enet_rd%d: 0x%" HWADDR_PRIx "\n",
>> +                      size, offset);
>>          return 0;
>>      }
>>  }
>> @@ -442,7 +445,9 @@ static void stellaris_enet_write(void *opaque, hwaddr offset,
>>          /* Ignored.  */
>>          break;
>>      default:
>> -        hw_error("stellaris_enet_write: Bad offset %x\n", (int)offset);
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "stellaris_enet_wr%d: 0x%" HWADDR_PRIx " = 0x%lx\n",
>> +                      size, offset, value);
>>      }
>>  }
> 
> Could you please keep the "bad offset" or a similar error message, so
> that the user knows that there is really something wrong here?

Sure.

[Qemu-devel] [PATCH 11/11] hw/net/smc91c111: Use qemu_log_mask(GUEST_ERROR) instead of hw_error

[Philippe Mathieu-Daudé]

hw_error() finally calls abort(), but there is no need to abort here.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/smc91c111.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index c8cc5379b7..705a034cb9 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -11,6 +11,7 @@
 #include "hw/sysbus.h"
 #include "net/net.h"
 #include "hw/devices.h"
+#include "qemu/log.h"
 /* For crc32 */
 #include <zlib.h>
 
@@ -478,7 +479,9 @@ static void smc91c111_writeb(void *opaque, hwaddr offset,
         }
         break;
     }
-    hw_error("smc91c111_write: Bad reg %d:%x\n", s->bank, (int)offset);
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "smc91c111_write(bank:%d) 0x%" HWADDR_PRIx " = 0x%x\n",
+                  s->bank, offset, value);
 }
 
 static uint32_t smc91c111_readb(void *opaque, hwaddr offset)
@@ -621,7 +624,9 @@ static uint32_t smc91c111_readb(void *opaque, hwaddr offset)
         }
         break;
     }
-    hw_error("smc91c111_read: Bad reg %d:%x\n", s->bank, (int)offset);
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "smc91c111_read(bank:%d) 0x%" HWADDR_PRIx "\n",
+                  s->bank, offset);
     return 0;
 }
 
-- 
2.18.0.rc2

Re: [Qemu-devel] [PATCH 11/11] hw/net/smc91c111: Use qemu_log_mask(GUEST_ERROR) instead of hw_error

[Thomas Huth]

On 21.06.2018 20:02, Philippe Mathieu-Daudé wrote:
> hw_error() finally calls abort(), but there is no need to abort here.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/net/smc91c111.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
> index c8cc5379b7..705a034cb9 100644
> --- a/hw/net/smc91c111.c
> +++ b/hw/net/smc91c111.c
> @@ -11,6 +11,7 @@
>  #include "hw/sysbus.h"
>  #include "net/net.h"
>  #include "hw/devices.h"
> +#include "qemu/log.h"
>  /* For crc32 */
>  #include <zlib.h>
>  
> @@ -478,7 +479,9 @@ static void smc91c111_writeb(void *opaque, hwaddr offset,
>          }
>          break;
>      }
> -    hw_error("smc91c111_write: Bad reg %d:%x\n", s->bank, (int)offset);
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "smc91c111_write(bank:%d) 0x%" HWADDR_PRIx " = 0x%x\n",
> +                  s->bank, offset, value);
>  }
>  
>  static uint32_t smc91c111_readb(void *opaque, hwaddr offset)
> @@ -621,7 +624,9 @@ static uint32_t smc91c111_readb(void *opaque, hwaddr offset)
>          }
>          break;
>      }
> -    hw_error("smc91c111_read: Bad reg %d:%x\n", s->bank, (int)offset);
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "smc91c111_read(bank:%d) 0x%" HWADDR_PRIx "\n",
> +                  s->bank, offset);
>      return 0;
>  }

Please keep "bad register" or a similar text in here.

 Thomas