Cc'ing Stefan
On 7/20/19 11:44 AM, Philippe Mathieu-Daudé wrote:
> 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 <frederic.kon...@adacore.com>
>> ---
>> 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.
Oh, now I see that HTIF_DEBUG() is just an obscure way to report
crippled log trace-events, not portable to all trace backends.
It is only used in 2 files:
- hw/riscv/riscv_htif.c
- target/riscv/pmp.c as PMP_DEBUG()
I'd rather remove these macros and use trace-events the same way the
rest of the codebase use them, usable by all backends.
> 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);
>> }
>> }
>>
>>
