qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Laurent Desnogues" <laurent.desnogues@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>
Subject: [PATCH] memory: Replace DEBUG_UNASSIGNED printf calls by trace events
Date: Fri, 20 Sep 2019 16:12:48 +0200	[thread overview]
Message-ID: <20190920141248.12887-1-philmd@redhat.com> (raw)

Now that the unassigned_access CPU hooks have been removed,
the unassigned_mem_read/write functions are only used for
debugging purpose.
Simplify by converting them to in-place trace events.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Based-on: <20190920125008.13604-1-peter.maydell@linaro.org>
https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg04668.html
https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg03705.html

I first wrote:

  These functions are declared using the CPUReadMemoryFunc/
  CPUWriteMemoryFunc prototypes. Since it is confusing to
  have such prototype only use for debugging, convert them
  to in-place trace events.

But it doesn't provide helpful information and is rather confusing.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 memory.c     | 24 +++---------------------
 trace-events |  2 ++
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/memory.c b/memory.c
index 93a05395cf..07e80a637a 100644
--- a/memory.c
+++ b/memory.c
@@ -35,8 +35,6 @@
 #include "hw/boards.h"
 #include "migration/vmstate.h"
 
-//#define DEBUG_UNASSIGNED
-
 static unsigned memory_region_transaction_depth;
 static bool memory_region_update_pending;
 static bool ioeventfd_update_pending;
@@ -1272,23 +1270,6 @@ static void iommu_memory_region_initfn(Object *obj)
     mr->is_iommu = true;
 }
 
-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
-    return 0;
-}
-
-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
-}
-
 static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
                                    unsigned size, bool is_write,
                                    MemTxAttrs attrs)
@@ -1437,7 +1418,8 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
     MemTxResult r;
 
     if (!memory_region_access_valid(mr, addr, size, false, attrs)) {
-        *pval = unassigned_mem_read(mr, addr, size);
+        trace_memory_region_invalid_read(size, addr);
+        *pval = 0; /* FIXME now this value shouldn't be accessed in guest */
         return MEMTX_DECODE_ERROR;
     }
 
@@ -1481,7 +1463,7 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
     unsigned size = memop_size(op);
 
     if (!memory_region_access_valid(mr, addr, size, true, attrs)) {
-        unassigned_mem_write(mr, addr, data, size);
+        trace_memory_region_invalid_write(size, addr, size << 1, data);
         return MEMTX_DECODE_ERROR;
     }
 
diff --git a/trace-events b/trace-events
index 823a4ae64e..83dbeb4b46 100644
--- a/trace-events
+++ b/trace-events
@@ -62,6 +62,8 @@ memory_region_tb_read(int cpu_index, uint64_t addr, uint64_t value, unsigned siz
 memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
+memory_region_invalid_read(unsigned size, uint64_t addr) "invalid read size %u addr 0x%"PRIx64
+memory_region_invalid_write(unsigned size, uint64_t addr, int fmt_width, uint64_t value) "invalid write size %u addr 0x%"PRIx64" value 0x%0*"PRIx64
 flatview_new(void *view, void *root) "%p (root %p)"
 flatview_destroy(void *view, void *root) "%p (root %p)"
 flatview_destroy_rcu(void *view, void *root) "%p (root %p)"
-- 
2.20.1



             reply	other threads:[~2019-09-20 14:19 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-20 14:12 Philippe Mathieu-Daudé [this message]
2019-09-20 14:17 ` [PATCH] memory: Replace DEBUG_UNASSIGNED printf calls by trace events Paolo Bonzini
2019-09-20 14:20   ` Peter Maydell
2019-10-08 20:40     ` Palmer Dabbelt
2019-10-08 20:41       ` Alistair Francis
2019-10-08 20:52         ` Alistair Francis
2019-09-20 14:19 ` Peter Maydell
2019-09-20 14:29   ` Philippe Mathieu-Daudé
2019-09-20 14:35     ` Peter Maydell
2019-09-20 14:39       ` Philippe Mathieu-Daudé
2019-09-20 15:01         ` Eric Blake
2019-09-20 16:03           ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190920141248.12887-1-philmd@redhat.com \
    --to=philmd@redhat.com \
    --cc=laurent.desnogues@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).