Re: [PATCH 05/14] hw/display/cirrus_vga: Use qemu_log_mask(ERROR) instead of debug printf

[Alistair Francis]

On Mon, May 25, 2020 at 11:27 PM Philippe Mathieu-Daudé <address@hidden> wrote:
>
> Replace some debug printf() calls by qemu_log_mask(LOG_GUEST_ERROR).
>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>

Reviewed-by: Alistair Francis <address@hidden>

Alistair

> ---
>  hw/display/cirrus_vga.c | 77 ++++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 44 deletions(-)
>
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index f9f837b850..76e2dc5bb6 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -978,9 +978,8 @@ static void cirrus_bitblt_start(CirrusVGAState * s)
>         s->cirrus_blt_pixelwidth = 4;
>         break;
>      default:
> -#ifdef DEBUG_BITBLT
> -       printf("cirrus: bitblt - pixel width is unknown\n");
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: bitblt - pixel width is unknown\n");
>         goto bitblt_ignore;
>      }
>      s->cirrus_blt_mode &= ~CIRRUS_BLTMODE_PIXELWIDTHMASK;
> @@ -1037,7 +1036,9 @@ static void cirrus_bitblt_start(CirrusVGAState * s)
>          } else {
>             if (s->cirrus_blt_mode & CIRRUS_BLTMODE_TRANSPARENTCOMP) {
>                 if (s->cirrus_blt_pixelwidth > 2) {
> -                   printf("src transparent without colorexpand must be 8bpp 
> or 16bpp\n");
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "cirrus: src transparent without colorexpand "
> +                          "must be 8bpp or 16bpp\n");
>                     goto bitblt_ignore;
>                 }
>                 if (s->cirrus_blt_mode & CIRRUS_BLTMODE_BACKWARDS) {
> @@ -1135,10 +1136,9 @@ static uint32_t cirrus_get_bpp16_depth(CirrusVGAState 
> * s)
>         ret = 16;
>         break;                  /* XGA HiColor */
>      default:
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: invalid DAC value %x in 16bpp\n",
> -              (s->cirrus_hidden_dac_data & 0xf));
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: invalid DAC value 0x%x in 16bpp\n",
> +                      (s->cirrus_hidden_dac_data & 0xf));
>         ret = 15;               /* XXX */
>         break;
>      }
> @@ -1307,11 +1307,9 @@ static int cirrus_vga_read_sr(CirrusVGAState * s)
>  #endif
>         return s->vga.sr[s->vga.sr_index];
>      default:
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: inport sr_index %02x\n", s->vga.sr_index);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: inport sr_index 0x%02x\n", s->vga.sr_index);
>         return 0xff;
> -       break;
>      }
>  }
>
> @@ -1400,10 +1398,9 @@ static void cirrus_vga_write_sr(CirrusVGAState * s, 
> uint32_t val)
>          cirrus_update_memory_access(s);
>          break;
>      default:
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: outport sr_index %02x, sr_value %02x\n",
> -               s->vga.sr_index, val);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: outport sr_index 0x%02x, sr_value 0x%02x\n",
> +                      s->vga.sr_index, val);
>         break;
>      }
>  }
> @@ -1501,9 +1498,8 @@ static int cirrus_vga_read_gr(CirrusVGAState * s, 
> unsigned reg_index)
>      if (reg_index < 0x3a) {
>         return s->vga.gr[reg_index];
>      } else {
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: inport gr_index %02x\n", reg_index);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: inport gr_index 0x%02x\n", reg_index);
>         return 0xff;
>      }
>  }
> @@ -1590,10 +1586,9 @@ cirrus_vga_write_gr(CirrusVGAState * s, unsigned 
> reg_index, int reg_value)
>         cirrus_write_bitblt(s, reg_value);
>         break;
>      default:
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: outport gr_index %02x, gr_value %02x\n", reg_index,
> -              reg_value);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: outport gr_index 0x%02x, gr_value 0x%02x\n",
> +                      reg_index, reg_value);
>         break;
>      }
>  }
> @@ -1648,9 +1643,8 @@ static int cirrus_vga_read_cr(CirrusVGAState * s, 
> unsigned reg_index)
>         return s->vga.ar_index & 0x3f;
>         break;
>      default:
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: inport cr_index %02x\n", reg_index);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: inport cr_index 0x%02x\n", reg_index);
>         return 0xff;
>      }
>  }
> @@ -1721,10 +1715,9 @@ static void cirrus_vga_write_cr(CirrusVGAState * s, 
> int reg_value)
>         break;
>      case 0x25:                 // Part Status
>      default:
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: outport cr_index %02x, cr_value %02x\n",
> -               s->vga.cr_index, reg_value);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: outport cr_index 0x%02x, cr_value 0x%02x\n",
> +                      s->vga.cr_index, reg_value);
>         break;
>      }
>  }
> @@ -1834,9 +1827,8 @@ static uint8_t cirrus_mmio_blt_read(CirrusVGAState * s, 
> unsigned address)
>         value = cirrus_vga_read_gr(s, 0x31);
>         break;
>      default:
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: mmio read - address 0x%04x\n", address);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: mmio read - address 0x%04x\n", address);
>         break;
>      }
>
> @@ -1946,10 +1938,9 @@ static void cirrus_mmio_blt_write(CirrusVGAState * s, 
> unsigned address,
>         cirrus_vga_write_gr(s, 0x31, value);
>         break;
>      default:
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: mmio write - addr 0x%04x val 0x%02x (ignored)\n",
> -              address, value);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: mmio write - addr 0x%04x val 0x%02x 
> (ignored)\n",
> +                      address, value);
>         break;
>      }
>  }
> @@ -2047,9 +2038,8 @@ static uint64_t cirrus_vga_mem_read(void *opaque,
>         }
>      } else {
>         val = 0xff;
> -#ifdef DEBUG_CIRRUS
> -       printf("cirrus: mem_readb " TARGET_FMT_plx "\n", addr);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: mem_readb 0x" TARGET_FMT_plx "\n", addr);
>      }
>      return val;
>  }
> @@ -2112,10 +2102,9 @@ static void cirrus_vga_mem_write(void *opaque,
>             cirrus_mmio_blt_write(s, addr & 0xff, mem_value);
>         }
>      } else {
> -#ifdef DEBUG_CIRRUS
> -        printf("cirrus: mem_writeb " TARGET_FMT_plx " value 0x%02" PRIu64 
> "\n", addr,
> -               mem_value);
> -#endif
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "cirrus: mem_writeb 0x" TARGET_FMT_plx " "
> +                      "value 0x%02" PRIu64 "\n", addr, mem_value);
>      }
>  }
>
> --
> 2.21.3
>
>