Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: htif: use qemu_log_mask instead of qemu_log

[Philippe Mathieu-Daudé]

Hi Frederic,

On 7/20/19 8:18 AM, KONRAD Frederic wrote:
> There are some debug trace appearing when using GDB with the HTIF mapped @0:
>  Invalid htif read: address 0000000000000002
>  Invalid htif read: address 0000000000000006
>  Invalid htif read: address 000000000000000a
>  Invalid htif read: address 000000000000000e
> 
> So don't show them unconditionally.
> 
> Signed-off-by: KONRAD Frederic <address@hidden>
> ---
>  hw/riscv/riscv_htif.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/riscv/riscv_htif.c b/hw/riscv/riscv_htif.c
> index 4f7b11d..6a8f0c2 100644
> --- a/hw/riscv/riscv_htif.c
> +++ b/hw/riscv/riscv_htif.c
> @@ -119,7 +119,8 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>      int resp = 0;
>  
>      HTIF_DEBUG("mtohost write: device: %d cmd: %d what: %02" PRIx64
> -        " -payload: %016" PRIx64 "\n", device, cmd, payload & 0xFF, payload);
> +               " -payload: %016" PRIx64 "\n", device, cmd, payload & 0xFF,
> +               payload);
>  
>      /*
>       * Currently, there is a fixed mapping of devices:
> @@ -137,7 +138,7 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>                  qemu_log_mask(LOG_UNIMP, "pk syscall proxy not supported\n");
>              }
>          } else {
> -            qemu_log("HTIF device %d: unknown command\n", device);
> +            HTIF_DEBUG("HTIF device %d: unknown command\n", device);

Generally, please don't go back to using DPRINTF() but use trace-events
instead.

However in this calls it seems using qemu_log_mask(LOG_UNIMP) or
qemu_log_mask(LOG_GUEST_ERROR) is appropriate.

>          }
>      } else if (likely(device == 0x1)) {
>          /* HTIF Console */
> @@ -150,12 +151,13 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>              qemu_chr_fe_write(&htifstate->chr, (uint8_t *)&payload, 1);
>              resp = 0x100 | (uint8_t)payload;
>          } else {
> -            qemu_log("HTIF device %d: unknown command\n", device);
> +            HTIF_DEBUG("HTIF device %d: unknown command\n", device);
>          }
>      } else {
> -        qemu_log("HTIF unknown device or command\n");
> +        HTIF_DEBUG("HTIF unknown device or command\n");
>          HTIF_DEBUG("device: %d cmd: %d what: %02" PRIx64
> -            " payload: %016" PRIx64, device, cmd, payload & 0xFF, payload);
> +                   " payload: %016" PRIx64, device, cmd, payload & 0xFF,
> +                   payload);
>      }
>      /*
>       * - latest bbl does not set fromhost to 0 if there is a value in tohost
> @@ -180,6 +182,7 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>  static uint64_t htif_mm_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      HTIFState *htifstate = opaque;
> +
>      if (addr == TOHOST_OFFSET1) {
>          return htifstate->env->mtohost & 0xFFFFFFFF;
>      } else if (addr == TOHOST_OFFSET2) {
> @@ -189,8 +192,8 @@ static uint64_t htif_mm_read(void *opaque, hwaddr addr, 
> unsigned size)
>      } else if (addr == FROMHOST_OFFSET2) {
>          return (htifstate->env->mfromhost >> 32) & 0xFFFFFFFF;
>      } else {
> -        qemu_log("Invalid htif read: address %016" PRIx64 "\n",
> -            (uint64_t)addr);
> +        HTIF_DEBUG("Invalid htif read: address %016" PRIx64 "\n",
> +                   (uint64_t)addr);
>          return 0;
>      }
>  }
> @@ -219,8 +222,8 @@ static void htif_mm_write(void *opaque, hwaddr addr,
>          htifstate->env->mfromhost |= value << 32;
>          htifstate->fromhost_inprogress = 0;
>      } else {
> -        qemu_log("Invalid htif write: address %016" PRIx64 "\n",
> -            (uint64_t)addr);
> +        HTIF_DEBUG("Invalid htif write: address %016" PRIx64 "\n",
> +                   (uint64_t)addr);
>      }
>  }
>  
>