[PATCH v3 02/25] system/physmem: Convert DEBUG_SUBPAGE printf() to trace events

Philippe Mathieu-Daudé posted 25 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v3 02/25] system/physmem: Convert DEBUG_SUBPAGE printf() to trace events
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago
Defining DEBUG_SUBPAGE allows to use raw printf() statements to
print information about some events; convert these to tracepoints.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/physmem.c    | 29 ++++++-----------------------
 system/trace-events |  6 ++++++
 2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index 1292f49095f..7e914ecf648 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -91,8 +91,6 @@
 
 #include "memory-internal.h"
 
-//#define DEBUG_SUBPAGE
-
 /* ram_list is read under rcu_read_lock()/rcu_read_unlock().  Writes
  * are protected by the ramlist lock.
  */
@@ -2903,10 +2901,7 @@ static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
     uint8_t buf[8];
     MemTxResult res;
 
-#if defined(DEBUG_SUBPAGE)
-    printf("%s: subpage %p len %u addr " HWADDR_FMT_plx "\n", __func__,
-           subpage, len, addr);
-#endif
+    trace_subpage_read(subpage, len, addr);
     res = flatview_read(subpage->fv, addr + subpage->base, attrs, buf, len);
     if (res) {
         return res;
@@ -2921,11 +2916,7 @@ static MemTxResult subpage_write(void *opaque, hwaddr addr,
     subpage_t *subpage = opaque;
     uint8_t buf[8];
 
-#if defined(DEBUG_SUBPAGE)
-    printf("%s: subpage %p len %u addr " HWADDR_FMT_plx
-           " value %"PRIx64"\n",
-           __func__, subpage, len, addr, value);
-#endif
+    trace_subpage_write(subpage, len, addr, value);
     stn_p(buf, len, value);
     return flatview_write(subpage->fv, addr + subpage->base, attrs, buf, len);
 }
@@ -2935,10 +2926,8 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
                             MemTxAttrs attrs)
 {
     subpage_t *subpage = opaque;
-#if defined(DEBUG_SUBPAGE)
-    printf("%s: subpage %p %c len %u addr " HWADDR_FMT_plx "\n",
-           __func__, subpage, is_write ? 'w' : 'r', len, addr);
-#endif
+
+    trace_subpage_accepts(subpage, is_write ? 'w' : 'r', len, addr);
 
     return flatview_access_valid(subpage->fv, addr + subpage->base,
                                  len, is_write, attrs);
@@ -2964,10 +2953,7 @@ static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end,
         return -1;
     idx = SUBPAGE_IDX(start);
     eidx = SUBPAGE_IDX(end);
-#if defined(DEBUG_SUBPAGE)
-    printf("%s: %p start %08x end %08x idx %08x eidx %08x section %d\n",
-           __func__, mmio, start, end, idx, eidx, section);
-#endif
+    trace_subpage_register(mmio, start, end, idx, eidx, section);
     for (; idx <= eidx; idx++) {
         mmio->sub_section[idx] = section;
     }
@@ -2986,10 +2972,7 @@ static subpage_t *subpage_init(FlatView *fv, hwaddr base)
     memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
                           NULL, TARGET_PAGE_SIZE);
     mmio->iomem.subpage = true;
-#if defined(DEBUG_SUBPAGE)
-    printf("%s: %p base " HWADDR_FMT_plx " len %08x\n", __func__,
-           mmio, base, TARGET_PAGE_SIZE);
-#endif
+    trace_subpage_init(mmio, base, TARGET_PAGE_SIZE);
 
     return mmio;
 }
diff --git a/system/trace-events b/system/trace-events
index 82856e44f2e..6d29a823f04 100644
--- a/system/trace-events
+++ b/system/trace-events
@@ -35,6 +35,12 @@ find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset, uint64_
 ram_block_discard_range(const char *rbname, void *hva, size_t length, bool need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d fallocate: %d ret: %d"
 qemu_ram_alloc_shared(const char *name, size_t size, size_t max_size, int fd, void *host) "%s size %zu max_size %zu fd %d host %p"
 
+subpage_register(void *subpage, uint32_t start, uint32_t end, int idx, int eidx, uint16_t section) "subpage %p start 0x%08x end 0x%08x idx 0x%08x eidx 0x%08x section %u"
+subpage_init(void *subpage, uint64_t base, uint64_t len) "subpage %p base 0x%08" PRIx64 " len 0x%08" PRIx64
+subpage_accepts(void *subpage, char access, unsigned len, uint64_t addr) "subpage %p %c len %u addr 0x%" PRIx64
+subpage_read(void *subpage, unsigned len, uint64_t addr) "subpage %p len %u addr 0x%" PRIx64
+subpage_write(void *subpage, unsigned len, uint64_t addr, uint64_t value) "subpage %p len %u addr 0x%" PRIx64 " value 0x%" PRIx64
+
 # cpus.c
 vm_stop_flush_all(int ret) "ret %d"
 
-- 
2.52.0


Re: [PATCH v3 02/25] system/physmem: Convert DEBUG_SUBPAGE printf() to trace events
Posted by Richard Henderson 1 month, 1 week ago
On 12/25/25 02:21, Philippe Mathieu-Daudé wrote:
> Defining DEBUG_SUBPAGE allows to use raw printf() statements to
> print information about some events; convert these to tracepoints.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/physmem.c    | 29 ++++++-----------------------
>   system/trace-events |  6 ++++++
>   2 files changed, 12 insertions(+), 23 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 02/25] system/physmem: Convert DEBUG_SUBPAGE printf() to trace events
Posted by Manos Pitsidianakis 1 month, 2 weeks ago
On Wed, Dec 24, 2025 at 5:22 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Defining DEBUG_SUBPAGE allows to use raw printf() statements to
> print information about some events; convert these to tracepoints.
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>


