Re: [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()

[Philippe Mathieu-Daudé]

Hi Aleksandar,

On 6/25/19 2:37 AM, Aleksandar Markovic wrote:
> 
> This patch is not only mechanical replacement of printf(), but it also
> improves existing log messages, and adds some new ones as well. Reflect
> that in both commit message title and body. Perhaps there are more spots
> that deserve logging. But, also, please, Philippe, doublecheck in real
> scenarios if we don't flood the log with too many messages.

While qemu_log(...) might flood the user, qemu_log_mask(mask, ...) do
not. By default the mask is empty, and you have to enable the specific
bits you want the relevant information to be logged.

The mask comes from:

$ qemu-system-mips -d help
Log items (comma separated):
out_asm         show generated host assembly code for each compiled TB
in_asm          show target assembly code for each compiled TB
op              show micro ops for each compiled TB
op_opt          show micro ops after optimization
op_ind          show micro ops before indirect lowering
int             show interrupts/exceptions in short format
exec            show trace before each executed TB (lots of logs)
cpu             show CPU registers before entering a TB (lots of logs)
fpu             include FPU registers in the 'cpu' logging
mmu             log MMU-related activities
pcall           x86 only: show protected mode far calls/returns/exceptions
cpu_reset       show CPU state before CPU resets
unimp           log unimplemented functionality
guest_errors    log when the guest OS does something invalid (eg accessing a
non-existent register)
page            dump pages at beginning of user mode emulation
nochain         do not chain compiled TBs so that "exec" and "cpu" show
complete traces
trace:PATTERN   enable trace events

>>  hw/mips/gt64xxx_pci.c | 48 +++++++++++++++++++++++++++++++++----------
>>  1 file changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>> index 0b9fb02475..f44326f14f 100644
>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -23,6 +23,7 @@
>>   */
>>
>>  #include "qemu/osdep.h"
>> +#include "qemu/log.h"
>>  #include "hw/hw.h"
>>  #include "hw/mips/mips.h"
>>  #include "hw/pci/pci.h"
>> @@ -466,12 +467,20 @@ static void gt64120_writel(void *opaque, hwaddr
> addr,
>>      case GT_CPUERR_DATAHI:
>>      case GT_CPUERR_PARITY:
>>          /* Read-only registers, do nothing */
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "gt64120: Read-only register write "
>> +                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
>> +                      saddr << 2, size, size << 1, val);
>>          break;
[...]

So here if you do not run with '-d guest_errors', invalid accesses won't
be logged.

Note that there is no equivalent of error_report_once() with qemu_log(),
but IMO in case of I/O access I am not sure it would make sense.

Regards,

Phil.