qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg
@ 2023-12-08  0:35 Ilya Leoshkevich
  2023-12-08  0:35 ` [PATCH v3 1/4] target: Make qemu_target_page_mask() available for *-user Ilya Leoshkevich
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Ilya Leoshkevich @ 2023-12-08  0:35 UTC (permalink / raw)
  To: Richard Henderson, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé
  Cc: qemu-devel, Ilya Leoshkevich

v2: https://patchew.org/QEMU/20230630234230.596193-1-iii@linux.ibm.com/
v2 -> v3: Rebased.
          This series was lost and forgotten until Philippe reminded me
          about it.

v1: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg07037.html
v1 -> v2: Move qemu_target_page_mask() hunk to patch 1.
          Fix typos.

Hi,

This series is a follow-up to discussion in [1]; the goal is to build
perf and debuginfo support only one time.

Best regards,
Ilya

[1] https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg06510.html

Ilya Leoshkevich (4):
  target: Make qemu_target_page_mask() available for *-user
  tcg: Make tb_cflags() usable from target-agnostic code
  accel/tcg: Remove #ifdef TARGET_I386 from perf.c
  accel/tcg: Move perf and debuginfo support to tcg

 accel/tcg/meson.build            |  2 --
 accel/tcg/translate-all.c        |  2 +-
 hw/core/loader.c                 |  2 +-
 include/exec/exec-all.h          |  6 ------
 include/exec/translation-block.h |  6 ++++++
 linux-user/elfload.c             |  2 +-
 linux-user/exit.c                |  2 +-
 linux-user/main.c                |  2 +-
 system/physmem.c                 |  5 -----
 system/vl.c                      |  2 +-
 target/meson.build               |  2 ++
 target/target-common.c           | 10 ++++++++++
 {accel/tcg => tcg}/debuginfo.c   |  0
 {accel/tcg => tcg}/debuginfo.h   |  4 ++--
 tcg/meson.build                  |  3 +++
 {accel/tcg => tcg}/perf.c        |  9 +++------
 {accel/tcg => tcg}/perf.h        |  4 ++--
 tcg/tcg.c                        |  2 +-
 18 files changed, 35 insertions(+), 30 deletions(-)
 create mode 100644 target/target-common.c
 rename {accel/tcg => tcg}/debuginfo.c (100%)
 rename {accel/tcg => tcg}/debuginfo.h (96%)
 rename {accel/tcg => tcg}/perf.c (98%)
 rename {accel/tcg => tcg}/perf.h (95%)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/4] target: Make qemu_target_page_mask() available for *-user
  2023-12-08  0:35 [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
@ 2023-12-08  0:35 ` Ilya Leoshkevich
  2023-12-08 10:58   ` Philippe Mathieu-Daudé
  2023-12-08  0:35 ` [PATCH v3 2/4] tcg: Make tb_cflags() usable from target-agnostic code Ilya Leoshkevich
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ilya Leoshkevich @ 2023-12-08  0:35 UTC (permalink / raw)
  To: Richard Henderson, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé
  Cc: qemu-devel, Ilya Leoshkevich

Currently qemu_target_page_mask() is usable only from the softmmu
code. Make it possible to use it from the *-user code as well.

Make use of it in perf.c.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 accel/tcg/perf.c       |  3 ++-
 system/physmem.c       |  5 -----
 target/meson.build     |  2 ++
 target/target-common.c | 10 ++++++++++
 4 files changed, 14 insertions(+), 6 deletions(-)
 create mode 100644 target/target-common.c

diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
index cd1aa99a7ee..ba75c1bbe45 100644
--- a/accel/tcg/perf.c
+++ b/accel/tcg/perf.c
@@ -10,6 +10,7 @@
 
 #include "qemu/osdep.h"
 #include "elf.h"
+#include "exec/target_page.h"
 #include "exec/exec-all.h"
 #include "qemu/timer.h"
 #include "tcg/tcg.h"