>  system/physmem.c    | 29 ++++++-----------------------
>  system/trace-events |  6 ++++++
>  2 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/system/physmem.c b/system/physmem.c
> index 1292f49095f..7e914ecf648 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -91,8 +91,6 @@
>
>  #include "memory-internal.h"
>
> -//#define DEBUG_SUBPAGE
> -
>  /* ram_list is read under rcu_read_lock()/rcu_read_unlock().  Writes
>   * are protected by the ramlist lock.
>   */
> @@ -2903,10 +2901,7 @@ static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
>      uint8_t buf[8];
>      MemTxResult res;
>
> -#if defined(DEBUG_SUBPAGE)
> -    printf("%s: subpage %p len %u addr " HWADDR_FMT_plx "\n", __func__,
> -           subpage, len, addr);
> -#endif
> +    trace_subpage_read(subpage, len, addr);
>      res = flatview_read(subpage->fv, addr + subpage->base, attrs, buf, len);
>      if (res) {
>          return res;
> @@ -2921,11 +2916,7 @@ static MemTxResult subpage_write(void *opaque, hwaddr addr,
>      subpage_t *subpage = opaque;
>      uint8_t buf[8];
>
> -#if defined(DEBUG_SUBPAGE)
> -    printf("%s: subpage %p len %u addr " HWADDR_FMT_plx
> -           " value %"PRIx64"\n",
> -           __func__, subpage, len, addr, value);
> -#endif
> +    trace_subpage_write(subpage, len, addr, value);
>      stn_p(buf, len, value);
>      return flatview_write(subpage->fv, addr + subpage->base, attrs, buf, len);
>  }
> @@ -2935,10 +2926,8 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
>                              MemTxAttrs attrs)
>  {
>      subpage_t *subpage = opaque;
> -#if defined(DEBUG_SUBPAGE)
> -    printf("%s: subpage %p %c len %u addr " HWADDR_FMT_plx "\n",
> -           __func__, subpage, is_write ? 'w' : 'r', len, addr);
> -#endif
> +
> +    trace_subpage_accepts(subpage, is_write ? 'w' : 'r', len, addr);
>
>      return flatview_access_valid(subpage->fv, addr + subpage->base,
>                                   len, is_write, attrs);
> @@ -2964,10 +2953,7 @@ static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end,
>          return -1;
>      idx = SUBPAGE_IDX(start);
>      eidx = SUBPAGE_IDX(end);
> -#if defined(DEBUG_SUBPAGE)
> -    printf("%s: %p start %08x end %08x idx %08x eidx %08x section %d\n",
> -           __func__, mmio, start, end, idx, eidx, section);
> -#endif
> +    trace_subpage_register(mmio, start, end, idx, eidx, section);
>      for (; idx <= eidx; idx++) {
>          mmio->sub_section[idx] = section;
>      }
> @@ -2986,10 +2972,7 @@ static subpage_t *subpage_init(FlatView *fv, hwaddr base)
>      memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
>                            NULL, TARGET_PAGE_SIZE);
>      mmio->iomem.subpage = true;
> -#if defined(DEBUG_SUBPAGE)
> -    printf("%s: %p base " HWADDR_FMT_plx " len %08x\n", __func__,
> -           mmio, base, TARGET_PAGE_SIZE);
> -#endif
> +    trace_subpage_init(mmio, base, TARGET_PAGE_SIZE);
>
>      return mmio;
>  }
> diff --git a/system/trace-events b/system/trace-events
> index 82856e44f2e..6d29a823f04 100644
> --- a/system/trace-events
> +++ b/system/trace-events
> @@ -35,6 +35,12 @@ find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset, uint64_
>  ram_block_discard_range(const char *rbname, void *hva, size_t length, bool need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d fallocate: %d ret: %d"
>  qemu_ram_alloc_shared(const char *name, size_t size, size_t max_size, int fd, void *host) "%s size %zu max_size %zu fd %d host %p"
>
> +subpage_register(void *subpage, uint32_t start, uint32_t end, int idx, int eidx, uint16_t section) "subpage %p start 0x%08x end 0x%08x idx 0x%08x eidx 0x%08x section %u"
> +subpage_init(void *subpage, uint64_t base, uint64_t len) "subpage %p base 0x%08" PRIx64 " len 0x%08" PRIx64
> +subpage_accepts(void *subpage, char access, unsigned len, uint64_t addr) "subpage %p %c len %u addr 0x%" PRIx64
> +subpage_read(void *subpage, unsigned len, uint64_t addr) "subpage %p len %u addr 0x%" PRIx64
> +subpage_write(void *subpage, unsigned len, uint64_t addr, uint64_t value) "subpage %p len %u addr 0x%" PRIx64 " value 0x%" PRIx64
> +
>  # cpus.c
>  vm_stop_flush_all(int ret) "ret %d"
>
> --
> 2.52.0
>