Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write

[Paolo Bonzini]

On 08/09/2016 07:43, Stefan Weil wrote:
> Am 07.09.2016 um 23:29 schrieb Peter Maydell:
>> On 7 September 2016 at 20:13, Stefan Weil <address@hidden> wrote:
>>> The old log messages are implemented by conditional compilation
>>> and not available by default.
>>>
>>> The new log messages can be enabled either by a command line option
>>> (-d unimp) or in the QEMU monitor (log unimp).
>>>
>>> Signed-off-by: Stefan Weil <address@hidden>
>>> ---
>>>
>>> The new code is very useful when implementing new platforms or
>>> looking for problems with existing platforms which are only
>>> partially emulated (I use it for Raspberry Pi).
>>>
>>> target-sparc/ldst_helper.c also uses DEBUG_UNASSIGNED
>>> and could get similar code.
>>>
>>> Regards,
>>> Stefan
>>>
>>>
>>>  memory.c | 12 ++++--------
>>>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> [...]
> 
>>> -#ifdef DEBUG_UNASSIGNED
>>> -    printf("Unassigned mem write " TARGET_FMT_plx " =
>>> 0x%"PRIx64"\n", addr, val);
>>> -#endif
>>> +    qemu_log_mask(LOG_UNIMP, "%s " TARGET_FMT_plx " = 0x%" PRIx64 "\n",
>>> +                  __func__, addr, val);
>>
>> Maybe this should be LOG_GUEST_ERROR rather than LOG_UNIMP ?
> 
> My first code used LOG_GUEST_ERROR. Of course a guest can try to access
> memory which is not available for the real hardware.
> 
> For my test object (Raspberry Pi), all unassigned memory accesses were
> not guest errors but unimplemented emulation in QEMU – that's why I
> changed from LOG_GUEST_ERROR to LOG_UNIMP.
> 
> A short test with i386 / x86_64 emulation also detected unassigned
> memory accesses (from BIOS) caused by an unimplemented debug device, so
> that was also not a guest error.

FWIW, the debug device is optional (-device isa-debugcon), not
unimplemented.  Similarly it's common, or at least it was before ACPI
was introduced, to poke at known ISA ports to detect legacy devices;
it's neither unimplemented nor a guest error in that case.

Perhaps it should just be a tracepoint?

Paolo