@@ -335,7 +336,7 @@ void perf_report_code(uint64_t guest_pc, TranslationBlock *tb,
         /* FIXME: This replicates the restore_state_to_opc() logic. */
         q[insn].address = gen_insn_data[insn * start_words + 0];
         if (tb_cflags(tb) & CF_PCREL) {
-            q[insn].address |= (guest_pc & TARGET_PAGE_MASK);
+            q[insn].address |= (guest_pc & qemu_target_page_mask());
         } else {
 #if defined(TARGET_I386)
             q[insn].address -= tb->cs_base;
diff --git a/system/physmem.c b/system/physmem.c
index a63853a7bc9..7cf4a735c3b 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3431,11 +3431,6 @@ size_t qemu_target_page_size(void)
     return TARGET_PAGE_SIZE;
 }
 
-int qemu_target_page_mask(void)
-{
-    return TARGET_PAGE_MASK;
-}
-
 int qemu_target_page_bits(void)
 {
     return TARGET_PAGE_BITS;
diff --git a/target/meson.build b/target/meson.build
index a53a60486fc..dee2ac47e02 100644
--- a/target/meson.build
+++ b/target/meson.build
@@ -19,3 +19,5 @@ subdir('sh4')
 subdir('sparc')
 subdir('tricore')
 subdir('xtensa')
+
+specific_ss.add(files('target-common.c'))
diff --git a/target/target-common.c b/target/target-common.c
new file mode 100644
index 00000000000..903b10cfe4b
--- /dev/null
+++ b/target/target-common.c
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#include "qemu/osdep.h"
+
+#include "cpu.h"
+#include "exec/target_page.h"
+
+int qemu_target_page_mask(void)
+{
+    return TARGET_PAGE_MASK;
+}
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/4] tcg: Make tb_cflags() usable from target-agnostic code
  2023-12-08  0:35 [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
  2023-12-08  0:35 ` [PATCH v3 1/4] target: Make qemu_target_page_mask() available for *-user Ilya Leoshkevich
@ 2023-12-08  0:35 ` Ilya Leoshkevich
  2023-12-11 18:47   ` Richard Henderson
  2023-12-08  0:35 ` [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c Ilya Leoshkevich
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ilya Leoshkevich @ 2023-12-08  0:35 UTC (permalink / raw)
  To: Richard Henderson, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé
  Cc: qemu-devel, Ilya Leoshkevich

Currently tb_cflags() is defined in exec-all.h, which is not usable
from target-agnostic code. Move it to translation-block.h, which is.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 include/exec/exec-all.h          | 6 ------
 include/exec/translation-block.h | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index ee90ef122bc..8d615b60c14 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -459,12 +459,6 @@ int probe_access_full_mmu(CPUArchState *env, vaddr addr, int size,
 
 #endif
 
-/* Hide the qatomic_read to make code a little easier on the eyes */
-static inline uint32_t tb_cflags(const TranslationBlock *tb)
-{
-    return qatomic_read(&tb->cflags);
-}
-
 static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb)
 {
 #ifdef CONFIG_USER_ONLY
diff --git a/include/exec/translation-block.h b/include/exec/translation-block.h
index e2b26e16da1..48211c890a7 100644
--- a/include/exec/translation-block.h
+++ b/include/exec/translation-block.h
@@ -145,4 +145,10 @@ struct TranslationBlock {
 /* The alignment given to TranslationBlock during allocation. */
 #define CODE_GEN_ALIGN  16
 
+/* Hide the qatomic_read to make code a little easier on the eyes */
+static inline uint32_t tb_cflags(const TranslationBlock *tb)
+{
+    return qatomic_read(&tb->cflags);
+}
+
 #endif /* EXEC_TRANSLATION_BLOCK_H */
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c
  2023-12-08  0:35 [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
  2023-12-08  0:35 ` [PATCH v3 1/4] target: Make qemu_target_page_mask() available for *-user Ilya Leoshkevich
  2023-12-08  0:35 ` [PATCH v3 2/4] tcg: Make tb_cflags() usable from target-agnostic code Ilya Leoshkevich
@ 2023-12-08  0:35 ` Ilya Leoshkevich
  2023-12-08  9:53   ` Alex Bennée
  2023-12-11 18:49   ` Richard Henderson
  2023-12-08  0:35 ` [PATCH v3 4/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
  2023-12-08 11:01 ` [PATCH v3 0/4] " Philippe Mathieu-Daudé
  4 siblings, 2 replies; 12+ messages in thread
From: Ilya Leoshkevich @ 2023-12-08  0:35 UTC (permalink / raw)
  To: Richard Henderson, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé
  Cc: qemu-devel, Ilya Leoshkevich

Preparation for moving perf.c to tcg/.

This affects only profiling guest code, which has code in a non-0 based
segment, e.g., 16-bit code, which is not particularly important.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 accel/tcg/perf.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
index ba75c1bbe45..68a46b1b524 100644
--- a/accel/tcg/perf.c
+++ b/accel/tcg/perf.c
@@ -337,10 +337,6 @@ void perf_report_code(uint64_t guest_pc, TranslationBlock *tb,
         q[insn].address = gen_insn_data[insn * start_words + 0];
         if (tb_cflags(tb) & CF_PCREL) {
             q[insn].address |= (guest_pc & qemu_target_page_mask());
-        } else {
-#if defined(TARGET_I386)
-            q[insn].address -= tb->cs_base;
-#endif
         }
         q[insn].flags = DEBUGINFO_SYMBOL | (jitdump ? DEBUGINFO_LINE : 0);
     }
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/4] accel/tcg: Move perf and debuginfo support to tcg
  2023-12-08  0:35 [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
                   ` (2 preceding siblings ...)
  2023-12-08  0:35 ` [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c Ilya Leoshkevich
@ 2023-12-08  0:35 ` Ilya Leoshkevich
  2023-12-11 18:52   ` Richard Henderson
  2023-12-08 11:01 ` [PATCH v3 0/4] " Philippe Mathieu-Daudé
  4 siblings, 1 reply; 12+ messages in thread
From: Ilya Leoshkevich @ 2023-12-08  0:35 UTC (permalink / raw)
  To: Richard Henderson, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé
  Cc: qemu-devel, Ilya Leoshkevich

tcg/ should not depend on accel/tcg/, but perf and debuginfo
support provided by the latter are being used by tcg/tcg.c.

Since that's the only user, move both to tcg/.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 accel/tcg/meson.build          | 2 --
 accel/tcg/translate-all.c      | 2 +-
 hw/core/loader.c               | 2 +-
 linux-user/elfload.c           | 2 +-
 linux-user/exit.c              | 2 +-
 linux-user/main.c              | 2 +-
 system/vl.c                    | 2 +-
 {accel/tcg => tcg}/debuginfo.c | 0
 {accel/tcg => tcg}/debuginfo.h | 4 ++--
 tcg/meson.build                | 3 +++
 {accel/tcg => tcg}/perf.c      | 2 +-
 {accel/tcg => tcg}/perf.h      | 4 ++--
 tcg/tcg.c                      | 2 +-
 13 files changed, 15 insertions(+), 14 deletions(-)
 rename {accel/tcg => tcg}/debuginfo.c (100%)
 rename {accel/tcg => tcg}/debuginfo.h (96%)
 rename {accel/tcg => tcg}/perf.c (99%)
 rename {accel/tcg => tcg}/perf.h (95%)

diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index 8783edd06ee..a7cb724edb2 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -16,8 +16,6 @@ tcg_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_false: files('user-exec-stub.c'))
 if get_option('plugins')
   tcg_ss.add(files('plugin-gen.c'))
 endif
-tcg_ss.add(when: libdw, if_true: files('debuginfo.c'))
-tcg_ss.add(when: 'CONFIG_LINUX', if_true: files('perf.c'))
 specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_ss)
 
 specific_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files(
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 79a88f5fb75..3c1ce69ff36 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -63,7 +63,7 @@
 #include "tb-context.h"
 #include "internal-common.h"
 #include "internal-target.h"
-#include "perf.h"
+#include "tcg/perf.h"
 #include "tcg/insn-start-words.h"
 
 TBContext tb_ctx;
diff --git a/hw/core/loader.c b/hw/core/loader.c
index e7a9b3775bb..b8e52f3fb0f 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -62,7 +62,7 @@
 #include "hw/boards.h"
 #include "qemu/cutils.h"
 #include "sysemu/runstate.h"
-#include "accel/tcg/debuginfo.h"
+#include "tcg/debuginfo.h"
 
 #include <zlib.h>
 
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index cf9e74468b1..62120c76151 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -21,7 +21,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "target_signal.h"
-#include "accel/tcg/debuginfo.h"
+#include "tcg/debuginfo.h"
 
 #ifdef TARGET_ARM
 #include "target/arm/cpu-features.h"
diff --git a/linux-user/exit.c b/linux-user/exit.c
index 50266314e0a..1ff8fe4f072 100644
--- a/linux-user/exit.c
+++ b/linux-user/exit.c
@@ -17,7 +17,7 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
-#include "accel/tcg/perf.h"
+#include "tcg/perf.h"
 #include "gdbstub/syscalls.h"
 #include "qemu.h"
 #include "user-internals.h"
diff --git a/linux-user/main.c b/linux-user/main.c
index 0cdaf30d346..2e075ae1e69 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -54,7 +54,7 @@
 #include "signal-common.h"
 #include "loader.h"
 #include "user-mmap.h"
-#include "accel/tcg/perf.h"
+#include "tcg/perf.h"
 
 #ifdef CONFIG_SEMIHOSTING
 #include "semihosting/semihost.h"
diff --git a/system/vl.c b/system/vl.c
index 2bcd9efb9a6..56baa1c81f2 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -96,7 +96,7 @@
 #endif
 #include "sysemu/qtest.h"
 #ifdef CONFIG_TCG
-#include "accel/tcg/perf.h"
+#include "tcg/perf.h"
 #endif
 
 #include "disas/disas.h"
diff --git a/accel/tcg/debuginfo.c b/tcg/debuginfo.c
similarity index 100%
rename from accel/tcg/debuginfo.c
rename to tcg/debuginfo.c
diff --git a/accel/tcg/debuginfo.h b/tcg/debuginfo.h
similarity index 96%
rename from accel/tcg/debuginfo.h
rename to tcg/debuginfo.h
index f064e1c144b..858535b5da5 100644
--- a/accel/tcg/debuginfo.h
+++ b/tcg/debuginfo.h
@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
-#ifndef ACCEL_TCG_DEBUGINFO_H
-#define ACCEL_TCG_DEBUGINFO_H
+#ifndef TCG_DEBUGINFO_H
+#define TCG_DEBUGINFO_H
 
 #include "qemu/bitops.h"
 
diff --git a/tcg/meson.build b/tcg/meson.build
index 895a11d3fa2..28a699b4228 100644
--- a/tcg/meson.build
+++ b/tcg/meson.build
@@ -22,6 +22,9 @@ if get_option('tcg_interpreter')
   tcg_ss.add(files('tci.c'))
 endif
 
+tcg_ss.add(when: libdw, if_true: files('debuginfo.c'))
+tcg_ss.add(when: 'CONFIG_LINUX', if_true: files('perf.c'))
+
 tcg_ss = tcg_ss.apply(config_targetos, strict: false)
 
 libtcg_user = static_library('tcg_user',
diff --git a/accel/tcg/perf.c b/tcg/perf.c
similarity index 99%
rename from accel/tcg/perf.c
rename to tcg/perf.c
index 68a46b1b524..de34248d92d 100644
--- a/accel/tcg/perf.c
+++ b/tcg/perf.c
@@ -11,7 +11,7 @@
 #include "qemu/osdep.h"
 #include "elf.h"
 #include "exec/target_page.h"
-#include "exec/exec-all.h"
+#include "exec/translation-block.h"
 #include "qemu/timer.h"
 #include "tcg/tcg.h"
 
diff --git a/accel/tcg/perf.h b/tcg/perf.h
similarity index 95%
rename from accel/tcg/perf.h
rename to tcg/perf.h
index f92dd52c699..c96b5920a3f 100644
--- a/accel/tcg/perf.h
+++ b/tcg/perf.h
@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
-#ifndef ACCEL_TCG_PERF_H
-#define ACCEL_TCG_PERF_H
+#ifndef TCG_PERF_H
+#define TCG_PERF_H
 
 #if defined(CONFIG_TCG) && defined(CONFIG_LINUX)
 /* Start writing perf-<pid>.map. */
diff --git a/tcg/tcg.c b/tcg/tcg.c
index d2ea22b397f..1a15a2a7c52 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -55,7 +55,7 @@
 #include "tcg/tcg-ldst.h"
 #include "tcg/tcg-temp-internal.h"
 #include "tcg-internal.h"
-#include "accel/tcg/perf.h"
+#include "tcg/perf.h"
 #ifdef CONFIG_USER_ONLY
 #include "exec/user/guest-base.h"
 #endif
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c
  2023-12-08  0:35 ` [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c Ilya Leoshkevich
@ 2023-12-08  9:53   ` Alex Bennée
  2023-12-08 10:35     ` Ilya Leoshkevich
  2023-12-11 18:49   ` Richard Henderson
  1 sibling, 1 reply; 12+ messages in thread
From: Alex Bennée @ 2023-12-08  9:53 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Richard Henderson, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé, qemu-devel

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> Preparation for moving perf.c to tcg/.
>
> This affects only profiling guest code, which has code in a non-0 based
> segment, e.g., 16-bit code, which is not particularly important.

I have no objection to removing the wart. Is it worth adding a note:: to
tcg.rst to say that profiles of 16-bit x64 code will be junk?

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  accel/tcg/perf.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
> index ba75c1bbe45..68a46b1b524 100644
> --- a/accel/tcg/perf.c
> +++ b/accel/tcg/perf.c
> @@ -337,10 +337,6 @@ void perf_report_code(uint64_t guest_pc, TranslationBlock *tb,
>          q[insn].address = gen_insn_data[insn * start_words + 0];
>          if (tb_cflags(tb) & CF_PCREL) {
>              q[insn].address |= (guest_pc & qemu_target_page_mask());
> -        } else {
> -#if defined(TARGET_I386)
> -            q[insn].address -= tb->cs_base;
> -#endif
>          }
>          q[insn].flags = DEBUGINFO_SYMBOL | (jitdump ? DEBUGINFO_LINE : 0);
>      }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c
  2023-12-08  9:53   ` Alex Bennée
@ 2023-12-08 10:35     ` Ilya Leoshkevich
  0 siblings, 0 replies; 12+ messages in thread
From: Ilya Leoshkevich @ 2023-12-08 10:35 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Richard Henderson, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé, qemu-devel

On Fri, 2023-12-08 at 09:53 +0000, Alex Bennée wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
> > Preparation for moving perf.c to tcg/.
> > 
> > This affects only profiling guest code, which has code in a non-0
> > based
> > segment, e.g., 16-bit code, which is not particularly important.
> 
> I have no objection to removing the wart. Is it worth adding a note::
> to
> tcg.rst to say that profiles of 16-bit x64 code will be junk?
> 
> Anyway:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

I haven't tried them, but I don't think they work well anyway. With the
current logic, the samples are assigned to ip (without cs!), so if
there is code in different segments, there will be, to put it mildly,
ambiguities. Come to think of it, the patch even improves the
situation, since now the samples are assigned to cs*16+ip. In any
case, I don't think there is any good tooling to work with 16-bit
profiles.

> > Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  accel/tcg/perf.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
> > index ba75c1bbe45..68a46b1b524 100644
> > --- a/accel/tcg/perf.c
> > +++ b/accel/tcg/perf.c
> > @@ -337,10 +337,6 @@ void perf_report_code(uint64_t guest_pc,
> > TranslationBlock *tb,
> >          q[insn].address = gen_insn_data[insn * start_words + 0];
> >          if (tb_cflags(tb) & CF_PCREL) {
> >              q[insn].address |= (guest_pc &
> > qemu_target_page_mask());
> > -        } else {
> > -#if defined(TARGET_I386)
> > -            q[insn].address -= tb->cs_base;
> > -#endif
> >          }
> >          q[insn].flags = DEBUGINFO_SYMBOL | (jitdump ?
> > DEBUGINFO_LINE : 0);
> >      }
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/4] target: Make qemu_target_page_mask() available for *-user
  2023-12-08  0:35 ` [PATCH v3 1/4] target: Make qemu_target_page_mask() available for *-user Ilya Leoshkevich
@ 2023-12-08 10:58   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-12-08 10:58 UTC (permalink / raw)
  To: Ilya Leoshkevich, Richard Henderson, Paolo Bonzini,
	Laurent Vivier, Peter Xu, David Hildenbrand
  Cc: qemu-devel

On 8/12/23 01:35, Ilya Leoshkevich wrote:
> Currently qemu_target_page_mask() is usable only from the softmmu
> code. Make it possible to use it from the *-user code as well.
> 
> Make use of it in perf.c.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   accel/tcg/perf.c       |  3 ++-
>   system/physmem.c       |  5 -----
>   target/meson.build     |  2 ++
>   target/target-common.c | 10 ++++++++++
>   4 files changed, 14 insertions(+), 6 deletions(-)
>   create mode 100644 target/target-common.c


> diff --git a/target/meson.build b/target/meson.build
> index a53a60486fc..dee2ac47e02 100644
> --- a/target/meson.build
> +++ b/target/meson.build
> @@ -19,3 +19,5 @@ subdir('sh4')
>   subdir('sparc')
>   subdir('tricore')
>   subdir('xtensa')
> +
> +specific_ss.add(files('target-common.c'))
> diff --git a/target/target-common.c b/target/target-common.c
> new file mode 100644
> index 00000000000..903b10cfe4b
> --- /dev/null
> +++ b/target/target-common.c
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#include "qemu/osdep.h"
> +
> +#include "cpu.h"
> +#include "exec/target_page.h"
> +
> +int qemu_target_page_mask(void)
> +{
> +    return TARGET_PAGE_MASK;
> +}

FYI I carry this patch and am going to post it soon:

-- >8 --
diff --git a/meson.build b/meson.build
index d2c4c2adb3..5fdc4ef8db 100644
--- a/meson.build
+++ b/meson.build
@@ -3488,7 +3488,7 @@ if get_option('b_lto')
    pagevary = declare_dependency(link_with: pagevary)
  endif
  common_ss.add(pagevary)
-specific_ss.add(files('page-vary-target.c'))
+specific_ss.add(files('page-target.c', 'page-vary-target.c'))

  subdir('backends')
  subdir('disas')
diff --git a/page-target.c b/page-target.c
new file mode 100644
index 0000000000..d286e2d58b
--- /dev/null
+++ b/page-target.c
@@ -0,0 +1,43 @@
+/*
+ * QEMU page values getters (target independent)
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "exec/target_page.h"
+#include "exec/cpu-defs.h"
+#include "exec/cpu-all.h"
+
+size_t qemu_target_page_size(void)
+{
+    return TARGET_PAGE_SIZE;
+}
+
+int qemu_target_page_mask(void)
+{
+    return TARGET_PAGE_MASK;
+}
+
+int qemu_target_page_bits(void)
+{
+    return TARGET_PAGE_BITS;
+}
+
+int qemu_target_page_bits_min(void)
+{
+    return TARGET_PAGE_BITS_MIN;
+}
+
+/* Convert target pages to MiB (2**20). */
+size_t qemu_target_pages_to_MiB(size_t pages)
+{
+    int page_bits = TARGET_PAGE_BITS;
+
+    /* So far, the largest (non-huge) page size is 64k, i.e. 16 bits. */
+    g_assert(page_bits < 20);
+
+    return pages >> (20 - page_bits);
+}
diff --git a/system/physmem.c b/system/physmem.c
index a63853a7bc..4bdb3d0592 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3422,41 +3422,6 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
      return 0;
  }

-/*
- * Allows code that needs to deal with migration bitmaps etc to still 
be built
- * target independent.
- */
-size_t qemu_target_page_size(void)
-{
-    return TARGET_PAGE_SIZE;
-}
-
-int qemu_target_page_mask(void)
-{
-    return TARGET_PAGE_MASK;
-}
-
-int qemu_target_page_bits(void)
-{
-    return TARGET_PAGE_BITS;
-}
-
-int qemu_target_page_bits_min(void)
-{
-    return TARGET_PAGE_BITS_MIN;
-}
-
-/* Convert target pages to MiB (2**20). */
-size_t qemu_target_pages_to_MiB(size_t pages)
-{
-    int page_bits = TARGET_PAGE_BITS;
-
-    /* So far, the largest (non-huge) page size is 64k, i.e. 16 bits. */
-    g_assert(page_bits < 20);
-
-    return pages >> (20 - page_bits);
-}
-
  bool cpu_physical_memory_is_io(hwaddr phys_addr)
  {
      MemoryRegion*mr;
---


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg
  2023-12-08  0:35 [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
                   ` (3 preceding siblings ...)
  2023-12-08  0:35 ` [PATCH v3 4/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
@ 2023-12-08 11:01 ` Philippe Mathieu-Daudé
  4 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-12-08 11:01 UTC (permalink / raw)
  To: Ilya Leoshkevich, Richard Henderson
  Cc: qemu-devel, David Hildenbrand, Peter Xu, Paolo Bonzini,
	Laurent Vivier

Hi Richard,

On 8/12/23 01:35, Ilya Leoshkevich wrote:
> v2: https://patchew.org/QEMU/20230630234230.596193-1-iii@linux.ibm.com/
> v2 -> v3: Rebased.
>            This series was lost and forgotten until Philippe reminded me
>            about it.
> 
> v1: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg07037.html
> v1 -> v2: Move qemu_target_page_mask() hunk to patch 1.
>            Fix typos.
> 
> Hi,
> 
> This series is a follow-up to discussion in [1]; the goal is to build
> perf and debuginfo support only one time.
> 
> Best regards,
> Ilya
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg06510.html
> 
> Ilya Leoshkevich (4):
>    target: Make qemu_target_page_mask() available for *-user
>    tcg: Make tb_cflags() usable from target-agnostic code
>    accel/tcg: Remove #ifdef TARGET_I386 from perf.c
>    accel/tcg: Move perf and debuginfo support to tcg

If you R-b/A-b this series, I'd rather integrate it in my exec/ rework.

Regards,

Phil.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/4] tcg: Make tb_cflags() usable from target-agnostic code
  2023-12-08  0:35 ` [PATCH v3 2/4] tcg: Make tb_cflags() usable from target-agnostic code Ilya Leoshkevich
@ 2023-12-11 18:47   ` Richard Henderson
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2023-12-11 18:47 UTC (permalink / raw)
  To: Ilya Leoshkevich, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé
  Cc: qemu-devel

On 12/7/23 16:35, Ilya Leoshkevich wrote:
> Currently tb_cflags() is defined in exec-all.h, which is not usable
> from target-agnostic code. Move it to translation-block.h, which is.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   include/exec/exec-all.h          | 6 ------
>   include/exec/translation-block.h | 6 ++++++
>   2 files changed, 6 insertions(+), 6 deletions(-)

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


r~


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c
  2023-12-08  0:35 ` [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c Ilya Leoshkevich
  2023-12-08  9:53   ` Alex Bennée
@ 2023-12-11 18:49   ` Richard Henderson
  1 sibling, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2023-12-11 18:49 UTC (permalink / raw)
  To: Ilya Leoshkevich, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé
  Cc: qemu-devel

On 12/7/23 16:35, Ilya Leoshkevich wrote:
> Preparation for moving perf.c to tcg/.
> 
> This affects only profiling guest code, which has code in a non-0 based
> segment, e.g., 16-bit code, which is not particularly important.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   accel/tcg/perf.c | 4 ----
>   1 file changed, 4 deletions(-)
> 


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


r~


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/4] accel/tcg: Move perf and debuginfo support to tcg
  2023-12-08  0:35 ` [PATCH v3 4/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
@ 2023-12-11 18:52   ` Richard Henderson
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2023-12-11 18:52 UTC (permalink / raw)
  To: Ilya Leoshkevich, Paolo Bonzini, Laurent Vivier, Peter Xu,
	David Hildenbrand, Philippe Mathieu-Daudé
  Cc: qemu-devel

On 12/7/23 16:35, Ilya Leoshkevich wrote:
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -63,7 +63,7 @@
>   #include "tb-context.h"
>   #include "internal-common.h"
>   #include "internal-target.h"
> -#include "perf.h"
> +#include "tcg/perf.h"
>   #include "tcg/insn-start-words.h"

Because this header is used outside of tcg/, the header should be include/tcg/perf.h.

> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index e7a9b3775bb..b8e52f3fb0f 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -62,7 +62,7 @@
>   #include "hw/boards.h"
>   #include "qemu/cutils.h"
>   #include "sysemu/runstate.h"
> -#include "accel/tcg/debuginfo.h"
> +#include "tcg/debuginfo.h"
>   
>   #include <zlib.h>
>   
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index cf9e74468b1..62120c76151 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -21,7 +21,7 @@
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
>   #include "target_signal.h"
> -#include "accel/tcg/debuginfo.h"
> +#include "tcg/debuginfo.h"
>   
>   #ifdef TARGET_ARM
>   #include "target/arm/cpu-features.h"

Likewise.

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

r~



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-12-11 18:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-08  0:35 [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
2023-12-08  0:35 ` [PATCH v3 1/4] target: Make qemu_target_page_mask() available for *-user Ilya Leoshkevich
2023-12-08 10:58   ` Philippe Mathieu-Daudé
2023-12-08  0:35 ` [PATCH v3 2/4] tcg: Make tb_cflags() usable from target-agnostic code Ilya Leoshkevich
2023-12-11 18:47   ` Richard Henderson
2023-12-08  0:35 ` [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c Ilya Leoshkevich
2023-12-08  9:53   ` Alex Bennée
2023-12-08 10:35     ` Ilya Leoshkevich
2023-12-11 18:49   ` Richard Henderson
2023-12-08  0:35 ` [PATCH v3 4/4] accel/tcg: Move perf and debuginfo support to tcg Ilya Leoshkevich
2023-12-11 18:52   ` Richard Henderson
2023-12-08 11:01 ` [PATCH v3 0/4] " Philippe Mathieu-Daudé

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).