Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write
Am 08.09.2016 um 11:37 schrieb 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 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
---
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
Yes, especially for x86 there are several devices which are optional
(e. g. serial and parallel ports). I also found the isa-debugcon device
when I searched for the reason of the unassigned memory access. Maybe
isa-debugcon would deserve better documentation.
Regarding using qemu_log_mask or using tracepoints
(trace_unassigned_mem_read, trace_unassigned_mem_write?): Both
alternatives are fine for me. I just want something which does not need
a new build with conditional compilation. It should be possible to
enable the log / trace message at any time. I can also prepare a new
patch with tracepoints. Any more feedback which variant is preferred?
Stefan
Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write
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 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
>>> ---
>>>
>>> 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
Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write
Am 07.09.2016 um 23:29 schrieb Peter Maydell:
On 7 September 2016 at 20:13, Stefan Weil 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
---
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.
Even bad memory access caused by the guest might need special handling
in QEMU, because the effect on real hardware can differ depending on the
address range. So although in that case LOG_GUEST_ERROR would be fine,
LOG_UNIMP matches as well.
It does not really matter whether the code uses LOG_UNIMP or
LOG_GUEST_ERROR, as long as it is possible to enable such log messages.
So whoever accepts this patch can simply replace that part if needed.
if (current_cpu != NULL) {
cpu_unassigned_access(current_cpu, addr, true, false, 0, size);
}
--
2.1.4
Idle thought: would it be worth having an "unimplemented-thing"
device which just existed to print LOG_GUEST_ERROR errors which
included a string indicating what the missing device was? The
board models would need to actually instantiate them in the
right places (hopefully just one-liners) but that's easier than
implementing lots of devices. And it would give us some
documentation in the source of what's still missing in a board.
That's what I am currently doing for RPi.
The main problem is that some devices for a certain platform are not
obvious, so you will miss them initially because of missing
documentation. Example for RPi: the random number generator.
Example for x86: the hardware debug device accesses by the BIOS.
Cheers
Stefan
Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write
On Wed, 09/07 22:50, Stefan Weil wrote: > Am 07.09.2016 um 22:17 schrieb no-re...@patchew.org: > > Hi, > > > > Your series failed automatic build test. Please find the testing commands > > and > > their output below. If you have docker installed, you can probably > > reproduce it > > locally. > > [...] > > > GTESTER tests/test-string-output-visitor > > GTESTER tests/test-qmp-event > > GTESTER tests/test-opts-visitor > > GTESTER tests/test-qht-par > > === OUTPUT END === > > > > Abort: command timeout (>3600 seconds) > > > It looks like the test failed because of a timeout after one hour. > Whatever caused that timeout, I don't think that my patch did it. Yes, it is because of the known vhost-user-test hang: https://patchwork.kernel.org/patch/9273189/ Fam
Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write
On 7 September 2016 at 20:13, Stefan Weil 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
> ---
>
> 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(-)
>
> diff --git a/memory.c b/memory.c
> index 0eb6895..92f8879 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -23,6 +23,7 @@
> #include "qapi/visitor.h"
> #include "qemu/bitops.h"
> #include "qemu/error-report.h"
> +#include "qemu/log.h"
> #include "qom/object.h"
> #include "trace.h"
>
> @@ -31,8 +32,6 @@
> #include "sysemu/kvm.h"
> #include "sysemu/sysemu.h"
>
> -//#define DEBUG_UNASSIGNED
> -
> static unsigned memory_region_transaction_depth;
> static bool memory_region_update_pending;
> static bool ioeventfd_update_pending;
> @@ -1101,9 +1100,7 @@ static void memory_region_initfn(Object *obj)
> static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> unsigned size)
> {
> -#ifdef DEBUG_UNASSIGNED
> -printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
> -#endif
> +qemu_log_mask(LOG_UNIMP, "%s " TARGET_FMT_plx "\n", __func__, addr);
> if (current_cpu != NULL) {
> cpu_unassigned_access(current_cpu, addr, false, false, 0, size);
> }
> @@ -1113,9 +1110,8 @@ static uint64_t unassigned_mem_read(void *opaque,
> hwaddr addr,
> static void unassigned_mem_write(void *opaque, hwaddr addr,
> uint64_t val, unsigned size)
> {
> -#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 ?
> if (current_cpu != NULL) {
> cpu_unassigned_access(current_cpu, addr, true, false, 0, size);
> }
> --
> 2.1.4
Idle thought: would it be worth having an "unimplemented-thing"
device which just existed to print LOG_GUEST_ERROR errors which
included a string indicating what the missing device was? The
board models would need to actually instantiate them in the
right places (hopefully just one-liners) but that's easier than
implementing lots of devices. And it would give us some
documentation in the source of what's still missing in a board.
thanks
-- PMM
Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write
Am 07.09.2016 um 22:17 schrieb no-re...@patchew.org: Hi, Your series failed automatic build test. Please find the testing commands and their output below. If you have docker installed, you can probably reproduce it locally. [...] GTESTER tests/test-string-output-visitor GTESTER tests/test-qmp-event GTESTER tests/test-opts-visitor GTESTER tests/test-qht-par === OUTPUT END === Abort: command timeout (>3600 seconds) It looks like the test failed because of a timeout after one hour. Whatever caused that timeout, I don't think that my patch did it. Stefan
Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write
Hi, Your series failed automatic build test. Please find the testing commands and their output below. If you have docker installed, you can probably reproduce it locally. Subject: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write Type: series Message-id: 1473275627-20024-1-git-send-email...@weilnetz.de === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc make J=8 docker-test-quick@centos6 make J=8 docker-test-mingw@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/1473275627-20024-1-git-send-email...@weilnetz.de -> patchew/1473275627-20024-1-git-send-email...@weilnetz.de Switched to a new branch 'test' 7b9c08c memory: Use standard log messages for unassigned memory read / write === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into 'dtc'... Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf' BUILD centos6 ARCHIVE qemu.tgz ARCHIVE dtc.tgz COPY RUNNER RUN test-quick in centos6 No C++ compiler available; disabling C++ specific optional code Install prefix/tmp/qemu-test/src/tests/docker/install BIOS directory/tmp/qemu-test/src/tests/docker/install/share/qemu binary directory /tmp/qemu-test/src/tests/docker/install/bin library directory /tmp/qemu-test/src/tests/docker/install/lib module directory /tmp/qemu-test/src/tests/docker/install/lib/qemu libexec directory /tmp/qemu-test/src/tests/docker/install/libexec include directory /tmp/qemu-test/src/tests/docker/install/include config directory /tmp/qemu-test/src/tests/docker/install/etc local state directory /tmp/qemu-test/src/tests/docker/install/var Manual directory /tmp/qemu-test/src/tests/docker/install/share/man ELF interp prefix /usr/gnemul/qemu-%M Source path /tmp/qemu-test/src C compilercc Host C compiler cc C++ compiler Objective-C compiler cc ARFLAGS rv CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -g QEMU_CFLAGS -I/usr/include/pixman-1-fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all LDFLAGS -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g make make install install pythonpython -B smbd /usr/sbin/smbd module supportno host CPU x86_64 host big endian no target list x86_64-softmmu aarch64-softmmu tcg debug enabled no gprof enabled no sparse enabledno strip binariesyes profiler no static build no pixmansystem SDL support yes (1.2.14) GTK support no GTK GL supportno VTE support no TLS priority NORMAL GNUTLS supportno GNUTLS rndno libgcrypt no libgcrypt kdf no nettleno nettle kdfno libtasn1 no curses supportno virgl support no curl support no mingw32 support no Audio drivers oss Block whitelist (rw) Block whitelist (ro) VirtFS supportno VNC support yes VNC SASL support no VNC JPEG support no VNC PNG support no xen support no brlapi supportno bluez supportno Documentation no PIE yes vde support no netmap supportno Linux AIO support no ATTR/XATTR support yes Install blobs yes KVM support yes RDMA support no TCG interpreter no fdt support yes preadv supportyes fdatasync yes madvise yes posix_madvise yes uuid support no libcap-ng support no vhost-net support yes vhost-scsi support yes Trace backendslog spice support no rbd support no xfsctl supportno smartcard support no libusbno usb net redir no OpenGL supportno OpenGL dmabufsno libiscsi support no libnfs supportno build guest agent yes QGA VSS support no QGA w32 disk info no QGA MSI support no seccomp support no coroutine backend ucontext coroutine poolyes GlusterFS support no Archipelago support no gcov gcov gcov enabled no TPM support yes libssh2 support no TPM passthrough yes QOM debugging yes vhdx no lzo support no snappy supportno bzip2 support no NUMA host support no tcmalloc support no jemalloc support no avx2 optimization no GEN x86_64-softmmu/config-devices.mak.tmp GEN aarch64-softmmu/config-devices.mak.tmp GEN conf
[Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write
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
---
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(-)
diff --git a/memory.c b/memory.c
index 0eb6895..92f8879 100644
--- a/memory.c
+++ b/memory.c
@@ -23,6 +23,7 @@
#include "qapi/visitor.h"
#include "qemu/bitops.h"
#include "qemu/error-report.h"
+#include "qemu/log.h"
#include "qom/object.h"
#include "trace.h"
@@ -31,8 +32,6 @@
#include "sysemu/kvm.h"
#include "sysemu/sysemu.h"
-//#define DEBUG_UNASSIGNED
-
static unsigned memory_region_transaction_depth;
static bool memory_region_update_pending;
static bool ioeventfd_update_pending;
@@ -1101,9 +1100,7 @@ static void memory_region_initfn(Object *obj)
static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
unsigned size)
{
-#ifdef DEBUG_UNASSIGNED
-printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
-#endif
+qemu_log_mask(LOG_UNIMP, "%s " TARGET_FMT_plx "\n", __func__, addr);
if (current_cpu != NULL) {
cpu_unassigned_access(current_cpu, addr, false, false, 0, size);
}
@@ -1113,9 +1110,8 @@ static uint64_t unassigned_mem_read(void *opaque, hwaddr
addr,
static void unassigned_mem_write(void *opaque, hwaddr addr,
uint64_t val, unsigned size)
{
-#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);
if (current_cpu != NULL) {
cpu_unassigned_access(current_cpu, addr, true, false, 0, size);
}
--
2.1.4
